You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "hgromer (via GitHub)" <gi...@apache.org> on 2025/06/10 01:39:31 UTC
[PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
hgromer opened a new pull request, #7084:
URL: https://github.com/apache/hbase/pull/7084
(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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136761371
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java:
##########
@@ -136,4 +138,48 @@ public void run() {
SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotProto, TABLE_NAME, CF);
SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotOnSameTableProto, TABLE_NAME, CF);
}
+
+ @Test
+ public void testItCanEnableTableWhileSnapshotProcedureIsRunning() throws Exception {
Review Comment:
This test fails without my changes, both procedures will run forever. This also functions as the reproduction of the phenomenon.
For reproducibility, I manually set the table state to ENABLING, and then kick off the EnableTableProcedure at the stage it gets stuck on (after subprocesses finish)
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136763016
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
Review Comment:
The Procedure class has a `holdLock` which returns true for SnapshotProcedure. This prevents us from releasing the lock even on suspension. I believe that at the stage where the deadlock could happen, it's safe to release the lock. We haven't done any snapshotting yet, so I believe there shouldn't be any weird data inconsistencies from region splits/merges.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2137626214
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
Alternatively, maybe a less confusing option would be to return a non-static value for `holdLock` in `SnapshotProcedure`
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2961394131
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | 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 24s | | master passed |
| +1 :green_heart: | compile | 0m 59s | | master passed |
| +1 :green_heart: | javadoc | 0m 26s | | master passed |
| +1 :green_heart: | shadedjars | 6m 2s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | 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 28s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 3s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 210m 32s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/4/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 237m 13s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux ddbd13f3d69c 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 / 937204375e22dffb465dab72bf5dc2d9daf9e313 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/4/testReport/ |
| Max. process+thread count | 5970 (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-7084/4/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2981903664
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 33s | | 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 55s | | master passed |
| +1 :green_heart: | compile | 0m 57s | | master passed |
| +1 :green_heart: | javadoc | 0m 29s | | master passed |
| +1 :green_heart: | shadedjars | 6m 12s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | the patch passed |
| +1 :green_heart: | compile | 0m 59s | | the patch passed |
| +1 :green_heart: | javac | 0m 59s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 4s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 226m 6s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/8/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 253m 15s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/8/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 2c2a9570b591 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 / 01997a45c9f2a618a363ba6fc6ac6c6a9354cd49 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/8/testReport/ |
| Max. process+thread count | 5505 (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-7084/8/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2997009685
I think the latest failed test is unrelated, merged an empty commit to trigger a re-build
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136759998
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java:
##########
@@ -136,4 +138,48 @@ public void run() {
SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotProto, TABLE_NAME, CF);
SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotOnSameTableProto, TABLE_NAME, CF);
}
+
+ @Test
+ public void testItCanEnableTableWhileSnapshotProcedureIsRunning() throws Exception {
Review Comment:
This test fails without my changes, both procedures will run forever
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2137612634
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
I don't think we should. If we did, we'd have potential inconsistencies. As I mention in my comment, this bypass allows us to release the lock _prior_ to us actually snapshotting any data. Once we start snapshotting (anything in at or past SNAPSHOT_SNAPSHOT_ONLINE_REGIONS) it is unsafe to release the lock.
I think the comment still holds. We want to avoid enabling/disabling/modifying/deleting a table during a snapshot. The procedure suspension occurs prior to us starting to snapshot any data, at SNAPSHOT_WRITE_SNAPSHOT_INFO.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3000495964
@Apache9 I think this PR is good to go, please let me know if you think everything makes sense!
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2985107179
> TestSnapshotProcedureRIT.testTableInMergeWhileTakingSnapshot failed, I guess this is related to our changes here?
Yes must be related I am taking a look
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2998499430
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | 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 9s | | master passed |
| +1 :green_heart: | compile | 0m 54s | | master passed |
| +1 :green_heart: | javadoc | 0m 28s | | master passed |
| +1 :green_heart: | shadedjars | 5m 50s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 56s | | the patch passed |
| +1 :green_heart: | compile | 0m 54s | | the patch passed |
| +1 :green_heart: | javac | 0m 54s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 26s | | the patch passed |
| +1 :green_heart: | shadedjars | 5m 44s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 225m 9s | | hbase-server in the patch passed. |
| | | 250m 42s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/10/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 21852ab9c1d1 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 / 2c3f6bd5122908d03f4167b233c4288a47d96084 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/10/testReport/ |
| Max. process+thread count | 4450 (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-7084/10/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2959924484
> > Enabling/disabling tables happens often enough that it might be fine to simply fail and force the user to manually handle the failure.
>
> Do you mean "happens ~often~ infrequently enough"? If so, I agree that snapshotting a disabled table should be a pretty exceptional request, and so it is fine to err on the side of simplicity and just fail the procedure
Yes I did; I am happy to fail the procedure if that's the general consensus. That being said, I do think modern backups start to stress this system out. Any call to the BackupAdmin triggers an enable table procedure, so we're more likely to get into this state.
If we are okay with failing, then the user will have to manually kick off another snapshot on failure.
Another possible solution would be to reset the state of the snapshot procedure, so that it needs to run from the beginning
```java
if (tableState.isEnabled()) {
// action
} else if (tableState.isDisabled) {
// action
} else {
// set up suspension timeout/persistence
setNextState(SnapshotState.SNAPSHOT_PREPARE);
}
```
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2960202435
going to proceed with the strategy of simply failing
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2961030370
: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 13s | | master passed |
| +1 :green_heart: | compile | 0m 58s | | master passed |
| +1 :green_heart: | javadoc | 0m 28s | | master passed |
| +1 :green_heart: | shadedjars | 6m 5s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 9s | | 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 26s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 2s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 211m 38s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/3/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 237m 37s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux ed03acff7815 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 / 5b6ad478212b6c97434828be09ba126041b87401 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/3/testReport/ |
| Max. process+thread count | 5234 (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-7084/3/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2962503043
> When implementing this SnapshotProcedure, we decided to use shared lock and holdLock = true to prevent other table procedure jumps in in the middle of our execution while not hurting the availability because exclusive lock will also prevent region assigning.
>
> In [HBASE-28683](https://issues.apache.org/jira/browse/HBASE-28683), we introduced a new way to only allow one procedure to run at the same time for the same table, so maybe a possible way is to make SnapshotProcedure also acquire exclusive lock and set holdLock = false, so it will not be executed at the same time with Enable and Disable procedure.
>
> Thanks.
That's good to know, thank you for the context. The issue here is that the EnableTableProcedure will release the lock after it's subprocedures finish, which allows the SnapshotProcedure to execute on a table that is enabling. The SnapshotProcedure then gets stuck continuously re-running the same state over again, and will refuse to release the lock, creating a deadlock
With all this said, I think it makes the most sense, for simplicity's sake, to fail the procedure if the table is in an invalid state.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2993003748
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | 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 53s | | master passed |
| +1 :green_heart: | compile | 0m 58s | | master passed |
| +1 :green_heart: | javadoc | 0m 28s | | master passed |
| +1 :green_heart: | shadedjars | 6m 5s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 11s | | 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 2s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 214m 55s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/9/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 242m 39s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/9/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 9a40d0fa1370 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 / f14a7cff03a1046a4eba2597e529844fa646c193 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/9/testReport/ |
| Max. process+thread count | 5312 (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-7084/9/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2140032482
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java:
##########
@@ -143,13 +144,18 @@ protected Flow executeFromState(MasterProcedureEnv env, SnapshotState state)
setNextState(SnapshotState.SNAPSHOT_WRITE_SNAPSHOT_INFO);
return Flow.HAS_MORE_STATE;
case SNAPSHOT_WRITE_SNAPSHOT_INFO:
- SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
TableState tableState =
env.getMasterServices().getTableStateManager().getTableState(snapshotTable);
if (tableState.isEnabled()) {
+ SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
setNextState(SnapshotState.SNAPSHOT_SNAPSHOT_ONLINE_REGIONS);
} else if (tableState.isDisabled()) {
+ SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
setNextState(SnapshotState.SNAPSHOT_SNAPSHOT_CLOSED_REGIONS);
+ } else {
+ setState(ProcedureState.FAILED);
+ throw new ProcedureAbortedException(
Review Comment:
Will do, thank you for the 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
Re: [PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136759998
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureConcurrently.java:
##########
@@ -136,4 +138,48 @@ public void run() {
SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotProto, TABLE_NAME, CF);
SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotOnSameTableProto, TABLE_NAME, CF);
}
+
+ @Test
+ public void testItCanEnableTableWhileSnapshotProcedureIsRunning() throws Exception {
Review Comment:
This test fails without my changes, both procedures will run forever
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2959803797
> Enabling/disabling tables happens often enough that it might be fine to simply fail and force the user to manually handle the failure.
Do you mean "happens ~often~ infrequently enough"? If so, I agree that snapshotting a disabled table should be a pretty exceptional request, and so it is fine to err on the side of simplicity and just fail the procedure
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2960308052
: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 23s | | master passed |
| +1 :green_heart: | compile | 3m 20s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 36s | | master passed |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | the patch passed |
| +1 :green_heart: | compile | 3m 15s | | the patch passed |
| +1 :green_heart: | javac | 3m 15s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 43s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 14s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | | The patch does not generate ASF License warnings. |
| | | 40m 2s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 5a56b88b319c 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 / 31d51955b8ec2bef0cda620c842704e48ca06137 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 86 (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-7084/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2960791629
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | 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 | 4m 37s | | master passed |
| +1 :green_heart: | compile | 4m 49s | | master passed |
| +1 :green_heart: | checkstyle | 0m 50s | | master passed |
| +1 :green_heart: | spotbugs | 2m 6s | | master passed |
| +1 :green_heart: | spotless | 1m 11s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 9s | | the patch passed |
| +1 :green_heart: | compile | 4m 18s | | the patch passed |
| +1 :green_heart: | javac | 4m 18s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 42s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 25s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 15m 5s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| -1 :x: | spotless | 0m 58s | | patch has 29 errors when running spotless:check, run spotless:apply to fix. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | | The patch does not generate ASF License warnings. |
| | | 51m 59s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux f4853626e3e9 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 / 5b6ad478212b6c97434828be09ba126041b87401 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/3/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 89 (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-7084/3/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2981323335
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | 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 45s | | master passed |
| +1 :green_heart: | compile | 3m 27s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 38s | | master passed |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | the patch passed |
| +1 :green_heart: | compile | 3m 16s | | the patch passed |
| +1 :green_heart: | javac | 3m 16s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 36s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 42s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 18s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | | The patch does not generate ASF License warnings. |
| | | 41m 45s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 9dd13ef03bae 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 / 01997a45c9f2a618a363ba6fc6ac6c6a9354cd49 |
| 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-7084/8/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2957459711
cc @charlesconnell
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2957449986
@rmdmattingly @ndimiduk @ @sidkhillon @krconv
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "charlesconnell (via GitHub)" <gi...@apache.org>.
charlesconnell commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2959265464
Another option, which is simpler to reason about, is to simply fail the `SnapshotProcedure` if the table isn't in a state it can handle. This was my approach in HBASE-29315, for the exact same reasons you're encountering here. I couldn't usefully "sleep" a `SplitTableRegionProcedure` because it used holdLock=true. I think it would be good to agree on a standard of what to do in these situations.
Failing a procedure is simpler and doesn't introduce more edge cases in the procedure executor state machine. However, obviously, it's a better user experience if your procedures get executed eventually.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2984577400
TestSnapshotProcedureRIT.testTableInMergeWhileTakingSnapshot failed, I guess this is related to our changes here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3027946741
Your direct memory limit is 1.75GB, which seems too small? What is your heap size? If you enable off heap blockcache and off heap memstore, you need to configure a higher direct memory limitation, which may be greater than your heap size.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2961108958
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 26s | | 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 7s | | master passed |
| +1 :green_heart: | compile | 3m 19s | | master passed |
| +1 :green_heart: | checkstyle | 0m 36s | | master passed |
| +1 :green_heart: | spotbugs | 1m 35s | | master passed |
| +1 :green_heart: | spotless | 0m 45s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 2s | | the patch passed |
| +1 :green_heart: | compile | 3m 21s | | the patch passed |
| +1 :green_heart: | javac | 3m 21s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 41s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 13s | | 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 18s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux eaf376fc4bc8 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 / 937204375e22dffb465dab72bf5dc2d9daf9e313 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (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-7084/4/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136766895
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java:
##########
@@ -143,13 +143,26 @@ protected Flow executeFromState(MasterProcedureEnv env, SnapshotState state)
setNextState(SnapshotState.SNAPSHOT_WRITE_SNAPSHOT_INFO);
return Flow.HAS_MORE_STATE;
case SNAPSHOT_WRITE_SNAPSHOT_INFO:
- SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
TableState tableState =
env.getMasterServices().getTableStateManager().getTableState(snapshotTable);
if (tableState.isEnabled()) {
+ SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
Review Comment:
Move inside the if statements to avoid unnecessary I/O in case we need to suspend
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3024649708
The holdLock is false for SnapshotProcedure, so unless it is blocked while execution, it will release the table lock when it can not go on. So I do not think it is the root cause for your problem.
And why SplitWALRemoteProcedure requires table locks? I haven't seen any table lock related code in SplitWALRemoteProcedure.
And the log you provided is not about SCP, it says the region is SPLITTING?
I think you have messed up SplitWALRemoteProcedure and SnapshotRegionProcedure?
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3027635862
Any updates here?
It is a bit strange that, we should have check in both SnapshotProcedure and SplitTableTableProcedure/MergeTableRegionsProcedure, to prevent they execute concurrently. So why you can see a region in SPLITTING state while executing SnapshotProcedure?
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3024611037
I think we need to revert this PR. This change can cause another deadlock, which can interfere with server crash procedures.
The SCP acquires a server exclusive lock, so it can run in parallel to a snapshot procedure. However the SCP will schedule SplitWALRemoteProcedure which do acquire table locks. The SplitWALRemoteProcedure won't run until the snapshot procedure finishes, however the snapshot procedure will get stuck at state SNAPSHOT_SNAPSHOT_SPLIT_REGIONS waiting for the server to go online.
```
2025-07-01T15:23:20,413 [PEWorker-2] WARN org.apache.hadoop.hbase.master.procedure.SnapshotRegionProcedure: pid=2365228, ppid=2365224, state=RUNNABLE, locked=true; SnapshotRegionProcedure 91f810e77abe57ea0791ea6e86ada219 can not run currently because target server of region migrate-test-1,\x7F\xFF\xFF\xFE,1751380484623.91f810e77abe57ea0791ea6e86ada219. na1-elegant-jaded-egg.iad03.hubinternal.net,60020,1751300917328 is in state SPLITTING, wait 600000 ms to retry
```
This puts us in a state where the children of the SCP will never finish, which means the SCP will never finish, which also blocks the snapshot procedure from finishing.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2972847153
> > This is certainly a cleaner implementation, though it means we can interleave other table procedures after cycles of the SnapshotProcedure.
>
> FWIW, if a SnapshotProcedure starts, no other table procedures which need a exclusive lock can be executed...
>
> And what I mean is that, we can reuse the mechanism introduced in [HBASE-28683](https://issues.apache.org/jira/browse/HBASE-28683) to simply fix the problem.
>
> Just change the code here
>
> https://github.com/apache/hbase/blob/64c582fe9a846b2f5e07b180cc6fae77431d726e/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java#L62
>
> Make Snapshot also return true, and change SnapshotProcedure's acquireLock method to also require exclusive lock, change holdLock to return false, then we are safe.
>
> Thanks.
Ah okay, I understand. This does prevent us from being able to take multiple snapshots of the table, as mentioned in [this](https://github.com/apache/hbase/blob/64c582fe9a846b2f5e07b180cc6fae77431d726e/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java#L111C37-L111C45) comment but that seems ok
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2972557282
> This is certainly a cleaner implementation, though it means we can interleave other table procedures after cycles of the SnapshotProcedure.
FWIW, if a SnapshotProcedure starts, no other table procedures which need a exclusive lock can be executed...
And what I mean is that, we can reuse the mechanism introduced in HBASE-28683 to simply fix the problem.
Just change the code here
https://github.com/apache/hbase/blob/64c582fe9a846b2f5e07b180cc6fae77431d726e/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java#L62
Make Snapshot also return true, and change SnapshotProcedure's acquireLock method to also require exclusive lock, change holdLock to return false, then we are safe.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2992470898
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 32s | | 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 54s | | master passed |
| +1 :green_heart: | compile | 3m 24s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 40s | | master passed |
| +1 :green_heart: | spotless | 0m 49s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | the patch passed |
| +1 :green_heart: | compile | 3m 17s | | the patch passed |
| +1 :green_heart: | javac | 3m 17s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 42s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 16s | | 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 10s | | The patch does not generate ASF License warnings. |
| | | 40m 38s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/9/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 4e14747c481e 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 / f14a7cff03a1046a4eba2597e529844fa646c193 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 84 (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-7084/9/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3027898590
The log line indicates that the server is in a SPLITTING state, not the region. The SnapshotProcedure will check [here](https://github.com/apache/hbase/blob/3bbed010622708c95229687249b88559a281ef9f/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotRegionProcedure.java#L181) if the target server is online and suspend the procedure if it isn't.
We were in a tricky scenario where a test cluster was trying to run various SCP, and those SCP were essentially blocking the SnapshotProcedure. I had though the SnapshotProcedure was preventing the SCP from proceeding, but it turns out SCP was failing when running a SplitWALRemoteProcedure due to memory issues
```
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster 2025-07-01T17:01:38,573 [KeepAlivePEWorker-15] WARN org.apache.hadoop.hbase.master.procedure.SplitWALRemoteProcedure: Failed split of hdfs://sandbox-hb2-a-qa:8020/hbase/WALs/na1-broad-grand-falcon.iad03.hubinternal.net,60020,1751299239767-splitting/na1-broad-grand-falcon.iad03.hubinternal.net%2C60020%2C1751299239767.1751381096903, retry...
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster org.apache.hadoop.hbase.procedure2.RemoteProcedureException: java.lang.Exception: Cannot reserve 262144 bytes of direct buffer memory (allocated: 1879042125, limit: 1879048192)
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.procedure2.RemoteProcedureException.fromProto(RemoteProcedureException.java:123) ~[hbase-procedure-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.master.MasterRpcServices.lambda$reportProcedureDone$4(MasterRpcServices.java:2573) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at java.util.ArrayList.forEach(ArrayList.java:1596) ~[?:?]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1116) ~[?:?]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.master.MasterRpcServices.reportProcedureDone(MasterRpcServices.java:2568) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProtos$RegionServerStatusService$2.callBlockingMethod(RegionServerStatusProtos.java:16726) ~[hbase-protocol-shaded-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:443) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.ipc.CallRunner.run(CallRunner.java:124) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.ipc.RpcHandler.run(RpcHandler.java:105) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.ipc.RpcHandler.run(RpcHandler.java:85) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster Caused by: java.lang.Exception: Cannot reserve 262144 bytes of direct buffer memory (allocated: 1879042125, limit: 1879048192)
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at java.nio.Bits.reserveMemory(Bits.java:178) ~[?:?]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at java.nio.DirectByteBuffer.<init>(DirectByteBuffer.java:111) ~[?:?]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at java.nio.ByteBuffer.allocateDirect(ByteBuffer.java:360) ~[?:?]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.io.compress.zstd.ZstdDecompressor.<init>(ZstdDecompressor.java:46) ~[hbase-compression-zstd-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.io.compress.zstd.ZstdCodec.createDecompressor(ZstdCodec.java:94) ~[hbase-compression-zstd-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.io.compress.CodecPool.getDecompressor(CodecPool.java:166) ~[hbase-common-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.io.compress.Compression$Algorithm.getDecompressor(Compression.java:487) ~[hbase-common-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.wal.CompressionContext$ValueCompressor.decompress(CompressionContext.java:119) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.wal.WALCellCodec$CompressedKvDecoder.readCompressedValue(WALCellCodec.java:385) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.wal.WALCellCodec$CompressedKvDecoder.parseCell(WALCellCodec.java:336) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.codec.BaseDecoder.advance(BaseDecoder.java:66) ~[hbase-common-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.wal.WALEdit.readFromCells(WALEdit.java:281) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.wal.ProtobufWALStreamReader.next(ProtobufWALStreamReader.java:84) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.wal.WALStreamReader.next(WALStreamReader.java:42) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.wal.WALSplitter.getNextLogLine(WALSplitter.java:490) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.wal.WALSplitter.splitWAL(WALSplitter.java:319) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.wal.WALSplitter.splitLogFile(WALSplitter.java:200) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.SplitLogWorker.splitLog(SplitLogWorker.java:108) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.SplitWALCallable.doCall(SplitWALCallable.java:86) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.procedure2.BaseRSProcedureCallable.call(BaseRSProcedureCallable.java:35) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.procedure2.BaseRSProcedureCallable.call(BaseRSProcedureCallable.java:23) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.regionserver.handler.RSProcedureHandler.process(RSProcedureHandler.java:56) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
hmaster-sandbox-hb2-a-1-557d64787b-9cgkh hmaster at org.apache.hadoop.hbase.executor.EventHandler.run(EventHandler.java:104) ~[hbase-server-2.6-hubspot-SNAPSHOT.jar:2.6-hubspot-SNAPSHOT]
```
I'm still attempting to track down exactly, the entire host's memory was being eaten up by RecoveredEditsOutputSink#append. We did have a disproportionately high number of regions per host on the cluster, which likely was the cause.
Importantly, the SnapshotProcedure was not the cause of these stuck SCP
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3027948184
Anyway, seems we are good to close this issue and go with the 2.6.3 release.
Thanks guys!
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3024736269
I may have conflated `StateMachineProcedure` and `AbstractStateMachineTableProcedure`, which is why I thought the SplitWALRemoteProcedure acquired a table lock. Taking a deeper look and will report back
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136763016
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
Review Comment:
The Procedure class has a `holdLock` which returns true for SnapshotProcedure. This prevents us from releasing the lock even on suspension. I believe that at the stage where the deadlock could happen, it's safe to release the lock. We haven't done any snapshotting yet, so I believe there shouldn't be any weird data inconsistencies from region splits/merges.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2136764338
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
The Procedure class has a `holdLock` method which returns true for SnapshotProcedure. This prevents us from releasing the lock even on suspension. I believe that at the stage where the deadlock could happen, it's safe to release the lock. We haven't done any snapshotting yet, so I believe there shouldn't be any weird data inconsistencies from region splits/merges.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "wchevreuil (via GitHub)" <gi...@apache.org>.
wchevreuil commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2137592020
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
Shouldn't we just modify the SnapshotProcedure.holdLock() behaviour? Afterall, this is contrary to what we say in that method comment:
` protected boolean holdLock(MasterProcedureEnv env) {
// In order to avoid enabling/disabling/modifying/deleting table during snapshot,
// we don't release lock during suspend
return true;
}
`
And you are already modifying SnapshotProcedure execution logic to reflect the special condition when the lock should be released, so better keep this there and avoid changing ProcedureSuspendedException?
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2978633283
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | 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 59s | | master passed |
| +1 :green_heart: | compile | 4m 3s | | master passed |
| +1 :green_heart: | checkstyle | 0m 42s | | master passed |
| +1 :green_heart: | spotbugs | 1m 44s | | master passed |
| +1 :green_heart: | spotless | 0m 53s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | | the patch passed |
| +1 :green_heart: | compile | 3m 30s | | the patch passed |
| +1 :green_heart: | javac | 3m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 1s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 40s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 1s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 28s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 44s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | | The patch does not generate ASF License warnings. |
| | | 42m 48s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 364abec9f6d4 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 / 8adc8bf77e3d8e98606d8a42c852841ff2038ee6 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (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-7084/7/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2167131491
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureRIT.java:
##########
@@ -53,8 +53,7 @@ public void testTableInMergeWhileTakingSnapshot() throws Exception {
() -> procExec.getProcedure(mergeProcId).getState() == ProcedureState.RUNNABLE);
SnapshotProcedure sp = new SnapshotProcedure(procExec.getEnvironment(), snapshotProto);
long snapshotProcId = procExec.submitProcedure(sp);
- TEST_UTIL.waitFor(2000, 1000, () -> procExec.getProcedure(snapshotProcId) != null
Review Comment:
Sure; the SnapshotProcedure will never reach the state where it is running at the same time that the merge procedures are running because it will now require an exclusive locks of the table to run. So when we kick off the SnapshotProcedure, it will sit in the `TableProcedureWaitingQueue` as RUNNABLE before being scheduled. Therefore, [this](https://github.com/apache/hbase/blob/f0e0f567c0df5013add6e77c903299a9e9c213f0/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java#L241) code path will never be executed. We can clean this code up too if we'd like
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2963029524
: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 11s | | master passed |
| +1 :green_heart: | compile | 0m 59s | | master passed |
| +1 :green_heart: | javadoc | 0m 27s | | master passed |
| +1 :green_heart: | shadedjars | 6m 5s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 3s | | 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 | 5m 57s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 211m 59s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/5/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 238m 0s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux a23e1a60072c 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 / 4143909ff899b9aa3c36092a3b655312e5ac9df6 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/5/testReport/ |
| Max. process+thread count | 5332 (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-7084/5/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2963179415
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 27s | | 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 9s | | master passed |
| +1 :green_heart: | compile | 3m 27s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 36s | | master passed |
| +1 :green_heart: | spotless | 0m 48s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 9s | | the patch passed |
| +1 :green_heart: | compile | 3m 13s | | the patch passed |
| +1 :green_heart: | javac | 3m 13s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 42s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 9s | | 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 27s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 2046f527496b 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 / a33389edf0504be46b9487ef5943a409c652506e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (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-7084/6/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2139009174
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java:
##########
@@ -143,13 +144,18 @@ protected Flow executeFromState(MasterProcedureEnv env, SnapshotState state)
setNextState(SnapshotState.SNAPSHOT_WRITE_SNAPSHOT_INFO);
return Flow.HAS_MORE_STATE;
case SNAPSHOT_WRITE_SNAPSHOT_INFO:
- SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
TableState tableState =
env.getMasterServices().getTableStateManager().getTableState(snapshotTable);
if (tableState.isEnabled()) {
+ SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
setNextState(SnapshotState.SNAPSHOT_SNAPSHOT_ONLINE_REGIONS);
} else if (tableState.isDisabled()) {
+ SnapshotDescriptionUtils.writeSnapshotInfo(snapshot, workingDir, workingDirFS);
setNextState(SnapshotState.SNAPSHOT_SNAPSHOT_CLOSED_REGIONS);
+ } else {
+ setState(ProcedureState.FAILED);
+ throw new ProcedureAbortedException(
Review Comment:
Just use setFailure and then return Flow.NO_MORE_STATE.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2137626214
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
Alternatively, maybe a less confusing option would be to return a non-static value for `holdLock` in `SnapshotProcedure`
This forces you to manage additional state at each stage though, which I why I opted for this implementation which I thought was safer
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2970866609
> > > When implementing this SnapshotProcedure, we decided to use shared lock and holdLock = true to prevent other table procedure jumps in in the middle of our execution while not hurting the availability because exclusive lock will also prevent region assigning.
> > > In [HBASE-28683](https://issues.apache.org/jira/browse/HBASE-28683), we introduced a new way to only allow one procedure to run at the same time for the same table, so maybe a possible way is to make SnapshotProcedure also acquire exclusive lock and set holdLock = false, so it will not be executed at the same time with Enable and Disable procedure.
> > > Thanks.
> >
> >
> > That's good to know, thank you for the context. The issue here is that the EnableTableProcedure will release the lock after it's subprocedures finish, which allows the SnapshotProcedure to execute on a table that is enabling. The SnapshotProcedure then gets stuck continuously re-running the same state over again, and will refuse to release the lock, creating a deadlock
> > With all this said, I think it makes the most sense, for simplicity's sake, to fail the procedure if the table is in an invalid state.
>
> Using the mechanism in [HBASE-28683](https://issues.apache.org/jira/browse/HBASE-28683), SnapshotProcedure can not be executed together with EnableTableProcedure/DisabledTableProcedure together, which could also solve the problem, and I think it is more stable. I'm not sure whether ModifyTableProcedure could also affect SnapshotProcedure if it jumps in just in the middle of the execution...
Yes, I'm able to avoid the deadlock if the SnapshotProcedure both sets `holdLock = true` and also yields after procedure execution. Essentially allowing both SnapshotProcedure and EnableTableProcedure to execute without the need to throw any sort of yield / suspended exception (which was my initial implementation).
This is certainly a cleaner implementation, though it means we can interleave other table procedures after cycles of the SnapshotProcedure.
You hint at this here
> I'm not sure whether ModifyTableProcedure could also affect SnapshotProcedure if it jumps in just in the middle of the execution...
and I believe that it would be problematic because you may snapshot regions multiple times, regions may move or go offline, so you may miss out on snapshotting regions.
My initial implementation prevented this by only allowing SNAPSHOT_WRITE_SNAPSHOT_INFO to release the lock (when it's still safe to do so). However, I don't think it's safe to yield the lock after _any_ cycle.
After some thought, I think we still have two solutions here:
1. My initial implementation, which throws an exception that indicates we should release the lock from within SNAPSHOT_WRITE_SNAPSHOT_INFO
2. Fail the procedure
It seems the consensus is to avoid complicating the procedure and fail if we encounter the table in an invalid state, so I'm leaning towards 2
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-3027991655
Yes, we were running on very small instance types. A test ran awry, which caused us to run into memory issues.
Thank you for jumping in here, and apologies for the back and forth
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2962231104
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 26s | | 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 6s | | master passed |
| +1 :green_heart: | compile | 3m 22s | | master passed |
| +1 :green_heart: | checkstyle | 0m 37s | | master passed |
| +1 :green_heart: | spotbugs | 1m 35s | | master passed |
| +1 :green_heart: | spotless | 0m 46s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 9s | | the patch passed |
| +1 :green_heart: | compile | 3m 24s | | the patch passed |
| +1 :green_heart: | javac | 3m 24s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 37s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 43s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 8s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| -1 :x: | spotless | 0m 38s | | patch has 29 errors when running spotless:check, run spotless:apply to fix. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | | The patch does not generate ASF License warnings. |
| | | 39m 22s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux 00d7ba8b3f5f 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 / 4143909ff899b9aa3c36092a3b655312e5ac9df6 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/5/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 84 (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-7084/5/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2998125852
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 29s | | 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 31s | | master passed |
| +1 :green_heart: | compile | 3m 26s | | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | | master passed |
| +1 :green_heart: | spotbugs | 1m 37s | | master passed |
| +1 :green_heart: | spotless | 0m 48s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 7s | | the patch passed |
| +1 :green_heart: | compile | 3m 16s | | the patch passed |
| +1 :green_heart: | javac | 3m 16s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 36s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 42s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 14s | | 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 10s | | The patch does not generate ASF License warnings. |
| | | 40m 1s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/10/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux e6b5fd73fd1f 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 / 2c3f6bd5122908d03f4167b233c4288a47d96084 |
| 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-7084/10/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2165954977
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureRIT.java:
##########
@@ -53,8 +53,7 @@ public void testTableInMergeWhileTakingSnapshot() throws Exception {
() -> procExec.getProcedure(mergeProcId).getState() == ProcedureState.RUNNABLE);
SnapshotProcedure sp = new SnapshotProcedure(procExec.getEnvironment(), snapshotProto);
long snapshotProcId = procExec.submitProcedure(sp);
- TEST_UTIL.waitFor(2000, 1000, () -> procExec.getProcedure(snapshotProcId) != null
Review Comment:
Mind explaining a bit why we need to remove the WAITING_TIMEOUT state 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
Re: [PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2968752816
> > When implementing this SnapshotProcedure, we decided to use shared lock and holdLock = true to prevent other table procedure jumps in in the middle of our execution while not hurting the availability because exclusive lock will also prevent region assigning.
> > In [HBASE-28683](https://issues.apache.org/jira/browse/HBASE-28683), we introduced a new way to only allow one procedure to run at the same time for the same table, so maybe a possible way is to make SnapshotProcedure also acquire exclusive lock and set holdLock = false, so it will not be executed at the same time with Enable and Disable procedure.
> > Thanks.
>
> That's good to know, thank you for the context. The issue here is that the EnableTableProcedure will release the lock after it's subprocedures finish, which allows the SnapshotProcedure to execute on a table that is enabling. The SnapshotProcedure then gets stuck continuously re-running the same state over again, and will refuse to release the lock, creating a deadlock
>
> With all this said, I think it makes the most sense, for simplicity's sake, to fail the procedure if the table is in an invalid state.
Using the mechanism in HBASE-28683, SnapshotProcedure can not be executed together with EnableTableProcedure/DisabledTableProcedure together, which could also solve the problem, and I think it is more stable. I'm not sure whether ModifyTableProcedure could also affect SnapshotProcedure if it jumps in just in the middle of the execution...
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2963822580
:confetti_ball: **+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 22s | | master passed |
| +1 :green_heart: | compile | 0m 58s | | master passed |
| +1 :green_heart: | javadoc | 0m 28s | | master passed |
| +1 :green_heart: | shadedjars | 6m 1s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 9s | | the patch passed |
| +1 :green_heart: | compile | 0m 58s | | the patch passed |
| +1 :green_heart: | javac | 0m 58s | | 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 | 234m 52s | | hbase-server in the patch passed. |
| | | 262m 12s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 1a1979d71c37 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 / a33389edf0504be46b9487ef5943a409c652506e |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/6/testReport/ |
| Max. process+thread count | 5249 (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-7084/6/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2957762660
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 27s | | 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 _ |
| +0 :ok: | mvndep | 0m 16s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 18s | | master passed |
| +1 :green_heart: | compile | 1m 13s | | master passed |
| +1 :green_heart: | javadoc | 0m 40s | | master passed |
| +1 :green_heart: | shadedjars | 6m 6s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 9s | | the patch passed |
| +1 :green_heart: | compile | 1m 12s | | the patch passed |
| +1 :green_heart: | javac | 1m 12s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 38s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 2s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 32s | | hbase-procedure in the patch passed. |
| +1 :green_heart: | unit | 211m 22s | | hbase-server in the patch passed. |
| | | 240m 22s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 11495b844953 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 / 18bd20f9d4d9d9f0b83017a1adbf05066f65ae68 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/1/testReport/ |
| Max. process+thread count | 5289 (vs. ulimit of 30000) |
| modules | C: hbase-procedure hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "hgromer (via GitHub)" <gi...@apache.org>.
hgromer commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2959311914
> Another option, which is simpler to reason about, is to simply fail the `SnapshotProcedure` if the table isn't in a state it can handle. This was my approach in [HBASE-29315](https://issues.apache.org/jira/browse/HBASE-29315), for the exact same reasons you're encountering here. I couldn't usefully "sleep" a `SplitTableRegionProcedure` because it used holdLock=true. I think it would be good to agree on a standard of what to do in these situations.
>
> Failing a procedure is simpler and doesn't introduce more edge cases in the procedure executor state machine. However, obviously, it's a better user experience if your procedures get executed eventually.
Agreed. The reason I opted for supsending the procedure is because this is already something we do if [we are splitting or merging regions](https://github.com/apache/hbase/blob/a71288f2e2fe3f0b3fe254ca9a1174ce42e72d6a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotProcedure.java#L241). So this procedure will already suspend and resume in the case the table state isn't optimal.
I don't think I have a very strong opinion one way or another, though. Enabling/disabling tables happens often enough that it might be fine to simply fail and force the user to manually handle the failure.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "wchevreuil (via GitHub)" <gi...@apache.org>.
wchevreuil commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2138089524
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease) {
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
>I think the comment still holds. We want to avoid enabling/disabling/modifying/deleting a table during a snapshot.
We may want to avoid, but now we are allowing a case where we suspend and release the lock.
> Alternatively, maybe a less confusing option would be to return a non-static value for `holdLock` in `SnapshotProcedure`
>
That's what I meant by "modify the SnapshotProcedure.holdLock() behaviour". You are already modifying SnapshotProcedure behaviour to cause a release lock suspension via a flag in the ProcedureSuspendedException. We could keep this "flag" internally in SnapshotProcedure and use it to define what SnapshotProcedure.holdLock() returns.
> This forces you to manage additional state at each stage though, which I why I opted for this implementation which I thought was safer
True. Yet, it seems more intuitive to me. But that's a personal opinion, just wanted to make sure it was also considered. Up to you to decide on the "final" solution. If going with the original, just please amend the comment in holdLock to explain the scenario where we release the lock.
--
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-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2960709703
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 28s | | 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 22s | | master passed |
| +1 :green_heart: | compile | 1m 5s | | master passed |
| +1 :green_heart: | javadoc | 0m 29s | | master passed |
| +1 :green_heart: | shadedjars | 6m 14s | | branch has no errors when building our shaded downstream artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 16s | | the patch passed |
| +1 :green_heart: | compile | 0m 59s | | the patch passed |
| +1 :green_heart: | javac | 0m 59s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 6s | | patch has no errors when building our shaded downstream artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 211m 40s | [/patch-unit-hbase-server.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/2/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-server.txt) | hbase-server in the patch failed. |
| | | 238m 48s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux eccc001e1c95 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 / 31d51955b8ec2bef0cda620c842704e48ca06137 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/2/testReport/ |
| Max. process+thread count | 5273 (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-7084/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
Re: [PR] HBASE-29386: SnapshotProcedure and EnableTableProcedure can cause a deadlock [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7084:
URL: https://github.com/apache/hbase/pull/7084#issuecomment-2957512585
: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 _ |
| +0 :ok: | mvndep | 0m 15s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 19s | | master passed |
| +1 :green_heart: | compile | 3m 36s | | master passed |
| +1 :green_heart: | checkstyle | 0m 46s | | master passed |
| +1 :green_heart: | spotbugs | 2m 0s | | master passed |
| +1 :green_heart: | spotless | 0m 50s | | branch has no errors when running spotless:check. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 5s | | the patch passed |
| +1 :green_heart: | compile | 3m 41s | | the patch passed |
| +1 :green_heart: | javac | 3m 41s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 46s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 11s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 12m 24s | | Patch does not cause any errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 46s | | patch has no errors when running spotless:check. |
|||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | | The patch does not generate ASF License warnings. |
| | | 42m 45s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/7084 |
| JIRA Issue | HBASE-29386 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux a5f316c1394b 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 / 18bd20f9d4d9d9f0b83017a1adbf05066f65ae68 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-procedure hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7084/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