HBASE-29519 Copy Bulkloaded Files in Continuous Backup #7222

vinayakphegde · 2025-08-13T13:41:05Z

No description provided.

Copilot

Pull Request Overview

This PR implements bulk-loaded file copying functionality for continuous backup in HBase. The feature ensures that bulk-loaded files are replicated and backed up along with WAL entries during continuous backup operations.

Adds bulk load processing capability to extract file paths from WAL entries containing bulk load descriptors
Implements file upload mechanism to copy bulk-loaded files to backup storage
Enhances test coverage for bulk load scenarios and validates backup integrity

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
BulkLoadProcessor.java	New utility class to process WAL entries and extract bulk load file paths
ContinuousBackupReplicationEndpoint.java	Enhanced to handle bulk load file backup during WAL replication
BackupFileSystemManager.java	Added bulk load files directory management
TestBulkLoadProcessor.java	Unit tests for bulk load processing functionality
TestContinuousBackupReplicationEndpoint.java	Updated tests to include bulk load scenarios and verification
FullTableBackupClient.java	Added informational message when bulk load replication is disabled

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-13T17:08:03Z

...e-backup/src/test/java/org/apache/hadoop/hbase/backup/replication/TestBulkLoadProcessor.java

+
+    assertEquals(2, paths.size());
+    System.out.println(paths);
+


Debug print statements should be removed from production test code. Use proper logging if debug output is needed for troubleshooting.

Suggested change

assertEquals(2, paths.size());

Copilot · 2025-08-13T17:08:03Z

...e-backup/src/test/java/org/apache/hadoop/hbase/backup/replication/TestBulkLoadProcessor.java

+
+    assertEquals(2, paths.size());
+    System.out.println(paths);
+


Debug print statements should be removed from production test code. Use proper logging if debug output is needed for troubleshooting.

Suggested change

assertEquals(2, paths.size());

Copilot · 2025-08-13T17:08:04Z

...e-backup/src/test/java/org/apache/hadoop/hbase/backup/replication/TestBulkLoadProcessor.java

+
+    List<Path> paths = BulkLoadProcessor.processBulkLoadFiles(Arrays.asList(entry, entry2));
+    System.out.println(paths);
+


Debug print statements should be removed from production test code. Use proper logging if debug output is needed for troubleshooting.

Suggested change

Copilot · 2025-08-13T17:08:04Z

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java

    backupInfo.setState(BackupState.COMPLETE);
