[GitHub] [hbase] Apache-HBase commented on pull request #3800: HBASE-26347 Support detect and exclude slow DNs in fan-out of WAL
Apache-HBase commented on pull request #3800: URL: https://github.com/apache/hbase/pull/3800#issuecomment-998546453 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 29s | 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 15s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 4m 17s | master passed | | +1 :green_heart: | compile | 3m 51s | master passed | | +1 :green_heart: | checkstyle | 1m 19s | master passed | | +1 :green_heart: | spotbugs | 2m 43s | master passed | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 4m 3s | the patch passed | | +1 :green_heart: | compile | 3m 51s | the patch passed | | -0 :warning: | javac | 0m 32s | hbase-asyncfs generated 2 new + 22 unchanged - 7 fixed = 24 total (was 29) | | +1 :green_heart: | checkstyle | 0m 13s | hbase-asyncfs: The patch generated 0 new + 1 unchanged - 1 fixed = 1 total (was 2) | | +1 :green_heart: | checkstyle | 1m 5s | The patch passed checkstyle in hbase-server | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | hadoopcheck | 20m 1s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. | | +1 :green_heart: | spotbugs | 3m 9s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 26s | The patch does not generate ASF License warnings. | | | | 55m 14s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3800/14/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3800 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile | | uname | Linux 28dacf538036 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3800/14/artifact/yetus-general-check/output/diff-compile-javac-hbase-asyncfs.txt | | Max. process+thread count | 96 (vs. ulimit of 3) | | modules | C: hbase-asyncfs hbase-server U: . | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3800/14/console | | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3970: HBASE-26447: Make enable/disable ReplicationPeer and addPeer no-op on retry
Apache-HBase commented on pull request #3970: URL: https://github.com/apache/hbase/pull/3970#issuecomment-998534333 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 25s | Docker mode activated. | | -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck | ||| _ Prechecks _ | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 4m 9s | master passed | | +1 :green_heart: | compile | 1m 0s | master passed | | +1 :green_heart: | shadedjars | 8m 20s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 40s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 50s | the patch passed | | +1 :green_heart: | compile | 1m 0s | the patch passed | | +1 :green_heart: | javac | 1m 0s | the patch passed | | +1 :green_heart: | shadedjars | 8m 13s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 37s | the patch passed | ||| _ Other Tests _ | | -1 :x: | unit | 150m 11s | hbase-server in the patch failed. | | | | 180m 27s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3970 | | JIRA Issue | HBASE-26447 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux 9a5c6fb24b60 4.15.0-161-generic #169-Ubuntu SMP Fri Oct 15 13:41:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/testReport/ | | Max. process+thread count | 3683 (vs. ulimit of 3) | | modules | C: hbase-server U: hbase-server | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] YutSean commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
YutSean commented on a change in pull request #3964: URL: https://github.com/apache/hbase/pull/3964#discussion_r772866871 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java ## @@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec compressor, InputStream cellBlock } Decompressor poolDecompressor = CodecPool.getDecompressor(compressor); CompressionInputStream cis = compressor.createInputStream(cellBlockStream, poolDecompressor); -ByteBufferOutputStream bbos; try { // TODO: This is ugly. The buffer will be resized on us if we guess wrong. - // TODO: Reuse buffers. - bbos = new ByteBufferOutputStream(osInitialSize); - IOUtils.copy(cis, bbos); - bbos.close(); - return bbos.getByteBuffer(); + if (this.decompressBuff.get() == null) { +this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize)); + } + ByteBufferOutputStream localBbos = this.decompressBuff.get(); + localBbos.clear(); + IOUtils.copy(cis, localBbos); + return localBbos.getByteBuffer(); Review comment: It seems that there is no better solution except returning a copy of the underlying bytebuffer. But copy is not a reuse of buffers. Currently, the threadlocal is not a good idea here... (this should be reconsidered). May I close this PR and the related ticket ? @Apache9 -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] YutSean commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
YutSean commented on a change in pull request #3964: URL: https://github.com/apache/hbase/pull/3964#discussion_r772866871 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java ## @@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec compressor, InputStream cellBlock } Decompressor poolDecompressor = CodecPool.getDecompressor(compressor); CompressionInputStream cis = compressor.createInputStream(cellBlockStream, poolDecompressor); -ByteBufferOutputStream bbos; try { // TODO: This is ugly. The buffer will be resized on us if we guess wrong. - // TODO: Reuse buffers. - bbos = new ByteBufferOutputStream(osInitialSize); - IOUtils.copy(cis, bbos); - bbos.close(); - return bbos.getByteBuffer(); + if (this.decompressBuff.get() == null) { +this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize)); + } + ByteBufferOutputStream localBbos = this.decompressBuff.get(); + localBbos.clear(); + IOUtils.copy(cis, localBbos); + return localBbos.getByteBuffer(); Review comment: It seems that there is no better solution than returning a copy of the underlying bytebuffer. But copy is not a reuse of buffers. Currently, the threadlocal is not a good idea here... (this should be reconsidered). May I close this PR and the related ticket ? @Apache9 -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3969: HBASE-26614 Refactor code related to "dump"ing ZK nodes
Apache-HBase commented on pull request #3969: URL: https://github.com/apache/hbase/pull/3969#issuecomment-998512399 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 5s | 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 _ | | +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 5m 11s | master passed | | +1 :green_heart: | compile | 2m 14s | master passed | | +1 :green_heart: | shadedjars | 9m 10s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 1m 19s | master passed | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 5m 1s | the patch passed | | +1 :green_heart: | compile | 2m 12s | the patch passed | | +1 :green_heart: | javac | 2m 12s | the patch passed | | +1 :green_heart: | shadedjars | 9m 7s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 1m 17s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 0m 54s | hbase-zookeeper in the patch passed. | | +1 :green_heart: | unit | 203m 47s | hbase-server in the patch passed. | | +1 :green_heart: | unit | 1m 8s | hbase-it in the patch passed. | | | | 245m 15s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3969 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux 25f0f92332af 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-11.0.10+9 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/testReport/ | | Max. process+thread count | 2731 (vs. ulimit of 3) | | modules | C: hbase-zookeeper hbase-server hbase-it U: . | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] YutSean commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
YutSean commented on a change in pull request #3964: URL: https://github.com/apache/hbase/pull/3964#discussion_r772854823 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java ## @@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec compressor, InputStream cellBlock } Decompressor poolDecompressor = CodecPool.getDecompressor(compressor); CompressionInputStream cis = compressor.createInputStream(cellBlockStream, poolDecompressor); -ByteBufferOutputStream bbos; try { // TODO: This is ugly. The buffer will be resized on us if we guess wrong. - // TODO: Reuse buffers. - bbos = new ByteBufferOutputStream(osInitialSize); - IOUtils.copy(cis, bbos); - bbos.close(); - return bbos.getByteBuffer(); + if (this.decompressBuff.get() == null) { +this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize)); + } + ByteBufferOutputStream localBbos = this.decompressBuff.get(); + localBbos.clear(); + IOUtils.copy(cis, localBbos); + return localBbos.getByteBuffer(); Review comment: Ah.That is really a problem. Let me think how to deal with this here... -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3952: HBASE-26474 Implement connection-level attributes
Apache9 commented on a change in pull request #3952: URL: https://github.com/apache/hbase/pull/3952#discussion_r772851451 ## File path: hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java ## @@ -90,16 +102,33 @@ public void tearDown() throws IOException { } private SpanData waitSpan(String name) { -Waiter.waitFor(CONF, 1000, - () -> traceRule.getSpans().stream().map(SpanData::getName).anyMatch(s -> s.equals(name))); -return traceRule.getSpans().stream().filter(s -> s.getName().equals(name)).findFirst().get(); +return waitSpan(hasName(name)); + } + + private SpanData waitSpan(Matcher matcher) { +Matcher spanLocator = allOf(matcher, hasEnded()); +try { + Waiter.waitFor(CONF, 1000, new MatcherPredicate<>( +"waiting for span", +() -> traceRule.getSpans(), hasItem(spanLocator))); +} catch (AssertionError e) { Review comment: Why catch AssertionError here? And at lease, let's use LOG to output the message, not System.err? ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocator.java ## @@ -117,18 +123,33 @@ private boolean isMeta(TableName tableName) { } } - private List getRegionName(RegionLocations locs) { -List names = new ArrayList<>(); -for (HRegionLocation loc : locs.getRegionLocations()) { - if (loc != null) { -names.add(loc.getRegion().getRegionNameAsString()); - } + private static List getRegionNames(RegionLocations locs) { +if (locs == null) { Review comment: if (locs == null || locs.getRegionLocations() == null)? -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3964: HBASE-26604 Replace new allocation with ThreadLocal in CellBlockBuilder to reduce GC
Apache9 commented on a change in pull request #3964: URL: https://github.com/apache/hbase/pull/3964#discussion_r772849330 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/CellBlockBuilder.java ## @@ -294,14 +296,15 @@ private ByteBuffer decompress(CompressionCodec compressor, InputStream cellBlock } Decompressor poolDecompressor = CodecPool.getDecompressor(compressor); CompressionInputStream cis = compressor.createInputStream(cellBlockStream, poolDecompressor); -ByteBufferOutputStream bbos; try { // TODO: This is ugly. The buffer will be resized on us if we guess wrong. - // TODO: Reuse buffers. - bbos = new ByteBufferOutputStream(osInitialSize); - IOUtils.copy(cis, bbos); - bbos.close(); - return bbos.getByteBuffer(); + if (this.decompressBuff.get() == null) { +this.decompressBuff.set(new ByteBufferOutputStream(osInitialSize)); + } + ByteBufferOutputStream localBbos = this.decompressBuff.get(); + localBbos.clear(); + IOUtils.copy(cis, localBbos); + return localBbos.getByteBuffer(); Review comment: This will leak the internal ByteBuffer output? What if the upper layer cache this ByteBuffer and pass it to other threads... -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3969: HBASE-26614 Refactor code related to "dump"ing ZK nodes
Apache-HBase commented on pull request #3969: URL: https://github.com/apache/hbase/pull/3969#issuecomment-998490523 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 33s | 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 16s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 4m 43s | master passed | | +1 :green_heart: | compile | 1m 56s | master passed | | +1 :green_heart: | shadedjars | 8m 54s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 1m 16s | master passed | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 4m 22s | the patch passed | | +1 :green_heart: | compile | 1m 56s | the patch passed | | +1 :green_heart: | javac | 1m 56s | the patch passed | | +1 :green_heart: | shadedjars | 9m 44s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 1m 14s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 0m 46s | hbase-zookeeper in the patch passed. | | +1 :green_heart: | unit | 153m 15s | hbase-server in the patch passed. | | +1 :green_heart: | unit | 1m 15s | hbase-it in the patch passed. | | | | 194m 16s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3969 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux 059a03d2cb3f 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/testReport/ | | Max. process+thread count | 3612 (vs. ulimit of 3) | | modules | C: hbase-zookeeper hbase-server hbase-it U: . | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3970: HBASE-26447: Make enable/disable ReplicationPeer and addPeer no-op on retry
Apache-HBase commented on pull request #3970: URL: https://github.com/apache/hbase/pull/3970#issuecomment-998477785 :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 13s | master passed | | +1 :green_heart: | compile | 3m 15s | master passed | | +1 :green_heart: | checkstyle | 1m 8s | master passed | | +1 :green_heart: | spotbugs | 2m 8s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 47s | the patch passed | | +1 :green_heart: | compile | 3m 14s | the patch passed | | +1 :green_heart: | javac | 3m 14s | the patch passed | | -0 :warning: | checkstyle | 1m 1s | hbase-server: 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 | 19m 18s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. | | +1 :green_heart: | spotbugs | 2m 18s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. | | | | 49m 48s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3970 | | JIRA Issue | HBASE-26447 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile | | uname | Linux 4b61790e1ba5 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 / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt | | Max. process+thread count | 96 (vs. ulimit of 3) | | modules | C: hbase-server U: hbase-server | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3970/1/console | | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache-HBase commented on pull request #3968: URL: https://github.com/apache/hbase/pull/3968#issuecomment-998445010 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 26s | 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 11s | master passed | | +1 :green_heart: | compile | 0m 52s | master passed | | +1 :green_heart: | checkstyle | 0m 27s | master passed | | +1 :green_heart: | spotbugs | 0m 47s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 44s | the patch passed | | +1 :green_heart: | compile | 0m 50s | the patch passed | | +1 :green_heart: | javac | 0m 50s | the patch passed | | +1 :green_heart: | checkstyle | 0m 25s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | hadoopcheck | 19m 14s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. | | +1 :green_heart: | spotbugs | 0m 56s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. | | | | 40m 5s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3968 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile | | uname | Linux 33c74ba46508 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 / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | Max. process+thread count | 95 (vs. ulimit of 3) | | modules | C: hbase-common U: hbase-common | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/console | | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache-HBase commented on pull request #3968: URL: https://github.com/apache/hbase/pull/3968#issuecomment-998442698 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 0s | 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 32s | master passed | | +1 :green_heart: | compile | 0m 23s | master passed | | +1 :green_heart: | shadedjars | 9m 2s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 22s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 4m 17s | the patch passed | | +1 :green_heart: | compile | 0m 24s | the patch passed | | +1 :green_heart: | javac | 0m 24s | the patch passed | | +1 :green_heart: | shadedjars | 9m 5s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 21s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 2m 16s | hbase-common in the patch passed. | | | | 32m 45s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3968 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux af958cfb5633 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/testReport/ | | Max. process+thread count | 221 (vs. ulimit of 3) | | modules | C: hbase-common U: hbase-common | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache-HBase commented on pull request #3968: URL: https://github.com/apache/hbase/pull/3968#issuecomment-998442081 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 25s | 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 41s | master passed | | +1 :green_heart: | compile | 0m 26s | master passed | | +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 28s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 4m 26s | 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 | 8m 9s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 25s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 2m 3s | hbase-common in the patch passed. | | | | 30m 48s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3968 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux caa6a9b690a9 4.15.0-161-generic #169-Ubuntu SMP Fri Oct 15 13:41:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-11.0.10+9 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/testReport/ | | Max. process+thread count | 290 (vs. ulimit of 3) | | modules | C: hbase-common U: hbase-common | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/2/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3969: HBASE-26614 Refactor code related to "dump"ing ZK nodes
Apache-HBase commented on pull request #3969: URL: https://github.com/apache/hbase/pull/3969#issuecomment-998440842 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 26s | 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 16s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 4m 10s | master passed | | +1 :green_heart: | compile | 4m 26s | master passed | | +1 :green_heart: | checkstyle | 1m 35s | master passed | | +1 :green_heart: | spotbugs | 3m 9s | master passed | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 3m 51s | the patch passed | | -1 :x: | compile | 0m 22s | hbase-zookeeper in the patch failed. | | -0 :warning: | javac | 0m 22s | hbase-zookeeper in the patch failed. | | -0 :warning: | checkstyle | 0m 13s | hbase-zookeeper: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | hadoopcheck | 19m 23s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. | | +1 :green_heart: | spotbugs | 3m 46s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 40s | The patch does not generate ASF License warnings. | | | | 56m 38s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3969 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile | | uname | Linux 3381f7458252 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/artifact/yetus-general-check/output/patch-compile-hbase-zookeeper.txt | | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/artifact/yetus-general-check/output/patch-compile-hbase-zookeeper.txt | | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-zookeeper.txt | | Max. process+thread count | 96 (vs. ulimit of 3) | | modules | C: hbase-zookeeper hbase-server hbase-it U: . | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3969/1/console | | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] YutSean commented on a change in pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
YutSean commented on a change in pull request #3968: URL: https://github.com/apache/hbase/pull/3968#discussion_r772795251 ## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java ## @@ -640,20 +640,17 @@ public static void incrementIv(byte[] iv) { } public static void incrementIv(byte[] iv, int v) { +// v should be > 0 int length = iv.length; -boolean carry = true; -// TODO: Optimize for v > 1, e.g. 16, 32 -do { - for (int i = 0; i < length; i++) { -if (carry) { - iv[i] = (byte) ((iv[i] + 1) & 0xFF); - carry = 0 == iv[i]; -} else { - break; -} +int sum = 0; +for (int i = 0; i < length; i++) { + if (v <= 0) { +break; } - v--; -} while (v > 0); + sum = v + iv[i]; Review comment: Right, we should care the negative here. Has refined the logic here and added negative UT cases. -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache-HBase commented on pull request #3968: URL: https://github.com/apache/hbase/pull/3968#issuecomment-998428575 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 26s | 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 6s | master passed | | +1 :green_heart: | compile | 0m 51s | master passed | | +1 :green_heart: | checkstyle | 0m 28s | master passed | | +1 :green_heart: | spotbugs | 0m 47s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 48s | the patch passed | | +1 :green_heart: | compile | 0m 50s | the patch passed | | +1 :green_heart: | javac | 0m 50s | the patch passed | | -0 :warning: | checkstyle | 0m 25s | hbase-common: The patch generated 3 new + 52 unchanged - 0 fixed = 55 total (was 52) | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | hadoopcheck | 19m 25s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. | | +1 :green_heart: | spotbugs | 0m 53s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. | | | | 40m 39s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3968 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile | | uname | Linux 36c42ac91fa0 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 / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-common.txt | | Max. process+thread count | 96 (vs. ulimit of 3) | | modules | C: hbase-common U: hbase-common | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/console | | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache-HBase commented on pull request #3968: URL: https://github.com/apache/hbase/pull/3968#issuecomment-998425927 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 59s | Docker mode activated. | | -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck | ||| _ Prechecks _ | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 4m 34s | master passed | | +1 :green_heart: | compile | 0m 23s | master passed | | +1 :green_heart: | shadedjars | 9m 7s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 22s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 4m 17s | the patch passed | | +1 :green_heart: | compile | 0m 25s | the patch passed | | +1 :green_heart: | javac | 0m 25s | the patch passed | | +1 :green_heart: | shadedjars | 9m 8s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 20s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 2m 17s | hbase-common in the patch passed. | | | | 33m 8s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3968 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux 0ee5263d9c15 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/testReport/ | | Max. process+thread count | 221 (vs. ulimit of 3) | | modules | C: hbase-common U: hbase-common | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache-HBase commented on pull request #3968: URL: https://github.com/apache/hbase/pull/3968#issuecomment-998425285 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 24s | 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 44s | master passed | | +1 :green_heart: | compile | 0m 28s | master passed | | +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 27s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 4m 31s | 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 | 8m 21s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 25s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 2m 3s | hbase-common in the patch passed. | | | | 31m 25s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3968 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux db63dc30cda9 4.15.0-161-generic #169-Ubuntu SMP Fri Oct 15 13:41:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-11.0.10+9 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/testReport/ | | Max. process+thread count | 277 (vs. ulimit of 3) | | modules | C: hbase-common U: hbase-common | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3968/1/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Resolved] (HBASE-22953) Supporting Hadoop 3.3.0
[ https://issues.apache.org/jira/browse/HBASE-22953?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Wei-Chiu Chuang resolved HBASE-22953. - Fix Version/s: 2.3.0 3.0.0-alpha-1 Resolution: Fixed > Supporting Hadoop 3.3.0 > --- > > Key: HBASE-22953 > URL: https://issues.apache.org/jira/browse/HBASE-22953 > Project: HBase > Issue Type: Umbrella >Reporter: Wei-Chiu Chuang >Priority: Major > Fix For: 2.3.0, 3.0.0-alpha-1 > > > The Hadoop community has started to discuss a 3.3.0 release. > [http://mail-archives.apache.org/mod_mbox/hadoop-hdfs-dev/201908.mbox/%3CCAD%2B%2BeCneLtC%2BkfxRRKferufnNxhaXXGa0YPaVp%3DEBbc-R5JfqA%40mail.gmail.com%3E] > While still early, it wouldn't hurt to start exploring what's coming in > Hadoop 3.3.0. In particular, there are a bunch of new features that brings in > all sorts of new dependencies. > > I will use this Jira to list things that are related to Hadoop 3.3.0. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[GitHub] [hbase] Apache9 commented on a change in pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
Apache9 commented on a change in pull request #3968: URL: https://github.com/apache/hbase/pull/3968#discussion_r772790543 ## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Encryption.java ## @@ -640,20 +640,17 @@ public static void incrementIv(byte[] iv) { } public static void incrementIv(byte[] iv, int v) { +// v should be > 0 int length = iv.length; -boolean carry = true; -// TODO: Optimize for v > 1, e.g. 16, 32 -do { - for (int i = 0; i < length; i++) { -if (carry) { - iv[i] = (byte) ((iv[i] + 1) & 0xFF); - carry = 0 == iv[i]; -} else { - break; -} +int sum = 0; +for (int i = 0; i < length; i++) { + if (v <= 0) { +break; } - v--; -} while (v > 0); + sum = v + iv[i]; Review comment: What if iv[i] is negative here? In a first thought I think we should consider each byte as unsigned? -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Updated] (HBASE-26613) The logic of the method incrementIV in Encryption class has problem
[ https://issues.apache.org/jira/browse/HBASE-26613?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Yutong Xiao updated HBASE-26613: Description: {code:java} public static void incrementIv(byte[] iv, int v) { int length = iv.length; boolean carry = true; // TODO: Optimize for v > 1, e.g. 16, 32 do { for (int i = 0; i < length; i++) { if (carry) { iv[i] = (byte) ((iv[i] + 1) & 0xFF); carry = 0 == iv[i]; } else { break; } } v--; } while (v > 0); } {code} The carry flag is outside the while loop and the inner loop check the carry value is true, so that when it was set to false, it will make the rest iterations busy spinning. And the logic becomes whatever the v is, the IV will be incremented by (1 or 2). As the description in todo, this function should support increment value > 1. So that this is not what we expect I think. was: {code:java} public static void incrementIv(byte[] iv, int v) { int length = iv.length; boolean carry = true; // TODO: Optimize for v > 1, e.g. 16, 32 do { for (int i = 0; i < length; i++) { if (carry) { iv[i] = (byte) ((iv[i] + 1) & 0xFF); carry = 0 == iv[i]; } else { break; } } v--; } while (v > 0); } {code} The carry flag is outside the while loop and the inner loop check the carry value is true, so that the when it was set to false, it will spinning in the last iteration. And the logic becomes whatever the v is, the IV will be incremented by (1 or 2). As the description in todo, this function should support increment value > 1. So that this is not what we expect I think. > The logic of the method incrementIV in Encryption class has problem > --- > > Key: HBASE-26613 > URL: https://issues.apache.org/jira/browse/HBASE-26613 > Project: HBase > Issue Type: Bug >Reporter: Yutong Xiao >Assignee: Yutong Xiao >Priority: Major > > {code:java} > public static void incrementIv(byte[] iv, int v) { > int length = iv.length; > boolean carry = true; > // TODO: Optimize for v > 1, e.g. 16, 32 > do { > for (int i = 0; i < length; i++) { > if (carry) { > iv[i] = (byte) ((iv[i] + 1) & 0xFF); > carry = 0 == iv[i]; > } else { > break; > } > } > v--; > } while (v > 0); > } > {code} > The carry flag is outside the while loop and the inner loop check the carry > value is true, so that when it was set to false, it will make the rest > iterations busy spinning. > And the logic becomes whatever the v is, the IV will be incremented by (1 or > 2). As the description in todo, this function should support increment value > > 1. So that this is not what we expect I think. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[GitHub] [hbase] Apache9 commented on pull request #3913: HBASE-26536 Checkstyle: permit single-line if-then statements
Apache9 commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-998421074 Yes, spotless should be another issue. And on the 'one line if', for me, as said above, the old fashion is to not have '{}' at all, in our code base this is common. But now, it is not recommand to not have '{}' for a code block, especially for the 'if' 'else' statement, so for me I prefer we just use the standard form ```java if (foo) { } else { } ``` No matter what is inside the code block. I do not think it introduces any difficulty on reading the code? And what concerns me more is changing a style will introduce more meaning less code changes in the future. If we are a new project, in fact I'm just fine with what ever style guidelines. I do not have very strong opinion on the code style, though I have some thoughts, but usually I will not call a veto. But we are a project which is more than 10 years old. We have already done lots of format work to format the 'if else' block to the suggested style, now if we change the style guide, we need a reformat again, which I do not think could gain much value. So I suggest we just open a discuss thread on the mailing list, to see what the community think about this. If there are more people think the one line if is good thing, then maybe it worth a try. If most people are just neutral, I would say let's just keep the old way. 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (HBASE-26613) The logic of the method incrementIV in Encryption class has problem
[ https://issues.apache.org/jira/browse/HBASE-26613?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17462935#comment-17462935 ] Yutong Xiao commented on HBASE-26613: - Yes, but for v > 1, the complexity is O(m*n), where m is iv.length, n is v. I have created an MR which optimized the condition that v > 1 and added a UT for this method (currently, we do not have such a UT for this util function) > The logic of the method incrementIV in Encryption class has problem > --- > > Key: HBASE-26613 > URL: https://issues.apache.org/jira/browse/HBASE-26613 > Project: HBase > Issue Type: Bug >Reporter: Yutong Xiao >Assignee: Yutong Xiao >Priority: Major > > {code:java} > public static void incrementIv(byte[] iv, int v) { > int length = iv.length; > boolean carry = true; > // TODO: Optimize for v > 1, e.g. 16, 32 > do { > for (int i = 0; i < length; i++) { > if (carry) { > iv[i] = (byte) ((iv[i] + 1) & 0xFF); > carry = 0 == iv[i]; > } else { > break; > } > } > v--; > } while (v > 0); > } > {code} > The carry flag is outside the while loop and the inner loop check the carry > value is true, so that the when it was set to false, it will spinning in the > last iteration. And the logic becomes whatever the v is, the IV will be > incremented by (1 or 2). As the description in todo, this function should > support increment value > 1. So that this is not what we expect I think. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[GitHub] [hbase] ndimiduk opened a new pull request #3969: HBASE-26614 Refactor code related to "dump"ing ZK nodes
ndimiduk opened a new pull request #3969: URL: https://github.com/apache/hbase/pull/3969 The code starting at `ZKUtil.dump(ZKWatcher)` is a small mess – it has cyclic dependencies woven through itself, `ZKWatcher` and `RecoverableZooKeeper`. It also initializes a static variable in `ZKUtil` through the factory for `RecoverableZooKeeper` instances. Let's decouple and clean it up. -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Created] (HBASE-26614) Refactor code related to "dump"ing ZK nodes
Nick Dimiduk created HBASE-26614: Summary: Refactor code related to "dump"ing ZK nodes Key: HBASE-26614 URL: https://issues.apache.org/jira/browse/HBASE-26614 Project: HBase Issue Type: Task Components: Zookeeper Reporter: Nick Dimiduk Assignee: Nick Dimiduk The code starting at {{ZKUtil.dump(ZKWatcher)}} is a small mess -- it has cyclic dependencies woven through itself, {{ZKWatcher}} and {{RecoverableZooKeeper}}. It also initializes a static variable in {{ZKUtil}} through the factory for {{RecoverableZooKeeper}} instances. Let's decouple and clean it up. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[GitHub] [hbase] YutSean opened a new pull request #3968: HBASE-26613 The logic of the method incrementIV in Encryption class has problem
YutSean opened a new pull request #3968: URL: https://github.com/apache/hbase/pull/3968 https://issues.apache.org/jira/browse/HBASE-26613 -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (HBASE-26613) The logic of the method incrementIV in Encryption class has problem
[ https://issues.apache.org/jira/browse/HBASE-26613?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17462926#comment-17462926 ] Duo Zhang commented on HBASE-26613: --- I think we need to reset carry to false at the beginning ofthe do while loop? > The logic of the method incrementIV in Encryption class has problem > --- > > Key: HBASE-26613 > URL: https://issues.apache.org/jira/browse/HBASE-26613 > Project: HBase > Issue Type: Bug >Reporter: Yutong Xiao >Assignee: Yutong Xiao >Priority: Major > > {code:java} > public static void incrementIv(byte[] iv, int v) { > int length = iv.length; > boolean carry = true; > // TODO: Optimize for v > 1, e.g. 16, 32 > do { > for (int i = 0; i < length; i++) { > if (carry) { > iv[i] = (byte) ((iv[i] + 1) & 0xFF); > carry = 0 == iv[i]; > } else { > break; > } > } > v--; > } while (v > 0); > } > {code} > The carry flag is outside the while loop and the inner loop check the carry > value is true, so that the when it was set to false, it will spinning in the > last iteration. And the logic becomes whatever the v is, the IV will be > incremented by (1 or 2). As the description in todo, this function should > support increment value > 1. So that this is not what we expect I think. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[jira] [Created] (HBASE-26613) The logic of the method incrementIV in Encryption class has problem
Yutong Xiao created HBASE-26613: --- Summary: The logic of the method incrementIV in Encryption class has problem Key: HBASE-26613 URL: https://issues.apache.org/jira/browse/HBASE-26613 Project: HBase Issue Type: Bug Reporter: Yutong Xiao Assignee: Yutong Xiao {code:java} public static void incrementIv(byte[] iv, int v) { int length = iv.length; boolean carry = true; // TODO: Optimize for v > 1, e.g. 16, 32 do { for (int i = 0; i < length; i++) { if (carry) { iv[i] = (byte) ((iv[i] + 1) & 0xFF); carry = 0 == iv[i]; } else { break; } } v--; } while (v > 0); } {code} The carry flag is outside the while loop and the inner loop check the carry value is true, so that the when it was set to false, it will spinning in the last iteration. And the logic becomes whatever the v is, the IV will be incremented by (1 or 2). As the description in todo, this function should support increment value > 1. So that this is not what we expect I think. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[jira] [Created] (HBASE-26612) Adjust log level when looking for .filelist/{f1,f2}
Josh Elser created HBASE-26612: -- Summary: Adjust log level when looking for .filelist/{f1,f2} Key: HBASE-26612 URL: https://issues.apache.org/jira/browse/HBASE-26612 Project: HBase Issue Type: Sub-task Reporter: Josh Elser Assignee: Josh Elser Currently, we get a really big exception in the RegionServer log under normal assignment conditions when we are currently using .filelist/f2 as the tracker file. We should move this to debug/trace to avoid making operators think there is a problem when there isn't one. -- This message was sent by Atlassian Jira (v8.20.1#820001)
[GitHub] [hbase] caroliney14 closed pull request #3606: HBASE-25815 RSGroupBasedLoadBalancer online status never updates after being set to true for the first time
caroliney14 closed pull request #3606: URL: https://github.com/apache/hbase/pull/3606 -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Updated] (HBASE-26536) Checkstyle: permit single-line if-then statements
[ https://issues.apache.org/jira/browse/HBASE-26536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Nick Dimiduk updated HBASE-26536: - Summary: Checkstyle: permit single-line if-then statements (was: Tweak checkstyle LeftCurly config to "nlow") > Checkstyle: permit single-line if-then statements > - > > Key: HBASE-26536 > URL: https://issues.apache.org/jira/browse/HBASE-26536 > Project: HBase > Issue Type: Task >Reporter: Nick Dimiduk >Assignee: Nick Dimiduk >Priority: Minor > > Change the > [LeftCurly|https://checkstyle.sourceforge.io/config_blocks.html#LeftCurly] > module > [LeftCurlyOption|https://checkstyle.sourceforge.io/property_types.html#LeftCurlyOption] > from the default {{eol}} to {{nlow}}. This will permit single-line code > blocks like > {noformat} > if (foo == null) { return null; } > {noformat} -- This message was sent by Atlassian Jira (v8.20.1#820001)
[GitHub] [hbase] ndimiduk commented on a change in pull request #3951: HBASE-26473 Introduce `db.hbase.container_operations` span attribute
ndimiduk commented on a change in pull request #3951: URL: https://github.com/apache/hbase/pull/3951#discussion_r772563706 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/trace/TableOperationSpanBuilder.java ## @@ -82,6 +89,72 @@ public TableOperationSpanBuilder setOperation(final Operation operation) { return this; } + // `setContainerOperations` perform a recursive descent expansion of all the operations + // contained within the provided "batch" object. + + public TableOperationSpanBuilder setContainerOperations(final RowMutations mutations) { +final Operation[] ops = mutations.getMutations() + .stream() + .flatMap(row -> Stream.concat(Stream.of(valueFrom(row)), unpackRowOperations(row).stream())) + .toArray(Operation[]::new); +return setContainerOperations(ops); + } + + public TableOperationSpanBuilder setContainerOperations(final Row row) { +final Operation[] ops = + Stream.concat(Stream.of(valueFrom(row)), unpackRowOperations(row).stream()) + .toArray(Operation[]::new); +return setContainerOperations(ops); + } + + public TableOperationSpanBuilder setContainerOperations( +final Collection operations + ) { +final Operation[] ops = operations.stream() + .flatMap(row -> Stream.concat(Stream.of(valueFrom(row)), unpackRowOperations(row).stream())) + .toArray(Operation[]::new); +return setContainerOperations(ops); + } + + private static Set unpackRowOperations(final Row row) { +final Set ops = new HashSet<>(); +if (row instanceof CheckAndMutate) { + final CheckAndMutate cam = (CheckAndMutate) row; + ops.addAll(unpackRowOperations(cam)); +} +if (row instanceof RowMutations) { + final RowMutations mutations = (RowMutations) row; + ops.addAll(unpackRowOperations(mutations)); +} +return ops; + } + + private static Set unpackRowOperations(final CheckAndMutate cam) { +final Set ops = new HashSet<>(); +final Operation op = valueFrom(cam.getAction()); +switch (op) { + case BATCH: + case CHECK_AND_MUTATE: +ops.addAll(unpackRowOperations(cam.getAction())); +break; + default: +ops.add(op); +} +return ops; + } + + public TableOperationSpanBuilder setContainerOperations( Review comment: That's not a bad idea. I don't now how the telemetry services will handle these kinds of fields. -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #3913: HBASE-26536 Tweak checkstyle LeftCurly config to "nlow"
ndimiduk commented on pull request #3913: URL: https://github.com/apache/hbase/pull/3913#issuecomment-998145153 @Apache9 if you'd like to try out spotless, please open a separate issue for that. I think it looks promising. In the mean time, what's wrong with the single line scoped block as I propose on this change? I find it this change makes code easier to read by reducing the space needed to represent a small, complete expression. It also honors the need for explicit block markers, something that the bare then-statement does not provide. Is there anything problematic about this style that I propose? 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3929: HBASE-26551 Add FastPath feature to HBase RWQueueRpcExecutor
Apache-HBase commented on pull request #3929: URL: https://github.com/apache/hbase/pull/3929#issuecomment-998087570 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 59s | Docker mode activated. | | -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck | ||| _ Prechecks _ | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 5m 17s | master passed | | +1 :green_heart: | compile | 1m 18s | master passed | | +1 :green_heart: | shadedjars | 9m 10s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 48s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 5m 9s | the patch passed | | +1 :green_heart: | compile | 1m 19s | the patch passed | | +1 :green_heart: | javac | 1m 19s | the patch passed | | +1 :green_heart: | shadedjars | 9m 10s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 42s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 205m 10s | hbase-server in the patch passed. | | | | 240m 45s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3929 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux 974913165de4 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-11.0.10+9 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/testReport/ | | Max. process+thread count | 2529 (vs. ulimit of 3) | | modules | C: hbase-server U: hbase-server | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3929: HBASE-26551 Add FastPath feature to HBase RWQueueRpcExecutor
Apache-HBase commented on pull request #3929: URL: https://github.com/apache/hbase/pull/3929#issuecomment-998035888 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 59s | 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 8s | master passed | | +1 :green_heart: | compile | 1m 4s | master passed | | +1 :green_heart: | shadedjars | 8m 22s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 40s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 50s | the patch passed | | +1 :green_heart: | compile | 1m 3s | the patch passed | | +1 :green_heart: | javac | 1m 3s | the patch passed | | +1 :green_heart: | shadedjars | 8m 17s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 36s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 153m 18s | hbase-server in the patch passed. | | | | 184m 28s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3929 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux f9f2c7823294 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 / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/testReport/ | | Max. process+thread count | 3577 (vs. ulimit of 3) | | modules | C: hbase-server U: hbase-server | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/console | | versions | git=2.17.1 maven=3.6.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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3929: HBASE-26551 Add FastPath feature to HBase RWQueueRpcExecutor
Apache-HBase commented on pull request #3929: URL: https://github.com/apache/hbase/pull/3929#issuecomment-997922417 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 2s | 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 12s | master passed | | +1 :green_heart: | compile | 3m 11s | master passed | | +1 :green_heart: | checkstyle | 1m 3s | master passed | | +1 :green_heart: | spotbugs | 2m 5s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 52s | the patch passed | | +1 :green_heart: | compile | 3m 13s | the patch passed | | +1 :green_heart: | javac | 3m 13s | the patch passed | | -0 :warning: | checkstyle | 1m 4s | hbase-server: The patch generated 5 new + 20 unchanged - 1 fixed = 25 total (was 21) | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | hadoopcheck | 19m 44s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. | | +1 :green_heart: | spotbugs | 2m 16s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. | | | | 49m 55s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/3929 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile | | uname | Linux 23f0d706ff92 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 314e924e96 | | Default Java | AdoptOpenJDK-1.8.0_282-b08 | | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt | | Max. process+thread count | 96 (vs. ulimit of 3) | | modules | C: hbase-server U: hbase-server | | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3929/5/console | | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 | | 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] YutSean commented on pull request #3929: HBASE-26551 Add FastPath feature to HBase RWQueueRpcExecutor
YutSean commented on pull request #3929: URL: https://github.com/apache/hbase/pull/3929#issuecomment-997885255 Refined the variable name in the latest commit. I think this is more readable then. -- 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-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] sunhelly commented on a change in pull request #3800: HBASE-26347 Support detect and exclude slow DNs in fan-out of WAL
sunhelly commented on a change in pull request #3800: URL: https://github.com/apache/hbase/pull/3800#discussion_r772230583 ## File path: hbase-asyncfs/src/main/java/org/apache/hadoop/hbase/io/asyncfs/monitor/StreamSlowMonitor.java ## @@ -0,0 +1,195 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.io.asyncfs.monitor; + +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.DEFAULT_WAL_EXCLUDE_DATANODE_TTL; +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.DEFAULT_WAL_MAX_EXCLUDE_SLOW_DATANODE_COUNT; +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.WAL_EXCLUDE_DATANODE_TTL_KEY; +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.WAL_MAX_EXCLUDE_SLOW_DATANODE_COUNT_KEY; + +import java.util.Deque; +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.TimeUnit; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.conf.ConfigurationObserver; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.hadoop.hdfs.protocol.DatanodeInfo; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.hbase.thirdparty.com.google.common.cache.Cache; +import org.apache.hbase.thirdparty.com.google.common.cache.CacheBuilder; + +/** + * Class for monitor the wal file flush performance. + * Each active wal file has a StreamSlowMonitor. + */ +@InterfaceAudience.Private +public class StreamSlowMonitor implements ConfigurationObserver { + private static final Logger LOG = LoggerFactory.getLogger(StreamSlowMonitor.class); + + /** + * Configure for the min count for a datanode detected slow. + * If a datanode is detected slow times up to this count, then it will be added to the exclude + * datanode cache by {@link ExcludeDatanodeManager#tryAddExcludeDN(DatanodeInfo, String)} + * of this regionsever. + */ + private static final String WAL_SLOW_DETECT_MIN_COUNT_KEY = +"hbase.regionserver.async.wal.min.slow.detect.count"; + private static final int DEFAULT_WAL_SLOW_DETECT_MIN_COUNT = 3; + + /** + * Configure for the TTL of the data that a datanode detected slow. + */ + private static final String WAL_SLOW_DETECT_DATA_TTL_KEY = +"hbase.regionserver.async.wal.slow.detect.data.ttl.ms"; + private static final long DEFAULT_WAL_SLOW_DETECT_DATA_TTL = 10 * 60 * 1000; // 10min in ms + + /** + * Configure for the slow packet process time, a duration from send to ACK. + */ + private static final String DATANODE_SLOW_PACKET_PROCESS_TIME_KEY = +"hbase.regionserver.async.wal.datanode.slow.packet.process.time.millis"; + private static final long DEFAULT_DATANODE_SLOW_PACKET_PROCESS_TIME = 6000; //6s in ms + + /** + * Configure for the packet flush speed. + */ + private static final String DATANODE_SLOW_PACKET_FLUSH_SPEED_KEY = +"hbase.regionserver.async.wal.datanode.slow.packet.flush.speed.kbs"; + private static final double DEFAULT_DATANODE_SLOW_PACKET_FLUSH_SPEED = 0.1; + + private final String name; + // this is a map of datanodeInfo->queued slow PacketAckData + private final Cache> datanodeSlowDataQueue; + private final ExcludeDatanodeManager excludeDatanodeManager; + + private int minSlowDetectCount; + private long slowDataTtl; + private long slowPacketAckMillis; + private double slowPacketAckSpeed; + + public StreamSlowMonitor(Configuration conf, String name, + ExcludeDatanodeManager excludeDatanodeManager) { +setConf(conf); +this.name = name; +this.excludeDatanodeManager = excludeDatanodeManager; +this.datanodeSlowDataQueue = CacheBuilder.newBuilder() + .initialCapacity(conf.getInt(WAL_MAX_EXCLUDE_SLOW_DATANODE_COUNT_KEY, +DEFAULT_WAL_MAX_EXCLUDE_SLOW_DATANODE_COUNT)) + .expireAfterWrite(conf.getLong(WAL_EXCLUDE_DATANODE_TTL_KEY, +DEFAULT_WAL_EXCLUDE_DATANODE_TTL), TimeUnit.HOURS) + .build(); +LOG.info("New stream slow monitor {}", this.name); + } + + public static StreamSlowMonitor create(Configuration conf, String name) { +return new
[GitHub] [hbase] sunhelly commented on a change in pull request #3800: HBASE-26347 Support detect and exclude slow DNs in fan-out of WAL
sunhelly commented on a change in pull request #3800: URL: https://github.com/apache/hbase/pull/3800#discussion_r772227119 ## File path: hbase-asyncfs/src/main/java/org/apache/hadoop/hbase/io/asyncfs/monitor/StreamSlowMonitor.java ## @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.io.asyncfs.monitor; + +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.DEFAULT_WAL_EXCLUDE_DATANODE_TTL; +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.DEFAULT_WAL_MAX_EXCLUDE_SLOW_DATANODE_COUNT; +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.WAL_EXCLUDE_DATANODE_TTL_KEY; +import static org.apache.hadoop.hbase.io.asyncfs.monitor.ExcludeDatanodeManager.WAL_MAX_EXCLUDE_SLOW_DATANODE_COUNT_KEY; + +import java.util.Deque; +import java.util.concurrent.ConcurrentLinkedDeque; +import java.util.concurrent.TimeUnit; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.conf.ConfigurationObserver; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.hadoop.hdfs.protocol.DatanodeInfo; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.hbase.thirdparty.com.google.common.cache.CacheBuilder; +import org.apache.hbase.thirdparty.com.google.common.cache.CacheLoader; +import org.apache.hbase.thirdparty.com.google.common.cache.LoadingCache; + +/** + * Class for monitor the wal file flush performance. + * Each active wal file has a StreamSlowMonitor. + */ +@InterfaceAudience.Private +public class StreamSlowMonitor implements ConfigurationObserver { + private static final Logger LOG = LoggerFactory.getLogger(StreamSlowMonitor.class); + + /** + * Configure for the min count for a datanode detected slow. + * If a datanode is detected slow times up to this count, then it will be added to the exclude + * datanode cache by {@link ExcludeDatanodeManager#tryAddExcludeDN(DatanodeInfo, String)} + * of this regionsever. + */ + private static final String WAL_SLOW_DETECT_MIN_COUNT_KEY = +"hbase.regionserver.async.wal.min.slow.detect.count"; + private static final int DEFAULT_WAL_SLOW_DETECT_MIN_COUNT = 3; + + /** + * Configure for the TTL of the data that a datanode detected slow. + */ + private static final String WAL_SLOW_DETECT_DATA_TTL_KEY = +"hbase.regionserver.async.wal.slow.detect.data.ttl.ms"; + private static final long DEFAULT_WAL_SLOW_DETECT_DATA_TTL = 10 * 60 * 1000; // 10min in ms + + /** + * Configure for the slow packet process time, a duration from send to ACK. + * It is preferred that this value should not be less than 1s. + */ + private static final String DATANODE_SLOW_PACKET_PROCESS_TIME_KEY = +"hbase.regionserver.async.wal.datanode.slow.packet.process.time.millis"; + private static final long DEFAULT_DATANODE_SLOW_PACKET_PROCESS_TIME = 6000; //6s in ms + + /** + * Configure for the packet flush speed. + * e.g. If the configured slow slow packet process time is larger than 1s, then here 0.1kbs means + * 100B should be processed in less than 1s. + * If the configured slow slow packet process time is larger than 10s, then here 0.1kbs means + * 1KB should be processed in less than 10s. + */ + private static final String DATANODE_SLOW_PACKET_FLUSH_SPEED_KEY = +"hbase.regionserver.async.wal.datanode.slow.packet.flush.speed.kbs"; + private static final double DEFAULT_DATANODE_SLOW_PACKET_FLUSH_SPEED = 0.1; + + private final String name; + // this is a map of datanodeInfo->queued slow PacketAckData + private final LoadingCache> datanodeSlowDataQueue; + private final ExcludeDatanodeManager excludeDatanodeManager; + + private int minSlowDetectCount; + private long slowDataTtl; + private long slowPacketAckMs; + private double slowPacketAckSpeed; + + public StreamSlowMonitor(Configuration conf, String name, + ExcludeDatanodeManager excludeDatanodeManager) { +setConf(conf); +this.name = name; +this.excludeDatanodeManager = excludeDatanodeManager; +this.datanodeSlowDataQueue =