-
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-24675: On Master restart all servers are assigned to default rs… #2053
Conversation
🎊 +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.
+1
@@ -591,7 +591,7 @@ private synchronized void refresh(boolean forceOnline) throws IOException { | |||
|
|||
// This is added to the last of the list so it overwrites the 'default' rsgroup loaded | |||
// from region group table or zk | |||
groupList.add(new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP, getDefaultServers())); | |||
groupList.add(new RSGroupInfo(RSGroupInfo.DEFAULT_GROUP, getDefaultServers(groupList))); |
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.
refresh fetching info from rsgroup table / ZK makes sense.
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.
So the issue is by this time, the in memory state 'holder' not created in the HM restart case. This is a very bad bug.. This should go into all active branches no?
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.
branch-1 fix was pending, raised PR #2102
FYI @Apache9 |
ee40730
to
6e7346a
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
…group Closes #2053 Signed-off-by: Viraj Jasani <[email protected]>
…group Closes #2053 Signed-off-by: Viraj Jasani <[email protected]>
return getDefaultServers(listRSGroups()/* get from rsGroupMap */); | ||
} | ||
|
||
// Called by ServerEventsListenerThread. Presume it has lock on this manager when it runs. |
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.
Also pls leave this comment. The ServerEventsListenerThread calls the above method only.
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.
right, comment is not relevant on getDefaultServers(List rsGroupInfoList) method.
but this PR is already merged. shall i create new PR
…group Closes #2053 Signed-off-by: Viraj Jasani <[email protected]>
…group Closes apache#2053 Signed-off-by: Viraj Jasani <[email protected]>
…group Closes apache#2053 Author: Mohammad Arshad Reason: Bug Ref: CDPD-15964 Signed-off-by: Viraj Jasani <[email protected]> Change-Id: I895e3f52e8b1b2a5d5bf3955e0526a09ad2748d1 (cherry picked from commit a2c63df)
…group Closes apache#2053 Signed-off-by: Viraj Jasani <[email protected]> (cherry picked from commit 8dec49a) Change-Id: I895e3f52e8b1b2a5d5bf3955e0526a09ad2748d1
…group.