-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-28151 Option to allow/disallow bypassing pre transit check for assing/unassign #5493
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Test failure seems unrealted to the change |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
I'm not a fan of forceOverride
. I would prefer it named something like force
and in the javadoc and hbck docs explain it as "force
skips any pre flight checks that would otherwise prevent the operation". This is a relatively minor detail and perhaps reasonable people can disagree. Giving my approval for the functional changes. Please do consider the naming update before committing this.
I wonder if we should make changes the other way around i.e. first make the changes in hbase-operator-tools by adding new option, documenting it and only when operator-tools bumps hbase version, we can commit this change here? WDYT @rahulLiving @apurtell @Apache9 ? |
Adding an option in |
Oh right, that won't work. |
@rahulLiving This API change would break old operator-tools's hbck2. Please make sure it is backward compatible. |
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.
@rahulLiving This API change would break old operator-tools's hbck2. Please make sure it is backward compatible.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@rvaleti @virajjasani I have updated the PR to keep backward compatibility i.e default is |
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.
lgtm
@virajjasani @apurtell Can you please help with the merge |
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.
Changes seem good at high level as force is enabled by default, unless there is any objection, let's get this merged sometime next week.
Though now i am convinced that we usually do need to bypass transit state checks almost majority of the times when we have to use hbck in prod :) |
Jira: HBASE-28151