-
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-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished #1629
Conversation
🎊 +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. |
Looks good, It actually addressed the issue for HBASE-24316. Let me put up an unitest as I planned to do for HBASE-24316. Mind put the following code in as well? (Please review first, :))
|
Yes, looks good. |
@saintstack could you help review this one ? This patch is related to HBASE-24250 |
🎊 +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.
If GCMultipleMergedRegionsProcedure processing is slower than the CatalogJanitor's scan interval, it will end resubmitting GCMultipleMergedRegionsProcedure for the same region, we can skip duplicate GCMultipleMergedRegionsProcedure
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.
.
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.
Can you address the nit @nyl3532016? Thanks.
@@ -78,6 +78,13 @@ protected Flow executeFromState(MasterProcedureEnv env, GCMergedRegionsState sta | |||
switch (state) { | |||
case GC_MERGED_REGIONS_PREPARE: | |||
// Nothing to do to prepare. |
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.
Can you review the comment line and new comment? Briefly explain what the code does.
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.
Hi @nyl3532016, can you put your above comment "If GCMultipleMergedRegionsProcedure processing is slower than the CatalogJanitor's scan interval, it will end resubmitting GCMultipleMergedRegionsProcedure for the same region, we can skip duplicate GCMultipleMergedRegionsProcedure" to the code above? It will help other developers's code reading easier. After that, you can merge, thanks.
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.
ok,thanks, I know. I will add this comment.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@nyl3532016 I merged the patch. Can you back port it to branch-2 and branch-2.3? Thanks |
No description provided.