Skip to content

Conversation

@kgeisz
Copy link
Contributor

@kgeisz kgeisz commented Oct 27, 2025

https://issues.apache.org/jira/browse/HBASE-29687

The original IntegrationTestBackupRestore.java class tests the functionality of full and incremental backups. It creates a full backup, and then iteratively performs an incremental backup plus multiple restores. Later, it merges the incremental backups and performs a final restore.

With the introduction of the continuous backup feature, this pull request extends IntegrationTestBackupRestore to also handle testing full and incremental backups with continuous backup enabled. This change turns IntegrationTestBackupRestore into an abstract base class. Much of the functionality within this new IntegrationTestBackupRestoreBase class is the same. Several new test cases have been added that are only performed for continuous backups (backup WAL directory verification, backup WAL file verification, etc.). In addition, snapshot verification and backup deletion has been added for both the continuous and the non-continuous test case. The existing testBackupRestore() method has been moved to its own IntegrationTestBackupRestore.java class that extends the new base class. Similarly, a new IntegrationTestContinuousBackupRestore.java test class has been added for the continuous backup case.

Note: The continuous backup test case only allows one region server per minicluster. When multiple region servers are used, there are issues with waiting for all of the region servers to catch up with replication. This results in only most of the rows being restored when a backup restore is performed, regardless of how long the test waits for.

There are some additional changes made to BackupTestUtil.java and TestContinuousBackup.java that allow IntegrationTestBackupRestoreBase to reuse some code. hbase-backup had to be added as a test artifact to hbase-it/pom.xml as a result.

kgeisz added 11 commits October 16, 2025 21:44
…s and non-continuous subclasses

Change-Id: I0c70c417b86c7732b58642a51c75897c35b16cb6
Change-Id: Id043400bf85c7b696bb94bef7cb17ed9dad13334
…a for loop

Change-Id: I5ba3276919e6bbdf343c134fa287c69f3854a8a2
Change-Id: I25fe484e9c227b7a31cb3768def3c12f66d617ac
Change-Id: Ie9aece8a3ec55739d618ebf2d2f173a41a116eb6
Change-Id: Ie345e623089979f028b13aed13e5ec93e025eff8
Change-Id: I98eb019dd93dfc8e21b6c730e0e2e60314102724
Change-Id: I4de6fc485aa1ff6e0d8d837e081f8dde20bb3f67
Change-Id: I911180a8f263f801a5c299d43d0215fe444f22d3
Change-Id: I78fe59f800cde7c89b11760a49d774c5173a862c
Change-Id: Ia150d21f48bb160d9e8bcf922799dc18c0b7c77c
@taklwu taklwu requested a review from Copilot October 27, 2025 20:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the backup integration tests to support continuous backup functionality by refactoring IntegrationTestBackupRestore into an abstract base class and creating separate test implementations for continuous and non-continuous backup scenarios. The primary addition is a new continuous backup test that validates backup WAL directory structure, replication peer subscription, and snapshot creation.

Key changes:

  • Abstract base class IntegrationTestBackupRestoreBase containing shared backup/restore test logic
  • New IntegrationTestContinuousBackupRestore class for continuous backup scenarios
  • Refactored IntegrationTestBackupRestore class extending the base class for non-continuous scenarios

Reviewed Changes

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

Show a summary per file
File Description
IntegrationTestContinuousBackupRestore.java New test class implementing continuous backup integration tests with single region server constraint
IntegrationTestBackupRestoreBase.java Abstract base class containing shared backup/restore test logic including snapshot verification, WAL verification, and backup deletion
IntegrationTestBackupRestore.java Refactored to extend base class for non-continuous backup testing
IntegrationTestBackupRestore.java (deleted) Original file relocated to backup package
hbase-it/pom.xml Added hbase-backup test-jar dependency
TestContinuousBackup.java Refactored to use shared utility method for replication peer verification
BackupTestUtil.java Added utility methods for enabling backup and verifying replication peer subscription

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@anmolnar
Copy link
Contributor

Great stuff @kgeisz ! I think with the new base class, the code has become pretty clean and easy to read.
I see that you haven't added the synchronization part that I mentioned to wait for the RS to ship the WAL entries to the destination. Does it work without it?

Copy link
Contributor

@taklwu taklwu left a comment

Choose a reason for hiding this comment

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

left few comments

@anmolnar
Copy link
Contributor

@kgeisz We're planning to ship integration tests separately from the feature to reduce the overhead on the review process. So, we probably don't want to merge this into the feature branch.

@kgeisz
Copy link
Contributor Author

kgeisz commented Oct 30, 2025

Great stuff @kgeisz ! I think with the new base class, the code has become pretty clean and easy to read. I see that you haven't added the synchronization part that I mentioned to wait for the RS to ship the WAL entries to the destination. Does it work without it?

@anmolnar Thanks! Also, I have not seen any issues even without the synchronization.

@kgeisz We're planning to ship integration tests separately from the feature to reduce the overhead on the review process. So, we probably don't want to merge this into the feature branch.

Sounds good. Should I just have this merge into master instead and we just won't merge this until the HBASE-28957 feature branch has been merged into master?

Change-Id: I9d5b55e36b44367ac8ace08a5859c42b796fefd4
@kgeisz kgeisz requested a review from taklwu October 30, 2025 01:10
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@kgeisz kgeisz changed the base branch from HBASE-28957 to HBASE-29164 October 30, 2025 17:01
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

