HBASE-25206 Data loss can happen if a cloned table loses original spl… #2569
Conversation
This file contains 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
This comment has been minimized.
This comment has been minimized.
Apache9
requested changes
Oct 20, 2020
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java
Outdated
Show resolved
Hide resolved
|
They are added by this issue. https://issues.apache.org/jira/browse/HBASE-23187 Let me take a look on the PR about why we add these checks. |
|
Oh the logic is already there before HBASE-23187. In HBASE-23187 we just add checks for RegionInfo. Let me check for earlier changes. |
|
Oh it is in HBASE-14614, where the patch is several MB, which means we can not know the actual reason on why the code is like this... If you do not mind, I plan to open an issue first to address the problem of the include method, and then we could come back to fix the bug here. Thanks. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thank you for reviewing this! @Apache9
Sure. Please go ahead. Thanks. |
Apache9
requested changes
Oct 22, 2020
| @@ -100,6 +101,12 @@ protected Flow executeFromState(final MasterProcedureEnv env, DeleteTableState s | |||
| // TODO: Move out... in the acquireLock() | |||
| LOG.debug("Waiting for RIT for {}", this); | |||
| regions = env.getAssignmentManager().getRegionStates().getRegionsOfTable(getTableName()); | |||
There was a problem hiding this comment.
I think here, we could add a method in RegionStates to get regions of table for disabling, so we do not need to interate on the region state nodes of this table again.
|
@Apache9 I just modified the patch for your review. Can you please review it when you get a chance? Thanks. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Let me check the failed UTs. |
…it region(delete table)
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
I don't think the failed UT in the last QA is related to the patch. The UT was successful locally. |
Apache9
pushed a commit
that referenced
this pull request
Oct 24, 2020
…it region(delete table) (#2569) Signed-off-by: Duo Zhang <zhangduo@apache.org>
Apache9
pushed a commit
that referenced
this pull request
Oct 24, 2020
…it region(delete table) (#2569) Signed-off-by: Duo Zhang <zhangduo@apache.org>
Apache9
pushed a commit
that referenced
this pull request
Oct 24, 2020
…it region(delete table) (#2569) Signed-off-by: Duo Zhang <zhangduo@apache.org>
huaxiangsun
reviewed
Nov 3, 2020
| @@ -348,6 +348,7 @@ protected Flow executeFromState(MasterProcedureEnv env, RegionStateTransitionSta | |||
| LOG.error( | |||
| "Cannot assign replica region {} because its primary region {} does not exist.", | |||
| regionNode.getRegionInfo(), defaultRI); | |||
| regionNode.unsetProcedure(this); | |||
There was a problem hiding this comment.
This is a good catch, moving the region out of RIT.
clarax
pushed a commit
to clarax/hbase
that referenced
this pull request
Nov 15, 2020
…it region(delete table) (apache#2569) Signed-off-by: Duo Zhang <zhangduo@apache.org>
wchevreuil
pushed a commit
to wchevreuil/hbase
that referenced
this pull request
May 24, 2021
…it region(delete table) (apache#2569) Signed-off-by: Duo Zhang <zhangduo@apache.org> (cherry picked from commit da4d639) Change-Id: Ia75c7678d2273b42bfb8b2ed5ca33a612b5ad74c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…it region(delete table)