Skip to content

HBASE-28584 RS SIGSEGV under heavy replication load - #6124

Closed
apurtell wants to merge 1 commit into
apache:masterfrom
apurtell:HBASE-28584
Closed

HBASE-28584 RS SIGSEGV under heavy replication load#6124
apurtell wants to merge 1 commit into
apache:masterfrom
apurtell:HBASE-28584

Conversation

@apurtell

@apurtellapurtell commented Jul 28, 2024

Copy link
Copy Markdown
Contributor

Clone the cells that are used to apply mutations on the local cluster. Some operations may still be in flight even as we fail to apply some other in-flight mutations and trigger failure handling including a release of the buffer underlying the cellScanner that is sourcing the cells.

Deep clone the cells that are used to apply mutations on the local cluster.
Some operations may still be in flight even as we fail to apply some other
in-flight mutations and trigger failure handling including a release of the
buffer underlying the cellScanner that is sourcing the cells.
@apurtell

Copy link
Copy Markdown
ContributorAuthor

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 41sDocker 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 _
+0 🆗mvndep0m 17sMaven dependency ordering for branch
+1 💚mvninstall4m 22smaster passed
+1 💚compile4m 30smaster passed
+1 💚checkstyle1m 8smaster passed
+1 💚spotbugs2m 42smaster passed
+1 💚spotless0m 59sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗mvndep0m 11sMaven dependency ordering for patch
+1 💚mvninstall3m 36sthe patch passed
+1 💚compile4m 16sthe patch passed
+1 💚javac4m 16sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle1m 4sthe patch passed
+1 💚spotbugs2m 46sthe patch passed
+1 💚hadoopcheck12m 56sPatch 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 21sThe patch does not generate ASF License warnings.
48m 41s
SubsystemReport/Notes
DockerClientAPI=1.46 ServerAPI=1.46 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6124/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#6124
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
unameLinux 30055f172d95 5.4.0-182-generic #202-Ubuntu SMP Fri Apr 26 12:29:36 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e0a3162
Default JavaEclipse Adoptium-17.0.11+9
Max. process+thread count84 (vs. ulimit of 30000)
modulesC: hbase-common hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6124/1/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.

@Apache9Apache9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in to fix the crash issue first.

IIRC, we have a similar problem when async wal is enabled. The rpc call will finish before we actually finish the WAL writing and cause we write out corrupt wal entries.

Let me check how we deal with the problem there. I guess the same trick can be used here too.

Thanks @apurtell for the analyzing.

@Apache9

Copy link
Copy Markdown
Contributor

OK, in ServerCall class, we have a retainByWAL method, where we will count the extra references of the ServerCall, mainly the CellScanners.

I think we can just change the method to retain, which means we want to retain it for other usage even after the rpc call is done, and also use it here.

Anyway, since the current PR has been well tested in producation, I think we can apply it first, and open another issue for optimizing.

Thanks.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec4m 11sDocker 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 _
+0 🆗mvndep0m 9sMaven dependency ordering for branch
+1 💚mvninstall4m 33smaster passed
+1 💚compile1m 43smaster passed
+1 💚javadoc1m 15smaster passed
+1 💚shadedjars6m 12sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗mvndep0m 13sMaven dependency ordering for patch
+1 💚mvninstall3m 0sthe patch passed
+1 💚compile1m 21sthe patch passed
+1 💚javac1m 21sthe patch passed
+1 💚javadoc0m 47sthe patch passed
+1 💚shadedjars5m 20spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit2m 29shbase-common in the patch passed.
+1 💚unit229m 24shbase-server in the patch passed.
265m 42s
SubsystemReport/Notes
DockerClientAPI=1.46 ServerAPI=1.46 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6124/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#6124
Optional Testsjavac javadoc unit compile shadedjars
unameLinux 265f2a29681d 5.4.0-177-generic #197-Ubuntu SMP Thu Mar 28 22:45:47 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / e0a3162
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6124/1/testReport/
Max. process+thread count5172 (vs. ulimit of 30000)
modulesC: hbase-common hbase-server U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6124/1/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@virajjasanivirajjasani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@virajjasanivirajjasani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit though

Comment on lines +2265 to +2267
* Deep clones the given cell if the cell supports deep cloning
* @param cell the cell to be cloned
* @return the cloned cell

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: javadoc to include @throws

@Apache9

Copy link
Copy Markdown
Contributor

Any updates here?

Thanks.

@Apache9

Copy link
Copy Markdown
Contributor

I think we should fix this in newer releases.

If you are all OK, I could try to implement the reference counting way to solve the problem.

@apurtell@virajjasani Thoughts?

Thanks.

@apurtell

Copy link
Copy Markdown
ContributorAuthor

I thought refcounting would be complex but am not opposed to it as a different solution. When and if we have that we could remove the copying.

@apurtell

Copy link
Copy Markdown
ContributorAuthor

We would not need this change if #6263 solves the problem instead.

@apurtell

Copy link
Copy Markdown
ContributorAuthor

Fixed by #6263

@apurtell
apurtell deleted the HBASE-28584 branch September 20, 2024 00:13
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.

6 participants

@apurtell@Apache-HBase@Apache9@mnpoonia@virajjasani@d-c-manning