You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Apache9 (via GitHub)" <gi...@apache.org> on 2025/06/07 14:33:32 UTC
[PR] HBASE-29380 Add a UT to reproduce [hbase]
Apache9 opened a new pull request, #7077:
URL: https://github.com/apache/hbase/pull/7077
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Two concurrent remove peer requests may hang [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7077:
URL: https://github.com/apache/hbase/pull/7077#discussion_r2134711105
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java:
##########
@@ -599,32 +593,47 @@ private void removeGlobalQueue(String globalId) {
}
private void tryCleanupGlobalQueue(String globalId, Procedure<?> procedure) {
+ tryCleanupQueue(globalId, procedure, () -> globalMap, GLOBAL_QUEUE_KEY_COMPARATOR,
+ locking::getGlobalLock, globalRunQueue, this::removeGlobalQueue);
+ }
+
+ private static boolean isGlobalProcedure(Procedure<?> proc) {
+ return proc instanceof GlobalProcedureInterface;
+ }
+
+ private static String getGlobalId(Procedure<?> proc) {
+ return ((GlobalProcedureInterface) proc).getGlobalId();
+ }
+
+ private <T extends Comparable<T>, TNode extends Queue<T>> boolean tryCleanupQueue(T id,
+ Procedure<?> proc, Supplier<TNode> getMap, AvlKeyComparator<TNode> comparator,
+ Function<T, LockAndQueue> getLock, FairQueue<T> runQueue, Consumer<T> removeQueue) {
schedLock();
try {
- GlobalQueue queue = AvlTree.get(globalMap, globalId, GLOBAL_QUEUE_KEY_COMPARATOR);
+ Queue<T> queue = AvlTree.get(getMap.get(), id, comparator);
if (queue == null) {
- return;
+ return true;
}
- final LockAndQueue lock = locking.getGlobalLock(globalId);
- if (queue.isEmpty() && lock.tryExclusiveLock(procedure)) {
- removeFromRunQueue(globalRunQueue, queue,
- () -> "clean up global queue after " + procedure + " completed");
- removeGlobalQueue(globalId);
+ LockAndQueue lock = getLock.apply(id);
+ if (queue.isEmpty() && lock.isWaitingQueueEmpty() && !lock.isLocked()) {
+ // 1. the queue is empty
+ // 2. no procedure is in the lock's waiting queue
+ // 3. no other one holds the lock. It is possible that someone else holds the lock, usually
+ // our parent procedures
+ // If we can meet all the above conditions, it is safe for us to remove the queue
+ removeFromRunQueue(runQueue, queue, () -> "clean up queue after " + proc + " completed");
+ removeQueue.accept(id);
+ return true;
Review Comment:
Checking `isWaitingQueueEmpty()` and `!isLocked()` without acquiring the lock is not atomic—another procedure could acquire the lock between these calls. To avoid races, consider using `lock.tryExclusiveLock(...)` as before or otherwise atomically claiming the lock before cleanup.
```suggestion
if (queue.isEmpty() && lock.tryExclusiveLock(proc)) {
try {
// 1. the queue is empty
// 2. we successfully acquired the lock exclusively
// If we can meet all the above conditions, it is safe for us to remove the queue
removeFromRunQueue(runQueue, queue, () -> "clean up queue after " + proc + " completed");
removeQueue.accept(id);
return true;
} finally {
lock.releaseExclusiveLock();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Two concurrent remove peer requests may hang [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7077:
URL: https://github.com/apache/hbase/pull/7077#discussion_r2135828873
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java:
##########
@@ -599,32 +593,47 @@ private void removeGlobalQueue(String globalId) {
}
private void tryCleanupGlobalQueue(String globalId, Procedure<?> procedure) {
+ tryCleanupQueue(globalId, procedure, () -> globalMap, GLOBAL_QUEUE_KEY_COMPARATOR,
+ locking::getGlobalLock, globalRunQueue, this::removeGlobalQueue);
+ }
+
+ private static boolean isGlobalProcedure(Procedure<?> proc) {
+ return proc instanceof GlobalProcedureInterface;
+ }
+
+ private static String getGlobalId(Procedure<?> proc) {
+ return ((GlobalProcedureInterface) proc).getGlobalId();
+ }
+
+ private <T extends Comparable<T>, TNode extends Queue<T>> boolean tryCleanupQueue(T id,
+ Procedure<?> proc, Supplier<TNode> getMap, AvlKeyComparator<TNode> comparator,
+ Function<T, LockAndQueue> getLock, FairQueue<T> runQueue, Consumer<T> removeQueue) {
schedLock();
try {
- GlobalQueue queue = AvlTree.get(globalMap, globalId, GLOBAL_QUEUE_KEY_COMPARATOR);
+ Queue<T> queue = AvlTree.get(getMap.get(), id, comparator);
if (queue == null) {
- return;
+ return true;
}
- final LockAndQueue lock = locking.getGlobalLock(globalId);
- if (queue.isEmpty() && lock.tryExclusiveLock(procedure)) {
- removeFromRunQueue(globalRunQueue, queue,
- () -> "clean up global queue after " + procedure + " completed");
- removeGlobalQueue(globalId);
+ LockAndQueue lock = getLock.apply(id);
+ if (queue.isEmpty() && lock.isWaitingQueueEmpty() && !lock.isLocked()) {
+ // 1. the queue is empty
+ // 2. no procedure is in the lock's waiting queue
+ // 3. no other one holds the lock. It is possible that someone else holds the lock, usually
+ // our parent procedures
+ // If we can meet all the above conditions, it is safe for us to remove the queue
+ removeFromRunQueue(runQueue, queue, () -> "clean up queue after " + proc + " completed");
+ removeQueue.accept(id);
+ return true;
Review Comment:
All the operation is protected by the scheLock, so this is not a problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Two concurrent remove peer requests may hang [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7077:
URL: https://github.com/apache/hbase/pull/7077#issuecomment-2952906103
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 32s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 6s | | master passed |
| +1 :green_heart: | compile | 3m 22s | | master passed |
| +1 :green_heart: | checkstyle | 0m 36s | | master passed |
| +1 :green_heart: | spotbugs | 1m 32s | | master passed |
| +1 :green_heart: | spotless | 0m 45s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 3m 22s | | the patch passed |
| +1 :green_heart: | javac | 3m 22s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | hbase-server: The patch generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1) |
| +1 :green_heart: | spotbugs | 1m 42s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 11m 58s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 45s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | | The patch does not generate ASF License warnings. |
| | | 39m 16s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7077 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 1de489362706 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 32d82b0c95375d1f081f12fae3ac610948a459de |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/2/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Two concurrent remove peer requests may hang [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7077:
URL: https://github.com/apache/hbase/pull/7077#issuecomment-2952860917
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 8s | | master passed |
| +1 :green_heart: | compile | 0m 55s | | master passed |
| +1 :green_heart: | javadoc | 0m 28s | | master passed |
| +1 :green_heart: | shadedjars | 6m 4s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 8s | | the patch passed |
| +1 :green_heart: | compile | 0m 57s | | the patch passed |
| +1 :green_heart: | javac | 0m 57s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 0s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 210m 55s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/1/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 237m 16s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7077 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 89323ab1d7bd 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / c0039516ca6e9ebae81dbf547b9884a4b1c8a80c |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/1/testReport/ |
| Max. process+thread count | 5145 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/1/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Add a UT to reproduce [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7077:
URL: https://github.com/apache/hbase/pull/7077#issuecomment-2952642109
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any anti-patterns. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 24s | | master passed |
| +1 :green_heart: | compile | 3m 22s | | master passed |
| +1 :green_heart: | checkstyle | 0m 39s | | master passed |
| +1 :green_heart: | spotbugs | 1m 36s | | master passed |
| +1 :green_heart: | spotless | 0m 50s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 19s | | the patch passed |
| +1 :green_heart: | compile | 3m 23s | | the patch passed |
| +1 :green_heart: | javac | 3m 23s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 38s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 41s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 6s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 45s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | | The patch does not generate ASF License warnings. |
| | | 40m 9s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7077 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux ff76d723f50d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / c0039516ca6e9ebae81dbf547b9884a4b1c8a80c |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/1/console |
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Add a UT to reproduce [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7077:
URL: https://github.com/apache/hbase/pull/7077#issuecomment-2952581509
I've successfully reproduced the problem with a simple UT.
Let me think how to fix it properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Two concurrent remove peer requests may hang [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 merged PR #7077:
URL: https://github.com/apache/hbase/pull/7077
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Add a UT to reproduce [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7077:
URL: https://github.com/apache/hbase/pull/7077#issuecomment-2952692239
I refactored the MasterProcedureScheduler so the cleanup work for all type of queues share the same cleanup function, so I think it is enough to just test one type in the UT.
Let's wait for the pre commit result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29380 Two concurrent remove peer requests may hang [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7077:
URL: https://github.com/apache/hbase/pull/7077#issuecomment-2953163617
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
|||| _ Prechecks _ |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 15s | | master passed |
| +1 :green_heart: | compile | 0m 56s | | master passed |
| +1 :green_heart: | javadoc | 0m 28s | | master passed |
| +1 :green_heart: | shadedjars | 5m 58s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 2s | | the patch passed |
| +1 :green_heart: | compile | 0m 57s | | the patch passed |
| +1 :green_heart: | javac | 0m 57s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 3s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 217m 26s | | hbase-server in the patch passed. |
| | | 243m 33s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7077 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 4cf092f63ab1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 32d82b0c95375d1f081f12fae3ac610948a459de |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/2/testReport/ |
| Max. process+thread count | 5634 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7077/2/console |
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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