-
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-27333 Abort RS when the hostname is different from master seen #4732
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Mind explain a bit? What do you mean by opposite? |
Thank, @Apache9 . The problem here is that the condition for whether using the servername seen from master or seen from local regionserver when they are different is opposite. It' a problem since branch-1. After line #1347 in HRegionServer, the servername of the regionserver is set to the name seen from master. But if when In short, when the hostname seen from local regionserver and master is different, the current behavior from the code should either restart the regionserver or use the local configed hostname, never the hostname seen from master. Correct me if I missed something. Thanks. |
22e6bb5
to
52632d3
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
I think we are good here @Apache9 ? Going to merge this tomorrow unless objection. |
This is introduced by https://issues.apache.org/jira/browse/HBASE-12954. It seemed that we wanted to solve the problem where a region server can not be accessed through the hostname which we bind to. From my understanding, the current logic seems reasonable. If we have configured the hostname to use, but it is different from what master seen us, then this should be a critical problem and we should abort the region server. If not, we just log it, and use the hostname seen by master as it is more likely to be accessed by others. Does anyone remember the reason why the code is like this? |
Thanks, @Apache9 and @apurtell . After looking at HBASE-12954, I think we should abort the RS when the hostname from master seen is different from RS seen, regardless of whether the hostname has been configured by the RS. |
🎊 +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 don't recall why the code is like this.
Since this PR has changed, I've changed my review status to Request Changes. TestMasterUseIp and TestRegionServerUseIp failures might be related. Will approve once the tests are green, or failures are excluded as not related.
Thanks @apurtell for the review, I'll dig more. |
The UT failed after HBASE-27304 #4713, @2005hithlj, can you take a look? |
@sunhelly |
I tested locally, and they definitely failed, @2005hithlj . |
@sunhelly |
@2005hithlj Good. |
@sunhelly In addition, if hbase.unsafe.regionserver.hostname/hbase.master.hostname is not null, even if hbase.server.useip.enabled is true, useThisHostnameInstead will work. If hbase.unsafe.regionserver.hostname/hbase.master.hostname is null, and hbase.server.useip.enabled is true, the Master and RegionServers info saved in meta and ZK are exposed to the client in the form of IP, which is the purpose of my PR(please see HBASE-27304 for details). This logic seems to be all right. |
💔 -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. |
…pache#4732) Signed-off-by: Andrew Purtell <[email protected]>
…4732) (#5148) Signed-off-by: Andrew Purtell <[email protected]>
No description provided.