-
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-22758 Remove the unneccesary info cf deletion in DeleteTableProcedure#deleteFromMeta #424
Conversation
💔 -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.
And I think the intention here is that, after the deletion, typically there should be no remaining rows, so the scan will return immediately?
@@ -1819,7 +1819,8 @@ private static void updateLocation(Connection connection, RegionInfo regionInfo, | |||
* @param regionInfo region to be deleted from META | |||
* @throws IOException | |||
*/ | |||
public static void deleteRegion(Connection connection, RegionInfo regionInfo) throws IOException { | |||
public static void deleteRegionInfo(Connection connection, RegionInfo regionInfo) |
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.
Why change the method name?
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.
Because the method only delete the info column family (will keep other cf), so name it deleteRegionInfo will be better.
@@ -1832,16 +1833,17 @@ public static void deleteRegion(Connection connection, RegionInfo regionInfo) th | |||
* @param connection connection we're using | |||
* @param regionsInfo list of regions to be deleted from META | |||
*/ | |||
public static void deleteRegions(Connection connection, List<RegionInfo> regionsInfo) | |||
public static void deleteRegionsInfo(Connection connection, List<RegionInfo> regionsInfo) |
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.
And this should be deleteRegionInfos?
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.
Sounds good, will do.
💔 -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.
Seems good to me (after fixing tests...)
💔 -1 overall
This message was automatically generated. |
OK, the new introduced TestCatalogJanitorCluster is depending on the deleteRegions now, it will broke the compiling. Let me update the patch. |
…cedure#deleteFromMeta
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
…cedure#deleteFromMeta (apache#424)
…cedure#deleteFromMeta (apache#424) (cherry picked from commit 09751f6) Change-Id: I2a9eb0f89c018d66e70dd5d3904c848b53cb6e3f
No description provided.