HBASE-29449 Update backup describe command for continuous backup #7045

ankitsol · 2025-05-28T05:21:43Z

This PR is to update "hbase backup describe" command for continuous backup. Here I have add continuousEnabled field in backupInfo proto which is returned when user runs this command

JIRA https://issues.apache.org/jira/browse/HBASE-29449

NihalJain · 2025-06-11T16:32:48Z

I see a few issues around resource/connection cleanup/reset etc. in the new test, lets see if copilot catches them.

Copilot

Pull Request Overview

This PR updates the hbase backup describe command to include a flag indicating whether continuous backup was enabled for a given backup run.

Added a new continuousBackupEnabled field to the BackupInfo protobuf definition.
Extended BackupInfo Java class to set/read the new field and include it in the short description.
Added a new unit test exercising the describe command for a continuous backup scenario.

Reviewed Changes

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

File	Description
hbase-protocol-shaded/src/main/protobuf/Backup.proto	Added `optional bool continuousBackupEnabled = 14`.
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java	Populated protobuf builder, parser, and short description with the new flag.
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java	Added `testBackupDescribeCommandForContinuousBackup` to verify the CLI output.

Comments suppressed due to low confidence (1)

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java:116

[nitpick] Consider adding a complementary test for the non-continuous case (e.g., without -enableContinuousBackup) to verify that the describe output shows IsContinuous=false.

