Skip to content

HBASE-28569: fix race condition during WAL splitting leading to corru… - #6266

Merged
Apache9 merged 1 commit into
apache:masterfrom
aristanetworks:HBASE-28569
Apr 7, 2025
Merged

HBASE-28569: fix race condition during WAL splitting leading to corru…#6266
Apache9 merged 1 commit into
apache:masterfrom
aristanetworks:HBASE-28569

Conversation

@ciacono

Copy link
Copy Markdown
Contributor

…pt recovered.edits

If an exception happens in the call to finishWriterThreads in the org.apache.hadoop.hbase.wal.RecoveredEditsOutputSink.close method, the call to closeWriters should not execute, as it may lead to a race condition that leads to file corruption if the regionserver aborts. The execution of closeWriters in this case would write the trailer in parallel with writer threads, causing corruption, and then the corrupt file would get renamed and finalized when it should not be. This corruption causes problems when the region is then to be assigned. By removing the try finally block, the problematic closeWriters would not execute in the case of an exception in finishWriterThreads, which should then prevent this race from occurring and causing recovered.edits corruption.

@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.

@tsuna

Copy link
Copy Markdown

Can we get some eyes on this review?

@ciacono
ciacono marked this pull request as draft March 19, 2025 19:10
@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.

@ciacono
ciacono marked this pull request as ready for review March 25, 2025 17:44
…pt recovered.edits
If an exception happens in the call to finishWriterThreads in the
org.apache.hadoop.hbase.wal.RecoveredEditsOutputSink.close method,
the call to closeWriters should not execute, as it may lead to a race condition
that leads to file corruption if the regionserver aborts. The execution of
closeWriters in this case would write the trailer in parallel with writer threads,
causing corruption, and then the corrupt file would get renamed and finalized
when it should not be. This corruption causes problems when the region is then
to be assigned.
To fix this, when finishWriterThreads throws an exception or is not successful,
the corrupt files should not be renamed and finalized.
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec1m 1sDocker 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.
_ master Compile Tests _
+1 💚mvninstall5m 4smaster passed
+1 💚compile4m 13smaster passed
+1 💚checkstyle0m 53smaster passed
+1 💚spotbugs2m 12smaster passed
+1 💚spotless1m 12sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚mvninstall4m 46sthe patch passed
+1 💚compile4m 32sthe patch passed
+1 💚javac4m 32sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle0m 58sthe patch passed
+1 💚spotbugs2m 18sthe patch passed
+1 💚hadoopcheck20m 54sPatch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚spotless0m 45spatch has no errors when running spotless:check.
_ Other Tests _
+1 💚asflicense0m 10sThe patch does not generate ASF License warnings.
59m 58s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6266/9/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#6266
JIRA IssueHBASE-28569
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
unameLinux 44f24c89b97a 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 revisionmaster / ca8c8a4
Default JavaEclipse Adoptium-17.0.11+9
Max. process+thread count84 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6266/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 🆗reexec0m 37sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed 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 💚mvninstall3m 55smaster passed
+1 💚compile1m 18smaster passed
+1 💚javadoc0m 40smaster passed
+1 💚shadedjars7m 22sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+1 💚mvninstall3m 47sthe patch passed
+1 💚compile1m 6sthe patch passed
+1 💚javac1m 6sthe patch passed
+1 💚javadoc0m 30sthe patch passed
+1 💚shadedjars6m 37spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit242m 53shbase-server in the patch passed.
273m 5s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6266/9/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#6266
JIRA IssueHBASE-28569
Optional Testsjavac javadoc unit compile shadedjars
unameLinux e4587a01c80d 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 revisionmaster / ca8c8a4
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6266/9/testReport/
Max. process+thread count4584 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6266/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.

@aaronbee

Copy link
Copy Markdown
Contributor

LGTM. @Apache9 Would you mind taking another look?

@Apache9

Copy link
Copy Markdown
Contributor

Please open a PR against branch-2 too? I will merge them at once.

Thanks for the fixing! @ciacono@aaronbee

@ciacono

Copy link
Copy Markdown
ContributorAuthor

Thanks for reviewing @Apache9
Please see the PR against branch-2 here: #6884

@Apache9
Apache9 merged commit e2e21f1 into apache:masterApr 7, 2025
Apache9 pushed a commit that referenced this pull request Apr 7, 2025
…t recovered.edits (#6266)
If an exception happens in the call to finishWriterThreads in the
org.apache.hadoop.hbase.wal.RecoveredEditsOutputSink.close method,
the call to closeWriters should not execute, as it may lead to a race condition
that leads to file corruption if the regionserver aborts. The execution of
closeWriters in this case would write the trailer in parallel with writer threads,
causing corruption, and then the corrupt file would get renamed and finalized
when it should not be. This corruption causes problems when the region is then
to be assigned.
To fix this, when finishWriterThreads throws an exception or is not successful,
the corrupt files should not be renamed and finalized.
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit e2e21f1)
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.

5 participants

@ciacono@Apache-HBase@tsuna@aaronbee@Apache9