You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/05/02 14:29:17 UTC

[GitHub] [hbase] nyl3532016 opened a new pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

nyl3532016 opened a new pull request #1629:
URL: https://github.com/apache/hbase/pull/1629


   … for the same region


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624066446


   > 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, :))
   > 
   > ```
   > diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   > index ac79e8aaff..e345483351 100644
   > --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   > +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   > @@ -1904,6 +1904,16 @@ public class MetaTableAccessor {
   >        qualifiers.add(qualifier);
   >        delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
   >      }
   > +
   > +    // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
   > +    // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
   > +    // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
   > +    if (qualifiers.isEmpty()) {
   > +      LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
   > +        " in meta table, they are cleaned up already, Skip.");
   > +      return;
   > +    }
   > +
   > ```
   
   Yes, looks good. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624155959


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  3s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 11s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  4s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  4s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  9s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 127m 58s |  hbase-server in the patch passed.  |
   |  |   | 152m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 5df9556206d1 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3340c0024e |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/testReport/ |
   | Max. process+thread count | 4173 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-622968754


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  8s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  1s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 22s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 16s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m  8s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  32m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 124d8ed191ce 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4f9eecbe61 |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 edited a comment on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
nyl3532016 edited a comment on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624068135


   @saintstack could you help review this one ? This patch is related to HBASE-24250


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on a change in pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#discussion_r419797107



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
##########
@@ -78,6 +78,13 @@ protected Flow executeFromState(MasterProcedureEnv env, GCMergedRegionsState sta
       switch (state) {
         case GC_MERGED_REGIONS_PREPARE:
           // Nothing to do to prepare.

Review comment:
       Can you review the comment line and new comment? Briefly explain what the code does.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624589153


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 10s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 15s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 43s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 50s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 57s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 29s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 46s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 214m 54s |  hbase-server in the patch passed.  |
   |  |   | 245m 43s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 57814e45deed 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / a9a1b9524d |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/testReport/ |
   | Max. process+thread count | 3167 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun edited a comment on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
huaxiangsun edited a comment on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623589712


   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, :))
   
   diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   index ac79e8aaff..e345483351 100644
   --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   @@ -1904,6 +1904,16 @@ public class MetaTableAccessor {
          qualifiers.add(qualifier);
          delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
        }
   +
   +    // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
   +    // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
   +    // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
   +    if (qualifiers.isEmpty()) {
   +      LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
   +        " in meta table, they are cleaned up already, Skip.");
   +      return;
   +    }
   +
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624560262


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  8s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 13s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m 23s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 17s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 17s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 17s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 17s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 150m 36s |  hbase-server in the patch passed.  |
   |  |   | 181m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux ab2c4aca2e8a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / a9a1b9524d |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/testReport/ |
   | Max. process+thread count | 4066 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on a change in pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#discussion_r420563488



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
##########
@@ -78,6 +78,13 @@ protected Flow executeFromState(MasterProcedureEnv env, GCMergedRegionsState sta
       switch (state) {
         case GC_MERGED_REGIONS_PREPARE:
           // Nothing to do to prepare.

Review comment:
       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.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 removed a comment on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
nyl3532016 removed a comment on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623927540


   @huaxiangsun, thanks for review .
    Yes,I think you can submit another PR for  HBASE-24316


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624495143


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 21s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 14s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 20s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  1s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 15s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 58s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 33s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  37m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 858e1f51f1d4 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / a9a1b9524d |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623927540


   @huaxiangsun, thanks for review .
    Yes,I think you can submit another PR for  HBASE-24316


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun edited a comment on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
huaxiangsun edited a comment on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623589712


   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, :))
   
   `diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   index ac79e8aaff..e345483351 100644
   --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   @@ -1904,6 +1904,16 @@ public class MetaTableAccessor {
          qualifiers.add(qualifier);
          delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
        }
   +
   +    // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
   +    // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
   +    // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
   +    if (qualifiers.isEmpty()) {
   +      LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
   +        " in meta table, they are cleaned up already, Skip.");
   +      return;
   +    }
   +
   `
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623117829


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 17s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 34s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 52s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 20s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 41s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 185m 57s |  hbase-server in the patch passed.  |
   |  |   | 213m 10s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 13e140a038eb 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4f9eecbe61 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/testReport/ |
   | Max. process+thread count | 3787 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624773194


   @nyl3532016 I merged the patch. Can you back port it to branch-2 and branch-2.3? Thanks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun edited a comment on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
