Skip to content

HBASE-27781 Fix case of action counter assertion error in handling of batch operation timeout exceeded - #6144

Closed
droudnitsky wants to merge 1 commit into
apache:branch-2from
droudnitsky:branch-2-HBASE-27781-
Closed

HBASE-27781 Fix case of action counter assertion error in handling of batch operation timeout exceeded#6144
droudnitsky wants to merge 1 commit into
apache:branch-2from
droudnitsky:branch-2-HBASE-27781-

Conversation

@droudnitsky

@droudnitskydroudnitsky commented Aug 8, 2024

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/HBASE-27781

In AsyncFutureRequestImpl we fail fast when operation timeout is exceeded during location resolution here. In that handling, we loop all actions and set them as failed. The problem is, some number of actions may already finished when we get to this spot. So the actionsInProgress would have been decremented for those already, and now we're going to decrement by all actions. This causes an assertion error since we go negative here

My understanding of AsynRequestFutureImpl is that the only actions passed to groupAndSendMultiAction which may have been completed already and we already decremented action counter for by the time we get to the operation timeout exceeded handling in that method are actions which were failed locally in groupAndSendMultiAction due to location error - the findAllLocationsOrFail method sets the action error and decrements the action counter on location resolution failure - we just need to avoid setting the error again on actions where location resolution failed when we get to this loop to avoid decrementing twice for those actions.

For replica calls, manageLocationError/manageError is responsible for result/error setting and preventing double action counter decrementing.

Test case reproduces the assertion error when one action in the batch fails on location error and then operation timeout is exceeded.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@droudnitsky

Copy link
Copy Markdown
ContributorAuthor

Two hbase-server tests that are not relevant to my changes are timing out :
precommit checks / yetus jdk8 Hadoop2 checks / (?) – org.apache.hadoop.hbase.replication.TestReplicationMetricsforUI
12m 29s
precommit checks / yetus jdk17 hadoop3 checks / (?) – org.apache.hadoop.hbase.wal.TestWALSplitCompressed
12m 39s

pushed empty commit to retrigger jenkins tests

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@droudnitsky

droudnitsky commented Aug 11, 2024

Copy link
Copy Markdown
ContributorAuthor

In the case of AsyncRequestFutureImpl with null results/no result tracking , I cannot see a way of checking if a given action was completed through an external method like isActionComplete, I think my current approach would have some issues in certain scenarios when results are null, need to understand AsyncRequestFutureImpl, maybe we can keep track of the actions failed locally in groupAndSendMultiAction as we loop over currentActions and avoid failing a done action a second time there if operation timeout is exceeded.

@droudnitsky
droudnitsky marked this pull request as draft August 12, 2024 01:03
@droudnitsky

Copy link
Copy Markdown
ContributorAuthor

I think keeping track of the actions failed locally in groupAndSendMultiAction as we loop over currentActions and avoiding failing an already locally failed action a second time if operation timeout is exceeded there should work for both null results and with replica actions. Working on those changes.

@droudnitsky
droudnitsky marked this pull request as ready for review September 30, 2024 14:16
@droudnitsky

Copy link
Copy Markdown
ContributorAuthor