+
+    if (!conf.getBoolean(REPLICATION_BULKLOAD_ENABLE_KEY, false)) {
+      System.out.println("NOTE: Bulkload replication is not enabled. "


Using System.out.println for user notifications in production code is not recommended. Consider using proper logging (LOG.info) or a more appropriate output mechanism for user-facing messages.

Suggested change

System.out.println("NOTE: Bulkload replication is not enabled. "

LOG.warn("NOTE: Bulkload replication is not enabled. "

Copilot · 2025-08-13T17:08:04Z

...ain/java/org/apache/hadoop/hbase/backup/replication/ContinuousBackupReplicationEndpoint.java

      }
+
      walWriter.sync(true);
+      uploadBulkLoadFiles(day, bulkLoadFiles);


The bulk load file upload is performed synchronously for each WAL entry batch. This could become a performance bottleneck if there are many bulk load files. Consider batching or asynchronous processing for better performance.

Yes, we considered and discussed this. We decided to keep it synchronous for now, as making it asynchronous would add complexity and might be a premature optimization. We can revisit this approach if it turns out to be a real performance bottleneck.
Any other thoughts?

I put my questions in the JIRA, let me link here as well, it's mainly about if we do any async upload or upload a list of files at the end of the WAL closing, we will need to be very careful and may have a different implementation. so having current approach may be the best for the moment.

Yeah, going with async has all the issues you mentioned. We’ll keep it as is for now.

taklwu

should we open this PR against apache:HBASE-28957_rebase ?

vinayakphegde · 2025-08-14T05:42:28Z

should we open this PR against apache:HBASE-28957_rebase ?

No need. This branch is rebased by @anmolnar already.

taklwu

we can update the comment for when hbase.replication.bulkload.enabled is disable, for now let's suppose we have instructions for user to manually move related bulkload to the backup location. if you think this should not be supported, we should fail the replication

anmolnar · 2025-08-18T17:06:48Z

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java

+        + "(hbase.replication.bulkload.enabled=true) and configure other necessary settings "
+        + "to properly enable bulkload replication.");


Message is confusing. "You" are HBase, so please tell the user exactly what needs to be done in order to enable bulkload backup.

anmolnar · 2025-08-18T17:08:05Z

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java

+
+    if (!conf.getBoolean(REPLICATION_BULKLOAD_ENABLE_KEY, false)) {
+      System.out.println("NOTE: Bulkload replication is not enabled. "
+        + "Bulk loaded files will not be backed up as part of continuous backup. "


Since continuous backup is using HBase replication, bulk loaded files won't be backup up as part of continuous backup.

anmolnar · 2025-08-18T17:08:28Z

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java

    backupInfo.setState(BackupState.COMPLETE);
+
+    if (!conf.getBoolean(REPLICATION_BULKLOAD_ENABLE_KEY, false)) {
+      System.out.println("NOTE: Bulkload replication is not enabled. "


WARNING: ...

anmolnar · 2025-08-18T17:15:14Z

...ain/java/org/apache/hadoop/hbase/backup/replication/ContinuousBackupReplicationEndpoint.java

+        String errorMsg = String.format("%s Failed to back up bulk load file %s to %s",
+          Utils.logPeerId(peerId), file, destPath);
+        LOG.error("{}: {}", errorMsg, e.getMessage(), e);
+        throw new IOException(errorMsg, e);


Before returning from a failed upload operation you probably want to make sure there was no garbage left at the destination. For instance, if continuing the upload is not supported, the partially uploaded file should be deleted.

Yeah, that makes sense. Since rename isn’t supported in Hadoop-S3, can we do something like
try the copy,
if it fails, attempt to delete the partially uploaded file,
and in case delete also fails, the next retry will always overwrite the file anyway, so we don’t get stuck with stale data.

Sounds good to me. You could attempt another delete here in the catch block. Depending on the failure, you might be able to delete the partially uploaded file right here.

Yeah, we are doing that. missed to mention here.

anmolnar · 2025-08-18T17:17:42Z

...ain/java/org/apache/hadoop/hbase/backup/replication/ContinuousBackupReplicationEndpoint.java

+    if (result == null) {
+      LOG.error("{} No bulk loaded file found in relative path: {}", Utils.logPeerId(peerId),
+        relativePathFromNamespace);
+      throw new IOException(


You might want to throw a different type of exception than what FileUtil.copy(...) operation throws.

…point

anmolnar · 2025-08-19T15:48:14Z

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java

+
+    if (!conf.getBoolean(REPLICATION_BULKLOAD_ENABLE_KEY, false)) {
+      System.out.println("WARNING: Bulkload replication is not enabled. "
+        + "Since continuous backup is using HBase replication, bulk loaded files won't be backup up as part of continuous backup. "


typo: "backed up"

anmolnar · 2025-08-19T16:21:09Z

...ain/java/org/apache/hadoop/hbase/backup/replication/ContinuousBackupReplicationEndpoint.java

+  static void copyWithCleanup(FileSystem srcFS, Path src, FileSystem dstFS, Path dst,
+    Configuration conf) throws IOException {
+    try {
+      if (dstFS.exists(dst)) {


If the file exists and the size (or hash) equals to the file that you're going to upload, you can skip the entire upload process.

Apache-HBase · 2025-08-19T17:56:53Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 29s		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 _
+1 💚	mvninstall	3m 7s		HBASE-28957 passed
+1 💚	compile	0m 33s		HBASE-28957 passed
-0 ⚠️	checkstyle	0m 9s	/buildtool-branch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	0m 30s		HBASE-28957 passed
+1 💚	spotless	0m 47s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 4s		the patch passed
+1 💚	compile	0m 32s		the patch passed
+1 💚	javac	0m 32s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 9s	/buildtool-patch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	0m 38s		the patch passed
+1 💚	hadoopcheck	12m 6s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 47s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 9s		The patch does not generate ASF License warnings.
		30m 39s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7222/5/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7222
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux 2b25405d513c 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 / `75d0d1d`
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	83 (vs. ulimit of 30000)
modules	C: hbase-backup U: hbase-backup
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7222/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 · 2025-08-19T18:12:16Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 52s		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 _
+1 💚	mvninstall	5m 17s		HBASE-28957 passed
+1 💚	compile	0m 38s		HBASE-28957 passed
+1 💚	javadoc	0m 26s		HBASE-28957 passed
+1 💚	shadedjars	8m 4s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 20s		the patch passed
+1 💚	compile	0m 22s		the patch passed
+1 💚	javac	0m 22s		the patch passed
+1 💚	javadoc	0m 15s		the patch passed
+1 💚	shadedjars	6m 10s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	19m 13s		hbase-backup in the patch passed.
		45m 52s

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

anmolnar

lgtm.

vinayakphegde · 2025-08-20T15:14:47Z

Thanks @anmolnar for the review. can we merge if it looks good?

taklwu · 2025-08-20T16:20:18Z

@vinayakphegde I merged it

Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Signed-off-by: Andor Molnár <andor@apache.org>

HBASE-29519: Copy Bulkloaded Files in Continuous Backup

73bf3d3

This comment has been minimized.

Sign in to view

replace the hardcoded configuration

0398116

This comment has been minimized.

Sign in to view

taklwu requested review from anmolnar, Copilot, stoty and taklwu August 13, 2025 17:06

Copilot AI reviewed Aug 13, 2025

View reviewed changes

Remove the debug print lines

b38d8ff

This comment has been minimized.

Sign in to view

taklwu reviewed Aug 13, 2025

View reviewed changes

taklwu approved these changes Aug 14, 2025

View reviewed changes

anmolnar reviewed Aug 18, 2025

View reviewed changes

Enhance the bulkload upload process in ContinuousBackupReplicationEnd…

7e29daf

…point

This comment has been minimized.

Sign in to view

anmolnar requested changes Aug 19, 2025

View reviewed changes

skip already uploaded files

75d0d1d

anmolnar approved these changes Aug 20, 2025

View reviewed changes

taklwu merged commit 7aaf8fb into apache:HBASE-28957 Aug 20, 2025
1 check passed

anmolnar pushed a commit that referenced this pull request Sep 11, 2025

HBASE-29519 Copy Bulkloaded Files in Continuous Backup (#7222)

187f5e7

Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Signed-off-by: Andor Molnár <andor@apache.org>

anmolnar pushed a commit that referenced this pull request Nov 6, 2025

HBASE-29519 Copy Bulkloaded Files in Continuous Backup (#7222)

176e8c6

Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Signed-off-by: Andor Molnár <andor@apache.org>


		List<Path> paths = BulkLoadProcessor.processBulkLoadFiles(Arrays.asList(entry, entry2));
		System.out.println(paths);

	System.out.println("NOTE: Bulkload replication is not enabled. "
	LOG.warn("NOTE: Bulkload replication is not enabled. "

		+ "(hbase.replication.bulkload.enabled=true) and configure other necessary settings "
		+ "to properly enable bulkload replication.");

HBASE-29519 Copy Bulkloaded Files in Continuous Backup #7222

HBASE-29519 Copy Bulkloaded Files in Continuous Backup #7222

Uh oh!

Conversation

vinayakphegde commented Aug 13, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

vinayakphegde commented Aug 14, 2025

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Aug 19, 2025

Uh oh!

Apache-HBase commented Aug 19, 2025

Uh oh!

anmolnar left a comment

Choose a reason for hiding this comment

Uh oh!

vinayakphegde commented Aug 20, 2025

Uh oh!

Uh oh!

taklwu commented Aug 20, 2025

Uh oh!