huaxiangsun edited a comment on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623589712


   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, :))
   
   ```
   diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   index ac79e8aaff..e345483351 100644
   --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   @@ -1904,6 +1904,16 @@ public class MetaTableAccessor {
          qualifiers.add(qualifier);
          delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
        }
   +
   +    // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
   +    // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
   +    // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
   +    if (qualifiers.isEmpty()) {
   +      LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
   +        " in meta table, they are cleaned up already, Skip.");
   +      return;
   +    }
   +
   ```
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623094389


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  5s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  2s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 16s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m  2s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  31m 42s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 46623121a5dc 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4f9eecbe61 |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 commented on a change in pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#discussion_r420518956



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
##########
@@ -78,6 +78,13 @@ protected Flow executeFromState(MasterProcedureEnv env, GCMergedRegionsState sta
       switch (state) {
         case GC_MERGED_REGIONS_PREPARE:
           // Nothing to do to prepare.
+          List<RegionInfo> parents = MetaTableAccessor.getMergeRegions(
+            env.getMasterServices().getConnection(), mergedChild.getRegionName());
+          if (parents == null || parents.isEmpty()) {

Review comment:
       to check whether skip GCMultipleMergedRegionsProcedure




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-622993688


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  4s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 26s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 22s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 190m 57s |  hbase-server in the patch failed.  |
   |  |   | 215m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7d8839f5b6df 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4f9eecbe61 |
   | Default Java | 1.8.0_232 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/testReport/ |
   | Max. process+thread count | 3277 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624068135


   @saintstack could you help review this one ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623118229


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 27s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 58s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 58s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 25s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 191m 21s |  hbase-server in the patch passed.  |
   |  |   | 215m 45s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 8eff76ba34e2 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4f9eecbe61 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/testReport/ |
   | Max. process+thread count | 3253 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 commented on a change in pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#discussion_r420518956



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
##########
@@ -78,6 +78,13 @@ protected Flow executeFromState(MasterProcedureEnv env, GCMergedRegionsState sta
       switch (state) {
         case GC_MERGED_REGIONS_PREPARE:
           // Nothing to do to prepare.
+          List<RegionInfo> parents = MetaTableAccessor.getMergeRegions(
+            env.getMasterServices().getConnection(), mergedChild.getRegionName());
+          if (parents == null || parents.isEmpty()) {

Review comment:
       to check whether skip GCMultipleMergedRegionsProcedure




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624199253


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  0s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 25s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 40s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 58s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 58s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 23s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 196m 53s |  hbase-server in the patch passed.  |
   |  |   | 221m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 16b7e41496ec 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3340c0024e |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/testReport/ |
   | Max. process+thread count | 3441 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-622987844


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 42s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 41s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m 13s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 54s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 22s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 25s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 25s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m  9s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 52s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 137m 26s |  hbase-server in the patch failed.  |
   |  |   | 170m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 1ad8d168287e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4f9eecbe61 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/testReport/ |
   | Max. process+thread count | 4495 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-624086438


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 40s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  7s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  3s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  3s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 13s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 10s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  32m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1629 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 8119d2316331 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3340c0024e |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1629/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun edited a comment on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
huaxiangsun edited a comment on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623589712


   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, :))
   `diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   index ac79e8aaff..e345483351 100644
   --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   @@ -1904,6 +1904,16 @@ public class MetaTableAccessor {
          qualifiers.add(qualifier);
          delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
        }
   +
   +    // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
   +    // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
   +    // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
   +    if (qualifiers.isEmpty()) {
   +      LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
   +        " in meta table, they are cleaned up already, Skip.");
   +      return;
   +    }
   +
   `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #1629: HBASE-24250 CatalogJanitor resubmits GCMultipleMergedRegionsProcedure…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#issuecomment-623589712


   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, :))
   
   
   {code}
   diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   index ac79e8aaff..e345483351 100644
   --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
   @@ -1904,6 +1904,16 @@ public class MetaTableAccessor {
          qualifiers.add(qualifier);
          delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
        }
   +
   +    // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
   +    // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
   +    // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
   +    if (qualifiers.isEmpty()) {
   +      LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
   +        " in meta table, they are cleaned up already, Skip.");
   +      return;
   +    }
   +
   
   {code}


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] nyl3532016 commented on a change in pull request #1629: HBASE-24328 skip duplicate GCMultipleMergedRegionsProcedure while previous finished

Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #1629:
URL: https://github.com/apache/hbase/pull/1629#discussion_r420571160



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
##########
@@ -78,6 +78,13 @@ protected Flow executeFromState(MasterProcedureEnv env, GCMergedRegionsState sta
       switch (state) {
         case GC_MERGED_REGIONS_PREPARE:
           // Nothing to do to prepare.

Review comment:
       ok,thanks, I know. I will add this comment. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org