-
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-24875 Remove the force param for unassign since it dose not take effect any more #2254
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Bit confused by the original description here, it suggests this was only a doc change, but there are new methods definitions and deprecations in the API.
Can you explain better the motivation behind this change?
Yeah, it indeed confusion, updated yet. |
…e effect any more
This comment has been minimized.
This comment has been minimized.
Is this driven by https://issues.apache.org/jira/browse/HBASE-20881 ? |
Seems earlier, by https://issues.apache.org/jira/browse/HBASE-14614(AMv2), though the UnassignProcedure.java has force variate, but does not take effect. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Okk sounds good, in this case, you can also evaluate removing |
No prob. @virajjasani |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
Outdated
Show resolved
Hide resolved
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
Show resolved
Hide resolved
@@ -537,8 +537,8 @@ def assign(region_name) | |||
|
|||
#---------------------------------------------------------------------------------------------- | |||
# Unassign a region | |||
def unassign(region_name, force) | |||
@admin.unassign(region_name.to_java_bytes, java.lang.Boolean.valueOf(force)) | |||
def unassign(region_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.
this breaks compatibility for the shell and will need to be release noted. Alternatively you could do the analagous thing as in the Admin apis and deprecate the second parameter by making it optional and stating it is ignored.
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.
Will fix later. Thanks.
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.
this compatibility break is still present. could you correct it similar to how you did for the unassign shell command?
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.
Fixed, Thanks.
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/UnassignProcedure.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
Show resolved
Hide resolved
🎊 +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. |
Ping. @busbey |
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.
updated the thread on the hbase shell Admin class having a compatibility issue.
Fixed, thanks. @busbey |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Ping. @busbey |
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.
looks good. thanks for working through things!
Thanks a lot. |
…e effect any more (#2254) Modified compared to main branch to deprecate obviated MasterObserver interface methods instead of remove them. Signed-off-by: Sean Busbey <busbey@apache.org> (cherry picked from commit c5ca191) Conflicts: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java
…e effect any more (apache#2254) Modified compared to main branch to deprecate obviated MasterObserver interface methods instead of remove them. Signed-off-by: Sean Busbey <busbey@apache.org> (cherry picked from commit c5ca191) Conflicts: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java
No description provided.