You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2021/07/15 21:18:20 UTC
[GitHub] [hbase] gkanade opened a new pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
gkanade opened a new pull request #3495:
URL: https://github.com/apache/hbase/pull/3495
…SYNC_WAL is used with multi operation
--
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
[GitHub] [hbase] apurtell commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672524407
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
I see, it's fine, if I misread the code.
I am guilty myself of employing this trick, where a high order bit of a counter is overloaded to be a flag. I feel it leads to less maintainable code than the alternative, but if it has to be done it has to be done. The issue is someone less familiar with the code than the current author and reviewers might forget to mask when reading or updating.
> Maybe we could introduce a separated class for this logic to make the code in ServerCall cleaner, and add clear document for the new class about the logic.
This is a good suggestion.
--
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
[GitHub] [hbase] huaxiangsun commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672393896
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
I did some test to show the binary bits are set correctly.
--
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
[GitHub] [hbase] apurtell commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r671519156
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
Review comment:
I am very glad to see we are removing this magic constant, it was/is a code smell.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883692030
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 | 1m 0s | master passed |
| +1 :green_heart: | shadedjars | 8m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 56s | root in the patch failed. |
| -1 :x: | compile | 0m 48s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 48s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 6m 12s | patch has 20 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 48s | hbase-server in the patch failed. |
| | | 25m 44s | |
| 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-3495/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux e3775c5a6bc7 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/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-3495/3/testReport/ |
| Max. process+thread count | 88 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] gkanade commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
gkanade commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672429950
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
Right - as @Apache9 mentions above, the check whether rpc ref is false AND wal reference count = 0 needs to happen atomically and make sure the ByteBuffer is released exactly once. Thus single atomicinteger mechanism provides synchronized way of dealing with this @anoopsjohn FYI
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-884021667
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 27s | 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 46s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 8m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 46s | 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 14s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 178m 12s | hbase-server in the patch failed. |
| | | 209m 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-3495/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0903af951f98 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/5/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-3495/5/testReport/ |
| Max. process+thread count | 4028 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-881081450
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | 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 57s | master passed |
| +1 :green_heart: | compile | 3m 9s | master passed |
| +1 :green_heart: | checkstyle | 1m 3s | master passed |
| +1 :green_heart: | spotbugs | 2m 7s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 36s | the patch passed |
| +1 :green_heart: | compile | 3m 8s | the patch passed |
| +1 :green_heart: | javac | 3m 8s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 0s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 28s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 13s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | The patch does not generate ASF License warnings. |
| | | 46m 50s | |
| 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-3495/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 76c3b9a42d48 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 0836695459 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] gkanade commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
gkanade commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883648732
Thx all for the reviews, I believe I have addressed all. Will create separate jira for the improvement related to separating out class
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-881126616
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 8m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 154m 21s | hbase-server in the patch failed. |
| | | 184m 42s | |
| 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-3495/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4106af76dd27 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 / 0836695459 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-3495/1/testReport/ |
| Max. process+thread count | 4547 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883927414
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 18s | 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 1s | master passed |
| +1 :green_heart: | compile | 3m 23s | master passed |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | spotbugs | 2m 11s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 4s | the patch passed |
| +1 :green_heart: | compile | 3m 19s | the patch passed |
| +1 :green_heart: | javac | 3m 19s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 9s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 30s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 23s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 52m 1s | |
| 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-3495/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux a537a3bdfaee 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] apurtell commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672524407
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
I see, it's fine, if I misread the code.
I am guilty myself of employing this trick, where a high order bit of a counter is overloaded to be a flag. I feel it leads to less maintainable code than the alternative, but if it has to be done it has to be done. The issue is someone less familiar with the code than the current author and reviewers might forget to mask when reading or updating.
> Maybe we could introduce a separated class for this logic to make the code in ServerCall cleaner, and add clear document for the new class about the logic.
This is a good suggestion.
--
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
[GitHub] [hbase] Apache9 commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672307084
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
No...
The rpc release and wal release could happen at the same time. We need a single atomic operation to avoid race here.
Maybe we could introduce a separated class for this logic to make the code in ServerCall cleaner, and add clear document for the new class about the logic.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
I suggest we add some comments here, to say that decrementAndGet still work even if the highest bit is 1, which means we are decrementing a negative value. I was confused at first but then I realized that the arithmetic is the same for positive and negative 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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883663542
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 36s | 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 50s | master passed |
| +1 :green_heart: | compile | 1m 14s | master passed |
| +1 :green_heart: | shadedjars | 8m 24s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 27s | root in the patch failed. |
| -1 :x: | compile | 0m 55s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 55s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 6m 28s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 54s | hbase-server in the patch failed. |
| | | 29m 22s | |
| 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-3495/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 736b5a86c8e0 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/testReport/ |
| Max. process+thread count | 97 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883693978
:broken_heart: **-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 | 4m 14s | master passed |
| +1 :green_heart: | compile | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 8m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 29s | root in the patch failed. |
| -1 :x: | compile | 0m 56s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 56s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 6m 29s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 55s | hbase-server in the patch failed. |
| | | 28m 1s | |
| 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-3495/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7b9adc40a1c9 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/testReport/ |
| Max. process+thread count | 101 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883661664
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | 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 25s | master passed |
| +1 :green_heart: | compile | 3m 37s | master passed |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | spotbugs | 2m 20s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 3s | root in the patch failed. |
| -1 :x: | compile | 1m 59s | hbase-server in the patch failed. |
| -0 :warning: | javac | 1m 59s | hbase-server in the patch failed. |
| +1 :green_heart: | checkstyle | 0m 59s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | hadoopcheck | 2m 8s | The patch causes 20 errors with Hadoop v3.1.2. |
| -1 :x: | hadoopcheck | 4m 24s | The patch causes 20 errors with Hadoop v3.2.1. |
| -1 :x: | hadoopcheck | 6m 39s | The patch causes 20 errors with Hadoop v3.3.0. |
| -1 :x: | spotbugs | 0m 32s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 26m 21s | |
| 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-3495/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux e0eefb9f2a8b 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-javac-3.3.0.txt |
| spotbugs | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883690647
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 40s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | compile | 3m 26s | master passed |
| +1 :green_heart: | checkstyle | 1m 2s | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 54s | root in the patch failed. |
| -1 :x: | compile | 1m 50s | hbase-server in the patch failed. |
| -0 :warning: | javac | 1m 50s | hbase-server in the patch failed. |
| +1 :green_heart: | checkstyle | 1m 0s | the patch passed |
| +1 :green_heart: | whitespace | 0m 1s | The patch has no whitespace issues. |
| -1 :x: | hadoopcheck | 2m 7s | The patch causes 20 errors with Hadoop v3.1.2. |
| -1 :x: | hadoopcheck | 4m 16s | The patch causes 20 errors with Hadoop v3.2.1. |
| -1 :x: | hadoopcheck | 6m 24s | The patch causes 20 errors with Hadoop v3.3.0. |
| -1 :x: | spotbugs | 0m 31s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 24m 52s | |
| 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-3495/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 6348dd242ec1 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-javac-3.3.0.txt |
| spotbugs | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/3/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883880029
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 52s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 41s | master passed |
| +1 :green_heart: | compile | 1m 41s | master passed |
| +1 :green_heart: | shadedjars | 10m 52s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 50s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 3m 2s | root in the patch failed. |
| -1 :x: | compile | 1m 4s | hbase-server in the patch failed. |
| -0 :warning: | javac | 1m 4s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 8m 25s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 52s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 1m 5s | hbase-server in the patch failed. |
| | | 36m 53s | |
| 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-3495/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 055ac9d4b950 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/testReport/ |
| Max. process+thread count | 97 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn merged pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
anoopsjohn merged pull request #3495:
URL: https://github.com/apache/hbase/pull/3495
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883875683
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | 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 38s | master passed |
| +1 :green_heart: | compile | 0m 58s | master passed |
| +1 :green_heart: | shadedjars | 8m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 58s | root in the patch failed. |
| -1 :x: | compile | 0m 47s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 47s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 6m 13s | patch has 20 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 46s | hbase-server in the patch failed. |
| | | 25m 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-3495/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9284dfeac61f 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/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-3495/4/testReport/ |
| Max. process+thread count | 88 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672307084
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
No...
The rpc release and wal release could happen at the same time. We need a single atomic operation to avoid race here.
Maybe we could introduce a separated class for this logic to make the code in ServerCall cleaner, and add clear document for the new class about the logic.
--
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
[GitHub] [hbase] anoopsjohn commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883008477
+1 to address the new class (generic) in a followup issue. Would like to get this committed and get this in for 2.3.6 @saintstack pinged for issues in dev@ mail chain. Multiple times this bug been raised.
--
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
[GitHub] [hbase] huaxiangsun commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672410979
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
Like this idea. This new class needs to be generic as not only in this rpc handler/wal subsystem needs to coordinate. The replication path also needs similar coordination.
--
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
[GitHub] [hbase] apurtell commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672524407
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
I see, it's fine, if I misread the code.
I am guilty myself of employing this trick, where a high order bit of a counter is overloaded to be a flag. I feel it leads to less maintainable code than the alternative, but if it has to be done it has to be done. The issue is someone less familiar with the code than the current author and reviewers might forget to mask when reading or updating.
> Maybe we could introduce a separated class for this logic to make the code in ServerCall cleaner, and add clear document for the new class about the logic.
This is a good suggestion.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-881151095
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 54s | 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 | 6m 0s | master passed |
| +1 :green_heart: | compile | 1m 37s | master passed |
| +1 :green_heart: | shadedjars | 9m 44s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 45s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 32s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 10m 58s | 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 | 213m 52s | hbase-server in the patch passed. |
| | | 254m 53s | |
| 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-3495/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f51704fd70b0 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 / 0836695459 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/1/testReport/ |
| Max. process+thread count | 3156 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] gkanade commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
gkanade commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672429950
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
Right - as @Apache9 mentions above, the check whether rpc ref is false AND wal reference count = 0 needs to happen atomically and make sure the ByteBuffer is released exactly once. Thus single atomicinteger mechanism provides synchronized way of dealing with this @anoopsjohn FYI
--
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
[GitHub] [hbase] anoopsjohn commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883008477
+1 to address the new class (generic) in a followup issue. Would like to get this committed and get this in for 2.3.6 @saintstack pinged for issues in dev@ mail chain. Multiple times this bug been raised.
--
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
[GitHub] [hbase] gkanade commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
gkanade commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r673425345
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
Thx for the suggestion; added comments. Plz check
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883662230
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 36s | 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 7s | master passed |
| +1 :green_heart: | compile | 1m 2s | master passed |
| +1 :green_heart: | shadedjars | 8m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 0s | root in the patch failed. |
| -1 :x: | compile | 0m 49s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 49s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 6m 17s | patch has 20 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 48s | hbase-server in the patch failed. |
| | | 27m 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-3495/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7bdccac8d04f 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/2/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-3495/2/testReport/ |
| Max. process+thread count | 88 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] huaxiangsun commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672393896
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
I did some test to show the binary bits are set correctly.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
Like this idea. This new class needs to be generic as not only in this rpc handler/wal subsystem needs to coordinate. The replication path also needs similar coordination.
--
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
[GitHub] [hbase] Apache9 commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672312582
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
I suggest we add some comments here, to say that decrementAndGet still work even if the highest bit is 1, which means we are decrementing a negative value. I was confused at first but then I realized that the arithmetic is the same for positive and negative 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
[GitHub] [hbase] huaxiangsun commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672393896
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
I did some test to show the binary bits are set correctly.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
Like this idea. This new class needs to be generic as not only in this rpc handler/wal subsystem needs to coordinate. The replication path also needs similar coordination.
--
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
[GitHub] [hbase] gkanade commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
gkanade commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672429950
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
Right - as @Apache9 mentions above, the check whether rpc ref is false AND wal reference count = 0 needs to happen atomically and make sure the ByteBuffer is released exactly once. Thus single atomicinteger mechanism provides synchronized way of dealing with this @anoopsjohn FYI
--
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
[GitHub] [hbase] apurtell commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r671515927
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
We can represent this as an atomic boolean and atomic integer together, right? We are either checking or adjusting the high order bit atomically or adjusting the refcount atomically, we aren't doing both together. There isn't a case where the high order bit is updated *and* the refcount is changed and both must be applied as a single atomic operation.
The code will be much more understandable and maintainable with a well-named atomic boolean field and the result is no longer a dirty hack.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-884016048
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 27s | 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 26s | master passed |
| +1 :green_heart: | compile | 1m 14s | master passed |
| +1 :green_heart: | shadedjars | 8m 20s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 26s | the patch passed |
| +1 :green_heart: | compile | 1m 14s | the patch passed |
| +1 :green_heart: | javac | 1m 14s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 28s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 167m 40s | hbase-server in the patch failed. |
| | | 201m 1s | |
| 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-3495/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c1a9dfc0cd85 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 / 16721239e7 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/5/testReport/ |
| Max. process+thread count | 4105 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883008477
+1 to address the new class (generic) in a followup issue. Would like to get this committed and get this in for 2.3.6 @saintstack pinged for issues in dev@ mail chain. Multiple times this bug been raised.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#issuecomment-883878292
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 14s | 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 22s | master passed |
| +1 :green_heart: | compile | 3m 27s | master passed |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | spotbugs | 2m 14s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 3s | root in the patch failed. |
| -1 :x: | compile | 1m 58s | hbase-server in the patch failed. |
| -0 :warning: | javac | 1m 58s | hbase-server in the patch failed. |
| +1 :green_heart: | checkstyle | 1m 8s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | hadoopcheck | 2m 15s | The patch causes 20 errors with Hadoop v3.1.2. |
| -1 :x: | hadoopcheck | 4m 34s | The patch causes 20 errors with Hadoop v3.2.1. |
| -1 :x: | hadoopcheck | 6m 56s | The patch causes 20 errors with Hadoop v3.3.0. |
| -1 :x: | spotbugs | 0m 31s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 31m 52s | |
| 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-3495/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3495 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 138b30204477 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 16721239e7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-javac-3.3.0.txt |
| spotbugs | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3495/4/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-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3495: HBASE-24984 WAL corruption due to early DBBs re-use when Durability.A…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672307084
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
private long exceptionSize = 0;
private final boolean retryImmediatelySupported;
- // This is a dirty hack to address HBASE-22539. The lowest bit is for normal rpc cleanup, and the
- // second bit is for WAL reference. We can only call release if both of them are zero. The reason
- // why we can not use a general reference counting is that, we may call cleanup multiple times in
- // the current implementation. We should fix this in the future.
- private final AtomicInteger reference = new AtomicInteger(0b01);
+ // This is a dirty hack to address HBASE-22539. The highest bit is for rpc ref and cleanup, and
+ // the rest of the bits are for WAL reference count. We can only call release if all of them are
+ // zero. The reason why we can not use a general reference counting is that, we may call cleanup
+ // multiple times in the current implementation. We should fix this in the future.
+ // The refCount here will start as 0x80000000 and increment with every WAL reference and decrement
+ // from WAL side on release
+ private final AtomicInteger reference = new AtomicInteger(0x80000000);
Review comment:
No...
The rpc release and wal release could happen at the same time. We need a single atomic operation to avoid race here.
Maybe we could introduce a separated class for this logic to make the code in ServerCall cleaner, and add clear document for the new class about the logic.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
}
}
- @Override
- public void cleanup() {
- release(0b01);
+ public void retainByWAL() {
+ reference.incrementAndGet();
}
- public void retainByWAL() {
- for (;;) {
- int ref = reference.get();
- int nextRef = ref | 0b10;
- if (reference.compareAndSet(ref, nextRef)) {
- return;
+ public void releaseByWAL() {
+ if (reference.decrementAndGet() == 0) {
Review comment:
I suggest we add some comments here, to say that decrementAndGet still work even if the highest bit is 1, which means we are decrementing a negative value. I was confused at first but then I realized that the arithmetic is the same for positive and negative 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