@Test
  public void testBackupDescribeCommandForContinuousBackup() throws Exception {

Copilot · 2025-06-11T16:33:06Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+    System.setOut(new PrintStream(baos));
+
+    String[] args = new String[] { "describe", backupId };
+    // Run backup
+    ret = ToolRunner.run(conf1, new BackupDriver(), args);
+    assertTrue(ret == 0);
+    String response = baos.toString();
+    assertTrue(response.indexOf(backupId) > 0);
+    assertTrue(response.indexOf("COMPLETE") > 0);
+    assertTrue(response.indexOf("IsContinuous=true") > 0);
+
+    BackupInfo status = table.readBackupInfo(backupId);
+    String desc = status.getShortDescription();
+    table.close();
+    assertTrue(response.indexOf(desc) >= 0);
+


Capture the original System.out (e.g., PrintStream original = System.out) and restore it in a finally block or @after method to avoid side effects on other tests.

Suggested change

System.setOut(new PrintStream(baos));

String[] args = new String[] { "describe", backupId };

// Run backup

ret = ToolRunner.run(conf1, new BackupDriver(), args);

assertTrue(ret == 0);

String response = baos.toString();

assertTrue(response.indexOf(backupId) > 0);

assertTrue(response.indexOf("COMPLETE") > 0);

assertTrue(response.indexOf("IsContinuous=true") > 0);

BackupInfo status = table.readBackupInfo(backupId);

String desc = status.getShortDescription();

table.close();

assertTrue(response.indexOf(desc) >= 0);

PrintStream originalOut = System.out; // Capture the original System.out

System.setOut(new PrintStream(baos));

try {

String[] args = new String[] { "describe", backupId };

// Run backup

ret = ToolRunner.run(conf1, new BackupDriver(), args);

assertTrue(ret == 0);

String response = baos.toString();

assertTrue(response.indexOf(backupId) > 0);

assertTrue(response.indexOf("COMPLETE") > 0);

assertTrue(response.indexOf("IsContinuous=true") > 0);

BackupInfo status = table.readBackupInfo(backupId);

String desc = status.getShortDescription();

table.close();

assertTrue(response.indexOf(desc) >= 0);

} finally {

System.setOut(originalOut); // Restore the original System.out

}

Copilot · 2025-06-11T16:33:06Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+    String response = baos.toString();
+    assertTrue(response.indexOf(backupId) > 0);
+    assertTrue(response.indexOf("COMPLETE") > 0);
+    assertTrue(response.indexOf("IsContinuous=true") > 0);


Use a non‐positional check like assertTrue(response.contains("IsContinuous=true")) or at least >= 0 to correctly assert presence even at the start of the string.

Suggested change

assertTrue(response.indexOf("IsContinuous=true") > 0);

assertTrue(response.contains("IsContinuous=true"));

NihalJain · 2025-06-11T16:35:47Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+  @Test
+  public void testBackupDescribeCommandForContinuousBackup() throws Exception {
+    LOG.info("test backup describe on a single table with data: command-line");
+    BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection());


nit: try-with-resources

NihalJain · 2025-06-11T16:36:13Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+    assertTrue(response.indexOf(desc) >= 0);
+
+    // cleanup
+    if (fs.exists(backupWalDir)) {


put this in finally

NihalJain · 2025-06-11T16:43:58Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+      table1.getNameAsString(), "-" + OPTION_ENABLE_CONTINUOUS_BACKUP };
+    int ret = ToolRunner.run(conf1, new BackupDriver(), backupArgs);
+    assertEquals("Backup should succeed", 0, ret);
+    String backupId = table.getBackupHistory().get(0).getBackupId();


ensure this is empty before we start the test or maybe take the index + 1 if otherwise

kgeisz

LGTM overall. I noticed HBASE-28994 mentions investigating changes for commands like merge, progress, etc. in addition to describe. Since this PR only makes changes for describe, I'm wondering if we should change the description of the ticket or at least make this a child-task of HBASE-28994.

kgeisz · 2025-06-27T02:16:35Z

Btw, I am also having trouble with TestIncrementalBackup.TestIncBackupRestoreWithOriginalSplits() and TestIncrementalBackup.TestIncBackupRestore() failing when I run locally on this branch. I know these have already caused problems in the past. I'm still looking into why exactly they are occurring.

taklwu

in the JIRA https://issues.apache.org/jira/browse/HBASE-28994 , it said it may need to update more than one command, and here is backup describe.

do we need to modify any further ?

taklwu · 2025-06-27T18:29:19Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+      assertTrue(ret == 0);
+      String response = baos.toString();
+      assertTrue(response.indexOf(backupId) > 0);
+      assertTrue(response.indexOf("COMPLETE") > 0);


nit: will we have more than one COMPLETE in the response? if not why don't check the occurrence of COMPLETE ?

taklwu · 2025-06-27T18:31:52Z

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java

+      assertTrue(response.indexOf("COMPLETE") > 0);
+      assertTrue(response.contains("IsContinuous=true"));
+
+      BackupInfo status = table.readBackupInfo(backupId);


for the incr_committed_wal_ts and message of Committed WAL time for incremental backup= , is it possible to add a new assertion ?

Makes sense, added now

ankitsol · 2025-07-10T15:25:00Z

Created subtask https://issues.apache.org/jira/browse/HBASE-29449 for "hbase backup describe" command

Apache-HBase · 2025-07-10T16:15:36Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 48s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+0 🆗	buf	0m 1s		buf was not available.
+0 🆗	buf	0m 1s		buf 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 37s		Maven dependency ordering for branch
+1 💚	mvninstall	4m 18s		HBASE-28957 passed
+1 💚	compile	1m 33s		HBASE-28957 passed
-0 ⚠️	checkstyle	0m 13s	/buildtool-branch-checkstyle-hbase-backup.txt	The patch fails to run checkstyle in hbase-backup
+1 💚	spotbugs	3m 33s		HBASE-28957 passed
+1 💚	spotless	0m 54s		branch has no errors when running spotless:check.
-0 ⚠️	patch	1m 8s		Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 41s		the patch passed
+1 💚	compile	1m 23s		the patch passed
+1 💚	cc	1m 23s		the patch passed
-0 ⚠️	javac	0m 38s	/results-compile-javac-hbase-backup.txt	hbase-backup generated 2 new + 116 unchanged - 0 fixed = 118 total (was 116)
+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	3m 25s		the patch passed
+1 💚	hadoopcheck	13m 16s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	hbaseprotoc	1m 1s		the patch passed
+1 💚	spotless	0m 46s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 18s		The patch does not generate ASF License warnings.
		44m 21s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7045/7/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#7045
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc
uname	Linux 628f586a8599 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 / `b16ebee`
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	85 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded hbase-backup U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7045/7/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-10T16:17:48Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 34s		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 _
+0 🆗	mvndep	0m 9s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 17s		HBASE-28957 passed
+1 💚	compile	0m 52s		HBASE-28957 passed
+1 💚	javadoc	0m 24s		HBASE-28957 passed
+1 💚	shadedjars	6m 3s		branch has no errors when building our shaded downstream artifacts.
-0 ⚠️	patch	6m 19s		Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 13s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 5s		the patch passed
+1 💚	compile	0m 52s		the patch passed
+1 💚	javac	0m 52s		the patch passed
+1 💚	javadoc	0m 23s		the patch passed
+1 💚	shadedjars	6m 2s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	0m 33s		hbase-protocol-shaded in the patch passed.
-1 ❌	unit	23m 4s	/patch-unit-hbase-backup.txt	hbase-backup in the patch failed.
		46m 39s

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

the same errors for javac and checkstyle were complaining here and #7150 , do you wanna come up another JIRA to fix it ? or could you fix it in this PR or #7150 ?

Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> Reviewed by: Kevin Geiszler <[email protected]>

…che#7045) Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> Reviewed by: Kevin Geiszler <[email protected]>

Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> Reviewed by: Kevin Geiszler <[email protected]>

HBASE-28994 Enhance backup describe command

bbeb823

This comment has been minimized.

Sign in to view

Merge remote-tracking branch 'hbase/HBASE-28957' into as.HBASE-28994

bb64278

This comment has been minimized.

Sign in to view

NihalJain requested a review from Copilot June 11, 2025 16:31

Copilot AI reviewed Jun 11, 2025

View reviewed changes

NihalJain reviewed Jun 11, 2025

View reviewed changes

Added incrCommittedWalTs

7222ba0

This comment has been minimized.

Sign in to view

Merge remote-tracking branch 'hbase/HBASE-28957' into as.HBASE-28994

5cb1a98

This comment has been minimized.

Sign in to view

Minor fix

58a57dc

This comment has been minimized.

Sign in to view

Minor fix

e641482

This comment has been minimized.

Sign in to view

kgeisz approved these changes Jun 27, 2025

View reviewed changes

taklwu requested changes Jun 27, 2025

View reviewed changes

asolomonC added 2 commits July 10, 2025 09:26

Merge remote-tracking branch 'hbase/HBASE-28957' into as.HBASE-28994

a9e15cc

Address review comments

b16ebee

ankitsol changed the title ~~HBASE-28994 Update backup describe command for continuous backup~~ HBASE-29449 Update backup describe command for continuous backup Jul 10, 2025

ankitsol requested a review from taklwu July 10, 2025 15:25

taklwu approved these changes Jul 14, 2025

View reviewed changes

taklwu merged commit 6d38775 into apache:HBASE-28957 Jul 15, 2025
1 check failed

anmolnar pushed a commit that referenced this pull request Jul 28, 2025

HBASE-29449 Update backup describe command for continuous backup (#7045)

39f1da8

Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> Reviewed by: Kevin Geiszler <[email protected]>

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

HBASE-29449 Update backup describe command for continuous backup (#7045)

9089e4a

Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> Reviewed by: Kevin Geiszler <[email protected]>

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

HBASE-29449 Update backup describe command for continuous backup (#7045)

27ea7b3

Signed-off-by: Tak Lon (Stephen) Wu <[email protected]> Reviewed by: Kevin Geiszler <[email protected]>

	assertTrue(response.indexOf("IsContinuous=true") > 0);
	assertTrue(response.contains("IsContinuous=true"));

HBASE-29449 Update backup describe command for continuous backup #7045

HBASE-29449 Update backup describe command for continuous backup #7045

Uh oh!

Conversation

ankitsol commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

NihalJain commented Jun 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

NihalJain Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

NihalJain Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

NihalJain Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kgeisz left a comment

Choose a reason for hiding this comment

Uh oh!

kgeisz commented Jun 27, 2025

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

taklwu Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

ankitsol Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

taklwu Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

ankitsol Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

ankitsol commented Jul 10, 2025

Uh oh!

Apache-HBase commented Jul 10, 2025

Uh oh!

Apache-HBase commented Jul 10, 2025

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ankitsol commented May 28, 2025 •

edited

Loading

NihalJain Jun 11, 2025 •

edited

Loading