HBASE-29445 Add Option to Specify Custom Backup Location in PITR #7153

vinayakphegde · 2025-07-14T09:23:04Z

No description provided.

Copilot

Pull Request Overview

Adds support for specifying a custom backup root directory when performing Point-In-Time Restore (PITR).

Introduces a PitrBackupMetadata abstraction with adapters for system‐table and custom‐location backups.
Refactors BackupAdminImpl and core PITR flow into AbstractPitrRestoreHandler with two handlers: default vs. custom location.
Updates tests to use a new PITRTestUtil helper and adds an end‐to‐end test for custom backup paths.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestPointInTimeRestoreWithCustomBackupPath.java	New integration test for PITR with a custom backup location
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestPointInTimeRestore.java	Updated existing PITR tests to use `PITRTestUtil` and null path
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/PITRTestUtil.java	New utility for building backup/PITR args and common test ops
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/PitrBackupMetadata.java	Defines unified interface for backup metadata in PITR
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/DefaultPitrRestoreHandler.java	Handler for PITR using the system backup table
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/CustomBackupLocationPitrRestoreHandler.java	Handler for PITR using a custom backup directory
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupInfoAdapter.java	Adapter to expose `BackupInfo` as `PitrBackupMetadata`
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupImageAdapter.java	Adapter to expose `BackupImage` as `PitrBackupMetadata`
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java	Delegates `pointInTimeRestore` to the new handlers
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/AbstractPitrRestoreHandler.java	Core PITR logic refactored to use handler/metadata abstraction

Copilot · 2025-07-15T18:52:24Z

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

+ */
+package org.apache.hadoop.hbase.backup.impl;
+
+import static java.awt.SystemColor.info;


Remove the static import of java.awt.SystemColor.info; it’s unused and causes the wrong identifier to be passed into the LOG.warn call. The code should reference the current backup variable (e.g., 'backup') instead.

Suggested change

import static java.awt.SystemColor.info;

please remove this

Copilot · 2025-07-15T18:52:25Z

...src/test/java/org/apache/hadoop/hbase/backup/TestPointInTimeRestoreWithCustomBackupPath.java

+    Path defaultBackupDir = new Path(BACKUP_ROOT_DIR);
+    for (FileStatus status : fs.listStatus(defaultBackupDir)) {
+      Path dst = new Path(customBackupDir, status.getPath().getName());
+      FileUtil.copy(fs, status, fs, dst, true, false, conf1);


The deleteSource flag is set to true, which removes the original backup files after copying. Change it to false to preserve the source backup directory.

Suggested change

FileUtil.copy(fs, status, fs, dst, true, false, conf1);

FileUtil.copy(fs, status, fs, dst, false, false, conf1);

ignore this comment, because you're trying to create the data in the customBackupDirectory.

taklwu

LGTM, please fix the unused import

nit: the title of this JIRA is to add the --backup-path in PITR, but this option was introduced in the previous commit, do we miss anything?

should the title of this JIRA be "Implement custom backup location in PITR "?

vinayakphegde · 2025-07-16T05:57:12Z

LGTM, please fix the unused import

nit: the title of this JIRA is to add the --backup-path in PITR, but this option was introduced in the previous commit, do we miss anything?

should the title of this JIRA be "Implement custom backup location in PITR "?

Sure, I will fix the import.
Yeah, I introduced the option in the previous commit only. but didn't implement the actual code. since the PR was too big.
Regarding the jira header, I changed that now.

Apache-HBase · 2025-07-16T06:35:50Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	2m 53s		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 35s		HBASE-28957 passed
+1 💚	compile	0m 35s		HBASE-28957 passed
-0 ⚠️	checkstyle	0m 11s	/buildtool-branch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	0m 36s		HBASE-28957 passed
+1 💚	spotless	0m 50s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 23s		the patch passed
+1 💚	compile	0m 35s		the patch passed
+1 💚	javac	0m 35s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 10s	/buildtool-patch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	0m 41s		the patch passed
+1 💚	hadoopcheck	12m 31s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	spotless	0m 48s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 10s		The patch does not generate ASF License warnings.
		34m 54s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7153/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7153
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname	Linux f75e20f3ca00 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 / `835cf42`
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-7153/2/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-07-16T06:46:48Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	2m 52s		Docker mode activated.
-0 ⚠️	yetus	0m 3s		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	2m 52s		HBASE-28957 passed
+1 💚	compile	0m 18s		HBASE-28957 passed
+1 💚	javadoc	0m 14s		HBASE-28957 passed
+1 💚	shadedjars	5m 49s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	2m 54s		the patch passed
+1 💚	compile	0m 18s		the patch passed
+1 💚	javac	0m 18s		the patch passed
+1 💚	javadoc	0m 13s		the patch passed
+1 💚	shadedjars	5m 45s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
-1 ❌	unit	23m 33s	/patch-unit-hbase-backup.txt	hbase-backup in the patch failed.
		45m 55s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7153/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#7153
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux 26075ec1912d 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 / `835cf42`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7153/2/testReport/
Max. process+thread count	3621 (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-7153/2/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.

taklwu

Lgtm

taklwu · 2025-07-16T17:15:55Z

LGTM, please fix the unused import
nit: the title of this JIRA is to add the --backup-path in PITR, but this option was introduced in the previous commit, do we miss anything?
should the title of this JIRA be "Implement custom backup location in PITR "?

Sure, I will fix the import. Yeah, I introduced the option in the previous commit only. but didn't implement the actual code. since the PR was too big. Regarding the jira header, I changed that now.

don't worry about the JIRA title, I merged it already and forgot to change it, so keep it as is.

HBASE-29445: Add Option to Specify Custom Backup Location in PITR

8ee4abb

This comment has been minimized.

Sign in to view

taklwu requested a review from Copilot July 15, 2025 18:49

Copilot AI reviewed Jul 15, 2025

View reviewed changes

taklwu reviewed Jul 15, 2025

View reviewed changes

remove unused imports

835cf42

taklwu approved these changes Jul 16, 2025

View reviewed changes

taklwu merged commit a13b5f9 into apache:HBASE-28957 Jul 16, 2025
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HBASE-29445 Add Option to Specify Custom Backup Location in PITR #7153

HBASE-29445 Add Option to Specify Custom Backup Location in PITR #7153

Uh oh!

vinayakphegde commented Jul 14, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

taklwu Jul 15, 2025

Uh oh!

Copilot AI Jul 15, 2025

Uh oh!

taklwu Jul 15, 2025

Uh oh!

taklwu left a comment

Uh oh!

vinayakphegde commented Jul 16, 2025

Uh oh!

Apache-HBase commented Jul 16, 2025

Uh oh!

Apache-HBase commented Jul 16, 2025

Uh oh!

taklwu left a comment

Uh oh!

Uh oh!

taklwu commented Jul 16, 2025

Uh oh!

Uh oh!

	FileUtil.copy(fs, status, fs, dst, true, false, conf1);
	FileUtil.copy(fs, status, fs, dst, false, false, conf1);

HBASE-29445 Add Option to Specify Custom Backup Location in PITR #7153

HBASE-29445 Add Option to Specify Custom Backup Location in PITR #7153

Uh oh!

Conversation

vinayakphegde commented Jul 14, 2025

Uh oh!

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 Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

taklwu Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

taklwu Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

vinayakphegde commented Jul 16, 2025

Uh oh!

Apache-HBase commented Jul 16, 2025

Uh oh!

Apache-HBase commented Jul 16, 2025

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

taklwu commented Jul 16, 2025

Uh oh!

Uh oh!