Skip to content

Adding CleanZookeeper utility to accumulo admin command#2925

Closed
cshannon wants to merge 3 commits intoapache:mainfrom
cshannon:cleanZoo
Closed

Adding CleanZookeeper utility to accumulo admin command#2925
cshannon wants to merge 3 commits intoapache:mainfrom
cshannon:cleanZoo

Conversation

@cshannon
Copy link
Copy Markdown
Contributor

@cshannon cshannon commented Sep 10, 2022

Found another utility that can be moved to the accumulo admin command. This utility cleans all old instances out of Zookeeper (except the current one). It's a bit different than the DeleteZooInstance command as that command allows deleting a single instance that is specified so it's worth having both.

This is a follow on to #2807

Copy link
Copy Markdown
Contributor

@dlmarion dlmarion left a comment

Choose a reason for hiding this comment

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

Small nit, LGTM

cshannon and others added 2 commits September 14, 2022 11:33
…nZookeeper.java


Update log statement

Co-authored-by: Dave Marion <dlmarion@apache.org>
@cshannon
Copy link
Copy Markdown
Contributor Author

@milleruntime - Any thoughts on this before merging since you looked at the others?

@milleruntime
Copy link
Copy Markdown
Contributor

This looks good but #2914 is a priority and they both touch Admin.

@ctubbsii
Copy link
Copy Markdown
Member

Found another utility that can be moved to the accumulo admin command. This utility cleans all old instances out of Zookeeper (except the current one). It's a bit different than the DeleteZooInstance command as that command allows deleting a single instance that is specified so it's worth having both.

I'm not sure it's worth having both. The delete one could just have a flag to delete all non-current ones.

@cshannon
Copy link
Copy Markdown
Contributor Author

Found another utility that can be moved to the accumulo admin command. This utility cleans all old instances out of Zookeeper (except the current one). It's a bit different than the DeleteZooInstance command as that command allows deleting a single instance that is specified so it's worth having both.

I'm not sure it's worth having both. The delete one could just have a flag to delete all non-current ones.

I like that idea, I can simply move this functionality to the other command pretty easily and have a flag as you suggested. Having 1 utility/command to clean up zookeeper instances is probably less confusing and I can just update the troubleshooting documentation.

If no one else has any other comments or objects I can create a new PR tomorrow (and close this one) that moves the functionality here into the other utility and then removes the CleanZookeeper utility as it won't be needed.

@cshannon
Copy link
Copy Markdown
Contributor Author

Closing this PR out as I'm going to create a new one and just merge the functionality of CleanZookeeper into the DeleteZooInstance utility with a flag as suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants