You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "aalhour (via GitHub)" <gi...@apache.org> on 2024/02/22 14:40:12 UTC

[PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

aalhour opened a new pull request, #5699:
URL: https://github.com/apache/hbase/pull/5699

   Fixes [HBASE-28354](https://issues.apache.org/jira/browse/HBASE-28354).
   
   This PR filters out the regions with `null` ServerName instances from the regions size calculations.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1959681076

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 24s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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  3s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 34s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 46s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 12s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  14m  4s |  hbase-mapreduce in the patch passed.  |
   |  |   |  33m 26s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 735c1395e7ca 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 7be588e0d4 |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/testReport/ |
   | Max. process+thread count | 2892 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on code in PR #5699:
URL: https://github.com/apache/hbase/pull/5699#discussion_r1499531595


##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java:
##########
@@ -96,11 +98,14 @@ private void init(RegionLocator regionLocator, Admin admin) throws IOException {
   }
 
   private Set<ServerName> getRegionServersOfTable(RegionLocator regionLocator) throws IOException {
-
     Set<ServerName> tableServers = Sets.newHashSet();
-    for (HRegionLocation regionLocation : regionLocator.getAllRegionLocations()) {
-      tableServers.add(regionLocation.getServerName());
-    }
+
+    // The region locations could contain `null` ServerName instances if the region is currently
+    // in transition, we filter those out for now, which impacts the size calculation for these
+    // regions temporarily until the ServerName gets filled in later
+    tableServers.addAll(regionLocator.getAllRegionLocations().stream()

Review Comment:
   Fixed, 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968643462

   Thanks for your quick address. @aalhour  Sorry that maybe I didn't express my thoughts clearly and caused you some confusion. In fact, we share some of the same views. 
   
   About the constant **UNKNOWN_SIZE**, I think maybe it should be defined as following:
   
   `private static final Size UNKNOWN_SIZE = new Size(1, Size.Unit.BYTE); `
   or
   `private static final long UNKNOWN_SIZE = 1;`
   
   The byte size of **UNKNOWN_SIZE** must be **greater** than 0 so that this input split will not be filtered out because we will use region byte size as the input split length. :)


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1959891082

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 23s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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  |   2m 41s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 37s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 45s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 34s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 12s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  14m  4s |  hbase-mapreduce in the patch passed.  |
   |  |   |  33m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux ae49833f3e83 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 7be588e0d4 |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/testReport/ |
   | Max. process+thread count | 2912 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968558053

   So, what you mean is that we can return `-1` as a value for regions that we don't have sizes for in the RegionSizeCalculator and that would be fine with the downstream dependencies? Why then make it a `byte` and not a `final long`? The method signature is going to return that byte as a long value.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968675456

   Hello @frostruan, thanks for clarifying, yeah that's in line with my previous thoughts, I was confused as to why we want to specify a byte and return it from a method that returns longs. Now it's clear. I have just pushed some changes, can you please take one final look and tell me if it's good to be merged?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1959876261

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 42s |  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 28s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 39s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 53s |  branch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 39s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 39s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 14s |  hbase-mapreduce: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |   5m 47s |  Patch does not cause any errors with Hadoop 3.3.6.  |
   | -1 :x: |  spotless  |   0m 47s |  patch has 23 errors when running spotless:check, run spotless:apply to fix.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m  8s |  The patch does not generate ASF License warnings.  |
   |  |   |  25m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
   | uname | Linux adf4ae570711 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 7be588e0d4 |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   | checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-mapreduce.txt |
   | spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/artifact/yetus-general-check/output/patch-spotless.txt |
   | Max. process+thread count | 79 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/console |
   | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1966144703

   @frostruan, which part of the code would handle that byte? Also, should that be added to this PR or yours (https://github.com/apache/hbase/pull/5700)?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968846003

   I think this PR is good enough to solve the problem it intends to solve. 
   
    +1 !
   
   I think there is still some room for optimization here, but this is another problem. We can file another issue to follow up.
   
   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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "ndimiduk (via GitHub)" <gi...@apache.org>.
ndimiduk commented on code in PR #5699:
URL: https://github.com/apache/hbase/pull/5699#discussion_r1506076104


##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java:
##########
@@ -115,7 +116,7 @@ public long getRegionSize(byte[] regionId) {
     Long size = sizeMap.get(regionId);
     if (size == null) {
       LOG.debug("Unknown region:" + Arrays.toString(regionId));
-      return 0;
+      return UNKNOWN_SIZE;

Review Comment:
   Please let's not conflate the two issues here. @frostruan if you want to change the 0-value response behavior, please do that in a dedicated ticket. We should have NPE protection independent of whatever solution is decided upon via HBASE-28399. In the mean time, this ticket can fix the NPE and return 0 in that case, which is consistent with the existing 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1961422323

   Thanks for the noticing @bbeaudreault 
   
   I think we can introduce a constant **UNKNOWN_SIZE** with type of Size and value of 1 byte to distinguish between having no data and not knowing the specific size. Another important reason for greater than 0 is as I mentioned in HBASE-28399, some computing engines will automatically prune empty partitions, if 0 is returned for in-transition regions, data may be lost.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1966464798

   I'm wondering what the byte constant would hold, other than `0`:
   - If it held `1`, it would mean that regions that no longer exist would still get reported with size (maybe?)
     - I'm having trouble understanding tests about inexistent regions and how HBase reacts to them eventually
   - If it held `-1`, we'd need to refactor the callers to handle the `< 0` case, which will make "UNKNOWN REGIONS" more explicit with comments
   
   Alternatively, if we return the average size of all other regions might be good but it will push the idea of a region in transition too deep into the RegionSizeCalculator and other areas won't know about it, maybe they should? What if there are currently no regions available? What if there is one (or more) region in transition and the size map is empty? What if the size map only contains a region that's empty? I am not sure if these cases are realistic, I'm still new to the codebase.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1966344106

   Also ping Duo~ @Apache9 
   
   This problem is related to my PR you reviewed yesterday, would you mind taking a look at this too ?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1969055381

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 47s |  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  |   4m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 45s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 55s |  branch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 41s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 22s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |   6m 28s |  Patch does not cause any errors with Hadoop 3.3.6.  |
   | +1 :green_heart: |  spotless  |   1m 12s |  patch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   1m 12s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 19s |  The patch does not generate ASF License warnings.  |
   |  |   |  29m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
   | uname | Linux 33165e59cbda 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   | Max. process+thread count | 79 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/console |
   | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1961245586

   I think the problem with that is it just introduces more code. I think there are 2-3 callers of this method and they all pass the result into an InputSplit which takes an int. So if we changed to optional we'd just end up having to update 3 places to do orElse(0)


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968740262

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 38s |  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 22s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 37s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 17s |  master passed  |
   | +1 :green_heart: |  spotless  |   1m  4s |  branch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |   5m 38s |  Patch does not cause any errors with Hadoop 3.3.6.  |
   | +1 :green_heart: |  spotless  |   0m 47s |  patch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 50s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  25m 51s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
   | uname | Linux 27db72339c2d 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   | Max. process+thread count | 79 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/console |
   | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "ndimiduk (via GitHub)" <gi...@apache.org>.
ndimiduk commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1971197788

   Thanks for the thoughtful reviews everyone!


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1970873939

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 47s |  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 54s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 13s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  8s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 22s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  19m  0s |  hbase-mapreduce in the patch passed.  |
   |  |   |  42m 55s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 11a0dbb66826 5.4.0-172-generic #190-Ubuntu SMP Fri Feb 2 23:24:22 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / fed3fdd89e |
   | Default Java | Eclipse Adoptium-17.0.10+7 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/testReport/ |
   | Max. process+thread count | 3490 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968753080

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 25s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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 21s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 46s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  13m 53s |  hbase-mapreduce in the patch passed.  |
   |  |   |  33m 40s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 6e2d1ee9eaa3 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/testReport/ |
   | Max. process+thread count | 2933 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968570283

   Cases where the region's size can be returned as `-1`:
   - [MultiTableInputFormatBase.java#L216](https://github.com/apache/hbase/blob/master/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableInputFormatBase.java#L216)
   - [mapreduce/TableInputFormatBase.java#L301](https://github.com/apache/hbase/blob/master/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java#L301)
   - [mapreduce/TableInputFormatBase.java#L345](https://github.com/apache/hbase/blob/master/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java#L345)
   
   Not sure if we should do anything in the [TableSplit.java](https://github.com/apache/hbase/blob/master/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableSplit.java) class which is used by the above.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on code in PR #5699:
URL: https://github.com/apache/hbase/pull/5699#discussion_r1506303056


##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java:
##########
@@ -115,7 +116,7 @@ public long getRegionSize(byte[] regionId) {
     Long size = sizeMap.get(regionId);
     if (size == null) {
       LOG.debug("Unknown region:" + Arrays.toString(regionId));
-      return 0;
+      return UNKNOWN_SIZE;

Review Comment:
   Ok, I agree. 



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1966499225

   I prefer to use 1 byte to represent unknown region size for two reasons:
   
   1. it cannot be a valid region size because even a minimal keyvalue is bigger than that.
   2. some computing engines, like spark, will filter out splits with size less than 0. You can see this for details.
   https://github.com/apache/spark/blob/v3.5.1-rc2/core/src/main/scala/org/apache/spark/rdd/NewHadoopRDD.scala#L138
   
   I'll try to answer your questions later, sorry have to catch the shuttle bus.
   
   Thanks. @aalhour 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1961441029

   > Per https://issues.apache.org/jira/browse/HBASE-28399 we might want to return something other than 0. Maybe even just 1, or maybe the average region size from the other regions.
   
   agree. I think 1 byte is enough. The size of any region with data cannot be 1 byte. In addition, I think we'd better add some more comments to tell users and developers the reasons for the context.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968644712

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 25s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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  |   2m 41s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 16s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 36s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 24s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 17s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 34s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 12s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  13m 53s |  hbase-mapreduce in the patch passed.  |
   |  |   |  32m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f4fc2944392e 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Temurin-1.8.0_352-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/testReport/ |
   | Max. process+thread count | 2886 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968754231

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 10s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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  |   2m 39s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m  4s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 26s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  6s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m  7s |  hbase-mapreduce in the patch passed.  |
   |  |   |  34m 34s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 954df016e909 5.4.0-172-generic #190-Ubuntu SMP Fri Feb 2 23:24:22 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Temurin-1.8.0_352-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/testReport/ |
   | Max. process+thread count | 2791 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/4/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1969449215

   > Out of curiosity, what remaining problems do you see that need attention? Can we write a quick ticket describing them? I'll see if there's appetite in the team (https://github.com/HubSpot) to work on them.
   
   Thanks a lot ! @aalhour 
   
   I used to think that we did not fully consider the situation of region merge/split. Once a region is split or merged, it may cause some overlap or loss of the data we read. However, after thinking about it again, I think it seems unlikely. , so please ignore me. :)


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1970783200

   Thanks @ndimiduk, @frostruan and @bbeaudreault for the feedback.
   
   I have just reverted the changes back to "returning 0 for unknown regions".


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1970871945

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  8s |  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 36s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 56s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 19s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 37s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 58s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  16m 57s |  hbase-mapreduce in the patch passed.  |
   |  |   |  41m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 24fbd431227b 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / fed3fdd89e |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/testReport/ |
   | Max. process+thread count | 2826 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1970870952

   :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  |   3m 11s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 12s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 51s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 54s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  19m 26s |  hbase-mapreduce in the patch passed.  |
   |  |   |  41m 16s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux c93b002b9b1e 5.4.0-169-generic #187-Ubuntu SMP Thu Nov 23 14:52:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / fed3fdd89e |
   | Default Java | Temurin-1.8.0_352-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/testReport/ |
   | Max. process+thread count | 3147 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1961422085

   Thanks for the noticing @bbeaudreault 
   
   I think we can introduce a constant **UNKNOWN_SIZE** with type of Size and value of 1 byte to distinguish between having no data and not knowing the specific size. Another important reason for greater than 0 is as I mentioned in HBASE-28399, some computing engines will automatically prune empty partitions, if 0 is returned for in-transition regions, data may be lost.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1961332040

   @frostruan what do you think?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1959694425

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  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 22s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m  1s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m  7s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 14s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m  2s |  hbase-mapreduce in the patch passed.  |
   |  |   |  40m 19s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7827af0a6078 5.4.0-169-generic #187-Ubuntu SMP Thu Nov 23 14:52:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 7be588e0d4 |
   | Default Java | Temurin-1.8.0_352-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/testReport/ |
   | Max. process+thread count | 2853 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5699:
URL: https://github.com/apache/hbase/pull/5699#discussion_r1499457380


##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java:
##########
@@ -96,11 +98,14 @@ private void init(RegionLocator regionLocator, Admin admin) throws IOException {
   }
 
   private Set<ServerName> getRegionServersOfTable(RegionLocator regionLocator) throws IOException {
-
     Set<ServerName> tableServers = Sets.newHashSet();
-    for (HRegionLocation regionLocation : regionLocator.getAllRegionLocations()) {
-      tableServers.add(regionLocation.getServerName());
-    }
+
+    // The region locations could contain `null` ServerName instances if the region is currently
+    // in transition, we filter those out for now, which impacts the size calculation for these
+    // regions temporarily until the ServerName gets filled in later
+    tableServers.addAll(regionLocator.getAllRegionLocations().stream()

Review Comment:
   rather than collect to a list just to add to a set, you could collect directly to a set



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1959666882

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 43s |  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 54s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 43s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 15s |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 57s |  branch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 43s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 37s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 39s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 39s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |   5m 35s |  Patch does not cause any errors with Hadoop 3.3.6.  |
   | +1 :green_heart: |  spotless  |   0m 56s |  patch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 51s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  26m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
   | uname | Linux f5a9de4ca7d5 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 7be588e0d4 |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   | Max. process+thread count | 77 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/1/console |
   | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968663119

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 42s |  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 30s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  3s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 21s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 42s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  1s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 24s |  hbase-mapreduce in the patch passed.  |
   |  |   |  41m 58s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 10aba391ed21 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/testReport/ |
   | Max. process+thread count | 2865 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968637258

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 56s |  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  |   4m 14s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 47s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 17s |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 56s |  branch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 41s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |   6m 31s |  Patch does not cause any errors with Hadoop 3.3.6.  |
   | +1 :green_heart: |  spotless  |   1m 17s |  patch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   1m 11s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 21s |  The patch does not generate ASF License warnings.  |
   |  |   |  29m  4s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
   | uname | Linux 017523f1539d 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   | Max. process+thread count | 77 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/3/console |
   | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1969078658

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 47s |  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 18s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 56s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 41s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  0s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m  2s |  hbase-mapreduce in the patch passed.  |
   |  |   |  41m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux dd2de3f607d9 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/testReport/ |
   | Max. process+thread count | 2828 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1970850153

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  6s |  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  |   4m 18s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 41s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 15s |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 54s |  branch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   0m 40s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 22s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 36s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |   6m 23s |  Patch does not cause any errors with Hadoop 3.3.6.  |
   | +1 :green_heart: |  spotless  |   1m 16s |  patch has no errors when running spotless:check.  |
   | +1 :green_heart: |  spotbugs  |   1m 10s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 19s |  The patch does not generate ASF License warnings.  |
   |  |   |  28m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
   | uname | Linux 3dbbc4a079c6 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / fed3fdd89e |
   | Default Java | Eclipse Adoptium-11.0.17+8 |
   | Max. process+thread count | 79 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/6/console |
   | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "ndimiduk (via GitHub)" <gi...@apache.org>.
ndimiduk merged PR #5699:
URL: https://github.com/apache/hbase/pull/5699


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1966711401

   > 1. If it held 1, it would mean that regions that no longer exist would still get reported with size (maybe?)
   
   I don't think regions in state transition and regions no longer exist are same thing. When we call regionLocator.getAllRegionLocations(), we will exlcude offlined split parent regions. You can see https://github.com/apache/hbase/blob/rel/3.0.0-beta-1/hbase-client/src/main/java/org/apache/hadoop/hbase/ClientMetaTableAccessor.java#L172 for details.
   
   2. I don't think it is appropriate to return the average size of other regions, we should not make any decisions for the user.
   
   > 3. What if there are currently no regions available? 
   
   I don't think this is a big deal. As @ndimiduk mentioned, it's just a hint for MapReduce scheduler. 
   
   Thanks. @aalhour 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "frostruan (via GitHub)" <gi...@apache.org>.
frostruan commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1966341246

   I think there are two problems here:
   1. For in-transition regions, the result of RegionLocation.getServerName() could be null. In the current PR, we just filter out these regions. With in-transition region missing, we may lost some data. 
   2. We first get all region servers with regions of target table, then we request each region server to get RegionMetrics. If any region is moved from this server during this period, this region will be missed too.
   
   For the first problem, I think maybe we can return something indicates that we can not know the specific data size now, so in the previous discussion, I propose introducing a new constant UNKNOWN_SIZE with value of 1 byte.
   For the second problem, I think maybe we'd better use a snapshot of cluster metrics to make sure we will not miss any region.
   
   What do you think ? 
   Thanks. @aalhour 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1961331421

   Per https://issues.apache.org/jira/browse/HBASE-28399 we might want to return something other than 0. Maybe even just 1, or maybe the average region size from the other regions.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1959893867

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 18s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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  |   2m 41s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 18s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m  5s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  5s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m  6s |  hbase-mapreduce in the patch passed.  |
   |  |   |  34m 44s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7943e0d2e2d7 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 7be588e0d4 |
   | Default Java | Temurin-1.8.0_352-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/testReport/ |
   | Max. process+thread count | 2798 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/2/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on code in PR #5699:
URL: https://github.com/apache/hbase/pull/5699#discussion_r1507305250


##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java:
##########
@@ -115,7 +116,7 @@ public long getRegionSize(byte[] regionId) {
     Long size = sizeMap.get(regionId);
     if (size == null) {
       LOG.debug("Unknown region:" + Arrays.toString(regionId));
-      return 0;
+      return UNKNOWN_SIZE;

Review Comment:
   Just reverted the changes.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "aalhour (via GitHub)" <gi...@apache.org>.
aalhour commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1968994818

   Awesome, thanks a lot @frostruan. I just pinged @ndimiduk to help me merge it.
   
   Out of curiosity, what remaining problems do you see that need attention? Can we write a quick ticket describing them? I'll see if there's appetite in the team (@HubSpot) to work on them.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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


Re: [PR] HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5699:
URL: https://github.com/apache/hbase/pull/5699#issuecomment-1969066409

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 13s |  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  |   2m 27s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 18s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m  4s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 26s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  7s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 21s |  hbase-mapreduce in the patch passed.  |
   |  |   |  34m 51s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/5699 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 93a431ca78b5 5.4.0-172-generic #190-Ubuntu SMP Fri Feb 2 23:24:22 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 4b552434db |
   | Default Java | Temurin-1.8.0_352-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/testReport/ |
   | Max. process+thread count | 2725 (vs. ulimit of 30000) |
   | modules | C: hbase-mapreduce U: hbase-mapreduce |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5699/5/console |
   | versions | git=2.34.1 maven=3.8.6 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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