@bbeaudreault may I kindly ask you to review this fix if you have the time

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@droudnitsky
droudnitskyforce-pushed the branch-2-HBASE-27781- branch 2 times, most recently from e379b25 to 225e68cCompareApril 17, 2025 20:44
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@droudnitsky
droudnitskyforce-pushed the branch-2-HBASE-27781- branch from 225e68c to 6caae7aCompareApril 18, 2025 12:36
@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 45sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗codespell0m 0scodespell was not available.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
_ branch-2 Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for branch
+1 💚mvninstall3m 10sbranch-2 passed
+1 💚compile3m 45sbranch-2 passed
+1 💚checkstyle0m 54sbranch-2 passed
+1 💚spotbugs2m 24sbranch-2 passed
+1 💚spotless0m 46sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗mvndep0m 15sMaven dependency ordering for patch
+1 💚mvninstall3m 3sthe patch passed
+1 💚compile3m 41sthe patch passed
+1 💚javac3m 41sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle0m 16shbase-client: The patch generated 0 new + 11 unchanged - 1 fixed = 11 total (was 12)
+1 💚checkstyle0m 38sThe patch passed checkstyle in hbase-server
+1 💚spotbugs2m 44sthe patch passed
+1 💚hadoopcheck16m 53sPatch does not cause any errors with Hadoop 2.10.2 or 3.3.6 3.4.0.
+1 💚spotless0m 44spatch has no errors when running spotless:check.
_ Other Tests _
+1 💚asflicense0m 19sThe patch does not generate ASF License warnings.
42m 29s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#6144
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
unameLinux 652fdf648b9f 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / 6caae7a
Default JavaEclipse Adoptium-11.0.23+9
Max. process+thread count76 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/console
versionsgit=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec31m 26sDocker mode activated.
-0 ⚠️yetus0m 15sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+0 🆗mvndep6m 15sMaven dependency ordering for branch
+1 💚mvninstall15m 30sbranch-2 passed
+1 💚compile5m 3sbranch-2 passed
+1 💚javadoc1m 50sbranch-2 passed
+1 💚shadedjars10m 36sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗mvndep0m 59sMaven dependency ordering for patch
+1 💚mvninstall5m 43sthe patch passed
+1 💚compile1m 45sthe patch passed
+1 💚javac1m 45sthe patch passed
+1 💚javadoc1m 20sthe patch passed
+1 💚shadedjars8m 18spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit9m 33shbase-client in the patch passed.
-1 ❌unit24m 34s/patch-unit-hbase-server.txthbase-server in the patch failed.
125m 4s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#6144
Optional Testsjavac javadoc unit compile shadedjars
unameLinux bd5b6e4af360 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / 6caae7a
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/testReport/
Max. process+thread count1865 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec9m 41sDocker mode activated.
-0 ⚠️yetus0m 8sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+0 🆗mvndep0m 14sMaven dependency ordering for branch
+1 💚mvninstall4m 5sbranch-2 passed
+1 💚compile1m 37sbranch-2 passed
+1 💚javadoc1m 10sbranch-2 passed
+1 💚shadedjars8m 51sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗mvndep0m 31sMaven dependency ordering for patch
+1 💚mvninstall4m 56sthe patch passed
+1 💚compile2m 1sthe patch passed
+1 💚javac2m 1sthe patch passed
+1 💚javadoc1m 9sthe patch passed
+1 💚shadedjars7m 53spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit9m 5shbase-client in the patch passed.
-1 ❌unit270m 50s/patch-unit-hbase-server.txthbase-server in the patch failed.
327m 37s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR#6144
Optional Testsjavac javadoc unit compile shadedjars
unameLinux 137e8f2db618 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / 6caae7a
Default JavaTemurin-1.8.0_412-b08
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/testReport/
Max. process+thread count4266 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec21m 8sDocker mode activated.
-0 ⚠️yetus0m 6sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+0 🆗mvndep1m 36sMaven dependency ordering for branch
+1 💚mvninstall6m 36sbranch-2 passed
+1 💚compile3m 37sbranch-2 passed
+1 💚javadoc1m 48sbranch-2 passed
+1 💚shadedjars13m 2sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗mvndep0m 29sMaven dependency ordering for patch
+1 💚mvninstall4m 32sthe patch passed
+1 💚compile1m 13sthe patch passed
+1 💚javac1m 13sthe patch passed
+1 💚javadoc0m 56sthe patch passed
+1 💚shadedjars6m 39spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit8m 20shbase-client in the patch passed.
-1 ❌unit248m 30s/patch-unit-hbase-server.txthbase-server in the patch failed.
326m 50s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#6144
Optional Testsjavac javadoc unit compile shadedjars
unameLinux 51b51c30f5f9 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / 6caae7a
Default JavaEclipse Adoptium-11.0.23+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/testReport/
Max. process+thread count4391 (vs. ulimit of 30000)
modulesC: hbase-client hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6144/9/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@droudnitsky

Copy link
Copy Markdown
ContributorAuthor

Going to open a new PR with clean history in attempt to be more review friendly

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@droudnitsky@Apache-HBase