Skip to content

HBASE-28850 Only return from ReplicationSink.replicationEntries while… - #6263

Merged
Apache9 merged 1 commit into
apache:masterfrom
Apache9:HBASE-28850
Sep 19, 2024
Merged

HBASE-28850 Only return from ReplicationSink.replicationEntries while…#6263
Apache9 merged 1 commit into
apache:masterfrom
Apache9:HBASE-28850

Conversation

@Apache9

Copy link
Copy Markdown
Contributor

… all background tasks are finished

@Apache9Apache9 self-assigned this Sep 18, 2024
@Apache-HBase

This comment has been minimized.

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

The trade off is this may change the performance of replicateEntries. Before, the replicateEntries call will fail fast at the first failure. After, the replicateEntries will block for the entire time it takes to confirm all local edits in the batch are applied or failed.

I have a testbed where I reproduced SEGV crashes for HBASE-28584. Let me apply this patch there and try it out.

@Apache-HBase

This comment has been minimized.

if (error == null) {
error = ioe;
} else {
error.addSuppressed(ioe);

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.

Logging the exception before it is suppressed might be helpful? (unless suppressed exceptions are all logged with original exception, which i don't recall if it happens)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

 public static void main(String[] args) {
Exception error = new Exception("root");
error.addSuppressed(new IOException("suppressed1"));
error.addSuppressed(new IOException("suppressed2"));
error.printStackTrace();
}

The output

java.lang.Exception: root
at org.apache.hadoop.hbase.regionserver.regionreplication.RegionReplicationSink.main(RegionReplicationSink.java:461)
Suppressed: java.io.IOException: suppressed1
at org.apache.hadoop.hbase.regionserver.regionreplication.RegionReplicationSink.main(RegionReplicationSink.java:462)
Suppressed: java.io.IOException: suppressed2
at org.apache.hadoop.hbase.regionserver.regionreplication.RegionReplicationSink.main(RegionReplicationSink.java:463)

So I think it is fine to not log it here?

@virajjasani

Copy link
Copy Markdown
Contributor

Just a minor comment above, +1 otherwise. Had an offline chat with @apurtell also reg this!

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

The change looks good. I rehydrated the testbed where I was able to reproduce SEGVs and everything has been stable and performing normally after 250M rows replicated on the way up to 1B. Not expecting any issues.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 28sDocker 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 💚mvninstall2m 53smaster passed
+1 💚compile3m 2smaster passed
+1 💚checkstyle0m 34smaster passed
+1 💚spotbugs1m 30smaster passed
+1 💚spotless0m 44sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚mvninstall3m 48sthe patch passed
+1 💚compile3m 28sthe patch passed
+1 💚javac3m 28sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle0m 45sthe patch passed
+1 💚spotbugs1m 53sthe patch passed
+1 💚hadoopcheck11m 59sPatch 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 9sThe patch does not generate ASF License warnings.
38m 45s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6263/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#6263
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
unameLinux d903d5c341b7 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 / adc9fcc
Default JavaEclipse Adoptium-17.0.11+9
Max. process+thread count85 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6263/2/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 38sDocker mode activated.
-0 ⚠️yetus0m 4sUnprocessed 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 6smaster passed
+1 💚compile0m 58smaster passed
+1 💚javadoc0m 30smaster passed
+1 💚shadedjars5m 15sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+1 💚mvninstall2m 56sthe patch passed
+1 💚compile0m 58sthe patch passed
+1 💚javac0m 58sthe patch passed
+1 💚javadoc0m 30sthe patch passed
+1 💚shadedjars5m 16spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚unit237m 30shbase-server in the patch passed.
261m 46s
SubsystemReport/Notes
DockerClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6263/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#6263
Optional Testsjavac javadoc unit compile shadedjars
unameLinux 2e2396adbb2b 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / adc9fcc
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6263/2/testReport/
Max. process+thread count4637 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6263/2/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Apache9 merged commit 52082bc into apache:masterSep 19, 2024
Apache9 added a commit that referenced this pull request Sep 19, 2024
… all background tasks are finished (#6263)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit 52082bc)
Apache9 added a commit to Apache9/hbase that referenced this pull request Sep 19, 2024
… all background tasks are finished (apache#6263)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit 52082bc)
Apache9 added a commit that referenced this pull request Sep 19, 2024
… all background tasks are finished (#6263) (#6271)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit 52082bc)
Apache9 added a commit that referenced this pull request Sep 19, 2024
… all background tasks are finished (#6263) (#6271)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit 52082bc)
(cherry picked from commit 0dc334f)
Apache9 added a commit that referenced this pull request Sep 19, 2024
… all background tasks are finished (#6263) (#6271)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit 52082bc)
(cherry picked from commit 0dc334f)
sanjeet006py pushed a commit to sanjeet006py/hbase that referenced this pull request Sep 26, 2025
… all background tasks are finished (apache#6263) (apache#6271)
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit 52082bc)
(cherry picked from commit 0dc334f)
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.

4 participants

@Apache9@Apache-HBase@virajjasani@apurtell