-
Notifications
You must be signed in to change notification settings - Fork 89
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
[admin-tool] Add a cluster batch processing framework command and a system store empty push task #1254
Conversation
53d2c05
to
bd3bc34
Compare
bd3bc34
to
10096f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Few minor comments
clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java
Outdated
Show resolved
Hide resolved
clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java
Outdated
Show resolved
Hide resolved
clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java
Outdated
Show resolved
Hide resolved
clients/venice-admin-tool/src/main/java/com/linkedin/venice/ClusterTaskRunner.java
Outdated
Show resolved
Hide resolved
clients/venice-admin-tool/src/main/java/com/linkedin/venice/ClusterTaskRunner.java
Outdated
Show resolved
Hide resolved
clients/venice-admin-tool/src/main/java/com/linkedin/venice/SystemStorePushTask.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good just some minor comments.
clients/venice-admin-tool/src/main/java/com/linkedin/venice/SystemStorePushTask.java
Outdated
Show resolved
Hide resolved
clients/venice-admin-tool/src/main/java/com/linkedin/venice/SystemStorePushTask.java
Outdated
Show resolved
Hide resolved
10096f7
to
d1611a0
Compare
clients/venice-admin-tool/src/main/java/com/linkedin/venice/AdminTool.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @sixpluszero! Just one minor comment. Please feel free to discard it if you like current name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
[admin-tool] Add a cluster batch processing framework command and a system store empty push task
This is just a side-effect PR I created for batch processing all stores in cluster when I am empty pushing all system stores to apply config updates.
Add the new command so you can write your store-oriented task and execute it cluster-wide with the admin-tool. It supports basic checkpointing and parallel processing.
With this, there is a system store empty push task in this PR as well, it did sanity checks and empty push to all system stores for a specific user store.
I think it can be further optimized and used for other purpose, but I found it efficient to empty push system store to apply ZK-shared store config update.
How was this PR tested?
This tool has been directly used in test environment and production environment.
Does this PR introduce any user-facing changes?