…h latest Put timestamp

Change-Id: Ic438ca292bc01827d46725e006bfa0c21bc95f01
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

Change-Id: I9d52e774e84dc389d42aa63315529a2590c40cb8
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@kgeisz
Copy link
Contributor Author

kgeisz commented Nov 3, 2025

The patch compile tests resulted in a -1 due to Maven spotless, but I have already run mvn spotless:apply.

I also found a wget error in the log:

[2025-11-01T00:41:23.733Z] Connecting to ci-hbase.apache.org (ci-hbase.apache.org)|95.216.8.237|:443... connected.
[2025-11-01T00:41:23.733Z] HTTP request sent, awaiting response... 404 
[2025-11-01T00:41:23.733Z] 2025-11-01 00:41:22 ERROR 404: (no description).
[2025-11-01T00:41:23.733Z] 
[2025-11-01T00:41:23.733Z] Wget error 8 in fetching excludes file from url https://ci-hbase.apache.org//job/HBase-Find-Flaky-Tests/job/HBASE-29164/lastSuccessfulBuild/artifact/output/excludes. Ignoring and proceeding.
[2025-11-01T00:41:26.175Z] cd /home/jenkins/workspace/Base-PreCommit-GitHub-PR_PR-7417/yetus-jdk17-hadoop3-check/src/hbase-backup

Change-Id: I27eec25091842376ee7a059a9688c6f5ab385ac7
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

Change-Id: I18ab629df4af4e93b42ec1b0d576fd411279c775
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

Copy link
Contributor

@taklwu taklwu left a comment

Choose a reason for hiding this comment

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

LGTM

Change-Id: Ibc96fd712e384cc3ca5a2c4575e47e65e62c60fa
@kgeisz
Copy link
Contributor Author

kgeisz commented Nov 4, 2025

I made another small change to this PR. The original IntegrationTestBackupRestore file had a bug in how it configures command line parameters. In IntegrationTestBackupRestore on the master branch, the processOptions() method is doing proper initialization. Later, in the setUp() method, this block of code overwrites these values. I was doing the same in my PR, but now I have realized I should not.

Change-Id: Ie8e94ce978836b1314525138726a13641360aae6
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

Change-Id: Ibeea379a65e801b60ec5124938b7aa17087025f0
@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 31s 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-29164 Compile Tests _
+0 🆗 mvndep 0m 24s Maven dependency ordering for branch
+1 💚 mvninstall 3m 57s HBASE-29164 passed
+1 💚 compile 1m 9s HBASE-29164 passed
-0 ⚠️ checkstyle 0m 14s /buildtool-branch-checkstyle-hbase-backup.txt The patch fails to run checkstyle in hbase-backup
+1 💚 spotbugs 1m 6s HBASE-29164 passed
+1 💚 spotless 1m 4s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 12s Maven dependency ordering for patch
+1 💚 mvninstall 3m 15s the patch passed
+1 💚 compile 1m 6s the patch passed
-0 ⚠️ javac 0m 33s /results-compile-javac-hbase-it.txt hbase-it generated 4 new + 105 unchanged - 4 fixed = 109 total (was 109)
+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 💚 xmllint 0m 0s No new issues.
+1 💚 spotbugs 1m 10s the patch passed
+1 💚 hadoopcheck 12m 46s Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚 spotless 0m 45s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 20s The patch does not generate ASF License warnings.
36m 26s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7417/10/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #7417
JIRA Issue HBASE-29687
Optional Tests dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless xmllint
uname Linux 012018e33c1f 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-29164 / 4cc466e
Default Java Eclipse Adoptium-17.0.11+9
Max. process+thread count 83 (vs. ulimit of 30000)
modules C: hbase-backup hbase-it U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7417/10/console
versions git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 13s 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-29164 Compile Tests _
+0 🆗 mvndep 0m 15s Maven dependency ordering for branch
+1 💚 mvninstall 4m 58s HBASE-29164 passed
+1 💚 compile 0m 52s HBASE-29164 passed
+1 💚 javadoc 0m 33s HBASE-29164 passed
+1 💚 shadedjars 7m 4s branch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 14s Maven dependency ordering for patch
+1 💚 mvninstall 4m 21s the patch passed
+1 💚 compile 0m 55s the patch passed
+1 💚 javac 0m 55s the patch passed
+1 💚 javadoc 0m 31s the patch passed
+1 💚 shadedjars 6m 55s patch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚 unit 36m 3s hbase-backup in the patch passed.
+1 💚 unit 1m 2s hbase-it in the patch passed.
65m 4s
Subsystem Report/Notes
Docker ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7417/10/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR #7417
JIRA Issue HBASE-29687
Optional Tests javac javadoc unit compile shadedjars
uname Linux bdcd7c37f11c 6.8.0-1024-aws #26~22.04.1-Ubuntu SMP Wed Feb 19 06:54:57 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision HBASE-29164 / 4cc466e
Default Java Eclipse Adoptium-17.0.11+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7417/10/testReport/
Max. process+thread count 2170 (vs. ulimit of 30000)
modules C: hbase-backup hbase-it U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7417/10/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 anmolnar merged commit 7537362 into apache:HBASE-29164 Nov 5, 2025
1 check passed
Sign up for free to 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