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/09/08 14:42:25 UTC

[GitHub] [hbase] Apache9 opened a new pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

Apache9 opened a new pull request #2366:
URL: https://github.com/apache/hbase/pull/2366


   


----------------------------------------------------------------
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] infraio commented on pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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


   The overall plan is? Which methods should move to RegionStateStore and which should keep? 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] Apache9 commented on a change in pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
##########
@@ -490,12 +493,56 @@ static Put addMergeRegions(Put put, Collection<RegionInfo> mergeRegions) throws
   // ============================================================================================
   //  Delete Region State helpers
   // ============================================================================================
+  /**
+   * Deletes the specified region.
+   */
   public void deleteRegion(final RegionInfo regionInfo) throws IOException {
     deleteRegions(Collections.singletonList(regionInfo));
   }
 
+  /**
+   * Deletes the specified regions.
+   */
   public void deleteRegions(final List<RegionInfo> regions) throws IOException {
-    MetaTableAccessor.deleteRegionInfos(master.getConnection(), regions);
+    deleteRegions(regions, EnvironmentEdgeManager.currentTime());
+  }
+
+  private void deleteRegions(List<RegionInfo> regions, long ts) throws IOException {
+    List<Delete> deletes = new ArrayList<>(regions.size());
+    for (RegionInfo hri : regions) {
+      Delete e = new Delete(hri.getRegionName());
+      e.addFamily(HConstants.CATALOG_FAMILY, ts);
+      deletes.add(e);
+    }
+    try (Table table = getMetaTable()) {
+      debugLogMutations(deletes);
+      table.delete(deletes);
+    }
+    LOG.info("Deleted {} regions from META", regions.size());
+    LOG.debug("Deleted regions: {}", regions);
+  }
+
+  /**
+   * Overwrites the specified regions from hbase:meta. Deletes old rows for the given regions and
+   * adds new ones. Regions added back have state CLOSED.
+   * @param connection connection we're using
+   * @param regionInfos list of regions to be added to META
+   */
+  public void overwriteRegions(List<RegionInfo> regionInfos, int regionReplication)
+    throws IOException {
+    // use master time for delete marker and the Put
+    long now = EnvironmentEdgeManager.currentTime();
+    deleteRegions(regionInfos, now);
+    // Why sleep? This is the easiest way to ensure that the previous deletes does not

Review comment:
       I think we'd better leave the comment here? It is the history of why the code becomes like this.




----------------------------------------------------------------
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] Apache9 commented on pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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


   > The overall plan is? Which methods should move to RegionStateStore and which should keep? Thanks.
   
   The final goal is to move all the meta edit related methods to RegionStateStore. As now we only allow editing meta from master, still leaving these methods in MetaTableAccessor will give developer an impression that you can modify meta table if you have a Connection instance, which is incorrect. But it is not easy to finish them at once(I tried in HBASE-24985 but the patch is too big...), so I plan to do them piece by piece.


----------------------------------------------------------------
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 #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   4m  0s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 22s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 19s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 35s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 37s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 17s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 16s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 50s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2366 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 0d9ea3b69007 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0d95a8f91b |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-balancer hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/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] Apache-HBase commented on pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  1s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 33s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 55s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 31s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 14s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 14s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 34s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 38s |  hbase-server generated 1 new + 28 unchanged - 0 fixed = 29 total (was 28)  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 23s |  hbase-balancer in the patch passed.  |
   | +1 :green_heart: |  unit  | 145m  1s |  hbase-server in the patch passed.  |
   |  |   | 174m 10s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2366 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f8d0dc535a1a 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0d95a8f91b |
   | Default Java | 1.8.0_232 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/testReport/ |
   | Max. process+thread count | 4309 (vs. ulimit of 12500) |
   | modules | C: hbase-balancer hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/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] infraio commented on a change in pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
##########
@@ -490,12 +493,56 @@ static Put addMergeRegions(Put put, Collection<RegionInfo> mergeRegions) throws
   // ============================================================================================
   //  Delete Region State helpers
   // ============================================================================================
+  /**
+   * Deletes the specified region.
+   */
   public void deleteRegion(final RegionInfo regionInfo) throws IOException {
     deleteRegions(Collections.singletonList(regionInfo));
   }
 
+  /**
+   * Deletes the specified regions.
+   */
   public void deleteRegions(final List<RegionInfo> regions) throws IOException {
-    MetaTableAccessor.deleteRegionInfos(master.getConnection(), regions);
+    deleteRegions(regions, EnvironmentEdgeManager.currentTime());
+  }
+
+  private void deleteRegions(List<RegionInfo> regions, long ts) throws IOException {
+    List<Delete> deletes = new ArrayList<>(regions.size());
+    for (RegionInfo hri : regions) {
+      Delete e = new Delete(hri.getRegionName());
+      e.addFamily(HConstants.CATALOG_FAMILY, ts);
+      deletes.add(e);
+    }
+    try (Table table = getMetaTable()) {
+      debugLogMutations(deletes);
+      table.delete(deletes);
+    }
+    LOG.info("Deleted {} regions from META", regions.size());
+    LOG.debug("Deleted regions: {}", regions);
+  }
+
+  /**
+   * Overwrites the specified regions from hbase:meta. Deletes old rows for the given regions and
+   * adds new ones. Regions added back have state CLOSED.
+   * @param connection connection we're using
+   * @param regionInfos list of regions to be added to META
+   */
+  public void overwriteRegions(List<RegionInfo> regionInfos, int regionReplication)
+    throws IOException {
+    // use master time for delete marker and the Put
+    long now = EnvironmentEdgeManager.currentTime();
+    deleteRegions(regionInfos, now);
+    // Why sleep? This is the easiest way to ensure that the previous deletes does not

Review comment:
       This comment not used anymore?




----------------------------------------------------------------
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] Apache9 merged pull request #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

Posted by GitBox <gi...@apache.org>.
Apache9 merged pull request #2366:
URL: https://github.com/apache/hbase/pull/2366


   


----------------------------------------------------------------
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 #2366: HBASE-25000 Move delete region info related methods to RegionStateStore

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 33s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  5s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-balancer in master failed.  |
   | -0 :warning: |  javadoc  |   0m 48s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 26s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 18s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 16s |  hbase-balancer in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 43s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 24s |  hbase-balancer in the patch passed.  |
   | +1 :green_heart: |  unit  | 129m 25s |  hbase-server in the patch passed.  |
   |  |   | 161m 48s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2366 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 69c924edc0c0 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 0d95a8f91b |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-balancer.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-balancer.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/1/testReport/ |
   | Max. process+thread count | 4118 (vs. ulimit of 12500) |
   | modules | C: hbase-balancer hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2366/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