-
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-28342 Decommissioned hosts should be rejected by the HMaster #5681
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
Show resolved
Hide resolved
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
...er/src/main/java/org/apache/hadoop/hbase/master/HostIsConsideredDecommissionedException.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
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. |
💔 -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. |
@ndimiduk, can you re-trigger a build manually for this PR? The CI keeps checking out old and outdated code it seems. The compile step is complaining about a file that no longer exists in my branch:
|
🎊 +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.
Overall the product change looks good to me, just some small questions. However, I think that your tests are not really valid because all region servers running in the mini-cluster context will have the same hostname.
hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/org/apache/hadoop/hbase/master/HostIsConsideredDecommissionedException.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
Show resolved
Hide resolved
...server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerReportForDuty.java
Outdated
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. |
🎊 +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.
Looks good to me. Let's see if anyone else has something to say about it. Meanwhile, @aalhour do you mind opening backport PRs for branch-3 and branch-2?
…pache#5681) Signed-off by: Nick Dimiduk <[email protected]>
…pache#5681) Signed-off by: Nick Dimiduk <[email protected]>
…pache#5681) Signed-off by: Nick Dimiduk <[email protected]>
…5681) Signed-off by: Nick Dimiduk <[email protected]>
…pache#5681) Signed-off by: Nick Dimiduk <[email protected]>
…pache#5681) Signed-off by: Nick Dimiduk <[email protected]>
…5681) Signed-off by: Nick Dimiduk <[email protected]> Co-authored-by: Ahmad Alhour <[email protected]>
…5681) Signed-off by: Nick Dimiduk <[email protected]> Co-authored-by: Ahmad Alhour <[email protected]>
… by the HMaster (apache#5681) Signed-off by: Nick Dimiduk <[email protected]> Co-authored-by: Ahmad Alhour <[email protected]>
… by the HMaster (apache#5681) Signed-off by: Nick Dimiduk <[email protected]> Co-authored-by: Ahmad Alhour <[email protected]>
Fixes: HBASE-28342.
This PR adds support to specifying a new configuration value in
hbase-site.xml
which is:"hbase.master.reject.decommissioned.hosts"
.When the value is set to
true
the Master will reject RegionServers reporting for duty if and only if their hostnames match any of the servers currently present in the list of decommissioned servers (in the draining servers list). Default value for the configuration isfalse
. This configuration is also implemented as live-loadable, theServerManager
class now implements theConfigurationObserver
interface and listens to changes to this value.This small patch allows operators to prevent faulty hosts from rejoining the cluster by specifying that configuration property to
true
, decommissioning the servers and then work on fixing them. Once fixed, operators can then rollback the config value tofalse
which ends up allowing the RegionServer to rejoin from now fixed host.