HBASE-24186: RegionMover ignores replicationId #1512
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add replicationId to RegionMover logic
|
🎊 +1 overall
This message was automatically generated. |
Contributor
|
Pending QA |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
HorizonNet
reviewed
Apr 14, 2020
| } | ||
| HRegionLocation loc = | ||
| conn.getRegionLocator(region.getTable()).getRegionLocation(region.getStartKey(), true); | ||
| conn.getRegionLocator(region.getTable()).getRegionLocation(region.getStartKey(), |
Contributor
There was a problem hiding this comment.
Is there already a test, which covers the old as well as the new behavior?
Contributor
There was a problem hiding this comment.
Existing test cases are good. But yes good to add additional test case specific for replicaId. @BukrosSzabolcs can you add a test case in TestRegionMover ?
Contributor
Author
There was a problem hiding this comment.
The issue was found by a test. IntegrationTestTimeBoundedRequestsWithRegionReplicas failed while running with chaos monkey using GracefulRollingRestartRsAction. So there is coverage just not in TestRegionMover. Would that be sufficient?
Contributor
There was a problem hiding this comment.
It's great that IT test found this issue but they are not run as part of QA build. Hence, if we can have one unit test, that would be really great. QA will always run it. Sounds good @BukrosSzabolcs ?
Contributor
There was a problem hiding this comment.
However, since we have an IT that covers this, ideally it should be fine and maybe you can take up unit test as separate child Jira of this Jira.
Should be good @HorizonNet ?
Contributor
Author
There was a problem hiding this comment.
I realized my original description of the issue is not correct. This does not prevent the moving of the region, the result of getServerNameForRegion() is only used in the validation of the move, so it only forces the move to try to repeat itself because it does not realize the move already happened. So it just slows down the process but not break it.
I tried to write a test for this, but the only thing I could come up with is a test that goes trough regions on a rs and checks if getServerNameForRegion founds the correct server for it. I can add it if you insist but I see very little added value.
HorizonNet
approved these changes
Apr 16, 2020
virajjasani
pushed a commit
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
virajjasani
pushed a commit
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
virajjasani
pushed a commit
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
virajjasani
pushed a commit
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
virajjasani
pushed a commit
that referenced
this pull request
Apr 16, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
Contributor
Author
|
Thanks a lot for the review and the merges! |
ddupg
pushed a commit
to ddupg/hbase
that referenced
this pull request
Apr 20, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
infraio
pushed a commit
to infraio/hbase
that referenced
this pull request
Aug 17, 2020
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]>
saintstack
added a commit
that referenced
this pull request
Nov 4, 2020
This reverts commit 975d246. Breaks build.
symat
pushed a commit
to symat/hbase
that referenced
this pull request
Feb 17, 2021
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]> (cherry picked from commit bd574ad) Change-Id: I1cd1e05b3dc13455684be864728303086d742ec1
wchevreuil
pushed a commit
to wchevreuil/hbase
that referenced
this pull request
May 24, 2021
Signed-off-by: Jan Hentschel <[email protected]> Signed-off-by: Viraj Jasani <[email protected]> (cherry picked from commit bd574ad) Change-Id: Iae6341e16adc862ad87cb32c9c8d819e0ef4c78a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add replicationId to RegionMover logic