HBASE-29441 ReplicationSourceShipper should delegate the empty wal entries handling to ReplicationEndpoint

[vinayakphegde]

No description provided.

[Copilot]

Pull Request Overview

This PR delegates empty WAL entry handling in ReplicationSourceShipper to each ReplicationEndpoint via a new EmptyEntriesPolicy enum and updates related code and tests.

• Introduce EmptyEntriesPolicy and getEmptyEntriesPolicy() in ReplicationEndpoint with a default of COMMIT.
• Update ReplicationSourceShipper.shipEdits() to use endpoint policy for empty batches and add helper getReplicationResult().
• Add sync and async endpoint implementations and corresponding tests; update ContinuousBackupReplicationEndpoint to return SUBMIT.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSource.java	Added AsyncReplicationEndpoint and SyncReplicationEndpoint tests for empty-batch behavior.
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java	Changed shipEdits visibility, delegated empty-batch logic using endpoint policy, and added getReplicationResult().
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/ReplicationEndpoint.java	Added default getEmptyEntriesPolicy() method and documentation.
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/EmptyEntriesPolicy.java	Introduced enum to distinguish COMMIT vs. SUBMIT for empty batches.
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/replication/ContinuousBackupReplicationEndpoint.java	Override getEmptyEntriesPolicy() to return SUBMIT for async backup.

Comments suppressed due to low confidence (3)

hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java:244

• [nitpick] The method name getReplicationResult is generic; consider renaming to getEmptyBatchReplicationResult or computeEmptyEntriesPolicyResult for clarity.

  private ReplicationResult getReplicationResult() {

hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java:278

• [nitpick] Document the return value for updateLogPosition, explaining what true or false indicates about the update operation.

  boolean updateLogPosition(WALEntryBatch batch, ReplicationResult replicated) {

hbase-server/src/main/java/org/apache/hadoop/hbase/replication/EmptyEntriesPolicy.java:23

• [nitpick] The Javadoc bullet list in this enum is formatted inline; consider using standard Javadoc list tags (<ul><li>) or separate lines for each policy for better readability.

 * Policy that defines what a replication endpoint should do when the entry batch is empty. This is

[Copilot]

[nitpick] Consider annotating this package-private method with @VisibleForTesting (or a project-specific equivalent) to clearly signal its test-only visibility.

[taklwu]

nit: VisibleForTesting seems the right comment, and it may save few words for testing only,

[taklwu]

LGTM

[taklwu]

nit: VisibleForTesting seems the right comment, and it may save few words for testing only,

[taklwu]

VisibleForTesting?

[vinayakphegde]

Got it.
I think we are not allowed to use @VisibleForTesting. I don't see any other instance in the codebase as well.
also, I see this

<restrictImports implementation="de.skuzzle.enforcer.restrictimports.rule.RestrictImports">
        <includeTestCode>true</includeTestCode>
        <commentLineBufferSize>512</commentLineBufferSize>
        <reason>You should never use this style of annotations(i.e, 'this is for test only')
          in IA.Public or IA.LimitedPrivate classes. Use IA.Private to tell users this is
          not for public use.
          For IA.Private classes, use RestrictedApi annotation in error prone instead.</reason>
        <bannedImports>
          <bannedImport>org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting</bannedImport>
        </bannedImports>
</restrictImports>

[Apache-HBase]

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	2m 59s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ HBASE-28957 Compile Tests _
+0 🆗	mvndep	0m 19s		Maven dependency ordering for branch
+1 💚	mvninstall	2m 59s		HBASE-28957 passed
+1 💚	compile	3m 30s		HBASE-28957 passed
-0 ⚠️	checkstyle	0m 8s	/buildtool-branch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	1m 58s		HBASE-28957 passed
+1 💚	spotless	0m 43s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 2s		the patch passed
+1 💚	compile	3m 41s		the patch passed
+1 💚	javac	3m 41s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 36s	/results-checkstyle-hbase-server.txt	hbase-server: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
-0 ⚠️	checkstyle	0m 9s	/buildtool-patch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	2m 13s		the patch passed
+1 💚	hadoopcheck	11m 45s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 43s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 17s		The patch does not generate ASF License warnings.
		43m 42s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7145/5/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7145
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux 0f35249e7853 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	HBASE-28957 / be71bb0
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	85 (vs. ulimit of 30000)
modules	C: hbase-server hbase-backup U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7145/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.

[Apache-HBase]

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 51s		Docker mode activated.
-0 ⚠️	yetus	0m 4s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ HBASE-28957 Compile Tests _
+0 🆗	mvndep	0m 20s		Maven dependency ordering for branch
+1 💚	mvninstall	4m 31s		HBASE-28957 passed
+1 💚	compile	2m 4s		HBASE-28957 passed
+1 💚	javadoc	1m 2s		HBASE-28957 passed
+1 💚	shadedjars	8m 49s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 19s		Maven dependency ordering for patch
+1 💚	mvninstall	4m 29s		the patch passed
+1 💚	compile	2m 1s		the patch passed
+1 💚	javac	2m 1s		the patch passed
+1 💚	javadoc	1m 12s		the patch passed
+1 💚	shadedjars	7m 53s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
-1 ❌	unit	346m 51s	/patch-unit-hbase-server.txt	hbase-server in the patch failed.
+1 💚	unit	37m 6s		hbase-backup in the patch passed.
		433m 18s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7145/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#7145
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux a2fa7fe8d801 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	HBASE-28957 / be71bb0
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7145/5/testReport/
Max. process+thread count	4394 (vs. ulimit of 30000)
modules	C: hbase-server hbase-backup U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7145/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.