Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CLI command to clear state of a specific instance or of all instances #1530

Merged
merged 1 commit into from
May 23, 2024

Conversation

slinkydeveloper
Copy link
Contributor

Similar to #1529, part of #898

@AhmedSoliman
Copy link
Contributor

Can you include usage, and output examples (or screenshots) in the PR description?

Copy link
Contributor

@AhmedSoliman AhmedSoliman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one question about the value and we should be good to go.
We should keep an eye on how this will look in reality, enumerating the state can be too large to practically view them in a table like this and/or this will cause a churn due to fast moving state and version invalidation inflight. I'm personally not a fond of the approach to enumerate and send a request per state key from the CLI but happy to be proven wrong by letting this go as an experiment.

service_name: Option<String>,
service_key: Option<String>,
key: Option<String>,
value: Option<Vec<u8>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to get the value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for generating the "version number" (that is the hash of key and value for all the state of a given service)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if this can be done on the server side, value can be too big to fetch for such a large number of keys.

@slinkydeveloper
Copy link
Contributor Author

We should keep an eye on how this will look in reality, enumerating the state can be too large to practically view them in a table like this and/or this will cause a churn due to fast moving state and version invalidation inflight. I'm personally not a fond of the approach to enumerate and send a request per state key from the CLI but happy to be proven wrong by letting this go as an experiment.

The reason for this command (and kill/all from the previous PR) is to really serve the dev experience, right now the simplest solution when developing is always "wipe all" and this is not great. I generally agree with your sentiment, those two commands can be catastrophic in production, and perhaps in future we could disable those and/or make them more safe for a production environment. But right now we don't have this distinction, no "dev mode" or anything of that sort, and time is short so I'm not sure we'll introduce something like that, so I'm trying to at least give some tools to easily get out of the pickle without wiping all.

@slinkydeveloper slinkydeveloper merged commit b07da55 into restatedev:main May 23, 2024
4 checks passed
@slinkydeveloper slinkydeveloper deleted the issues/898 branch May 23, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants