Skip to content

HBASE-29029 Refactor BackupHFileCleaner + fix test #6533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 13, 2025
Merged

HBASE-29029 Refactor BackupHFileCleaner + fix test #6533

merged 1 commit into from
Jun 13, 2025

Conversation

DieterDP-ng
Copy link
Contributor

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
ndimiduk
ndimiduk reviewed Jan 7, 2025
View reviewed changes
Copy link
Member

@ndimiduk ndimiduk left a comment

Choose a reason for hiding this comment

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

Looks alright to me. I have just a couple questions.

@DieterDP-ng
Copy link
Contributor Author

Remarks have been addressed.

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
ndimiduk
ndimiduk approved these changes Feb 26, 2025
View reviewed changes
Copy link
Member

@ndimiduk ndimiduk left a comment

Choose a reason for hiding this comment

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

This looks good to me. I think that a spotless run will also clean up the checkstyle nits.

@DieterDP-ng
HBASE-29029 Refactor BackupHFileCleaner + fix test
4100b6d 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).
@DieterDP-ng
Copy link
Contributor Author

Rebased & ran spotless:apply.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 37s 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.
_ master Compile Tests _
+1 💚 mvninstall 4m 10s master passed
+1 💚 compile 0m 38s master passed
+1 💚 checkstyle 0m 13s master passed
+1 💚 spotbugs 0m 41s master passed
+1 💚 spotless 1m 2s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚 mvninstall 3m 53s the patch passed
+1 💚 compile 0m 37s the patch passed
+1 💚 javac 0m 37s hbase-backup generated 0 new + 103 unchanged - 1 fixed = 103 total (was 104)
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 0m 14s /results-checkstyle-hbase-backup.txt hbase-backup: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚 xmllint 0m 0s No new issues.
+1 💚 spotbugs 0m 44s the patch passed
+1 💚 hadoopcheck 14m 44s Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚 spotless 0m 53s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 12s The patch does not generate ASF License warnings.
37m 12s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6533/6/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #6533
Optional Tests dupname asflicense javac codespell detsecrets xmllint hadoopcheck spotless compile spotbugs checkstyle hbaseanti
uname Linux b2d73081e701 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 master / 4100b6d
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-6533/6/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 29s 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 _
_ master Compile Tests _
+1 💚 mvninstall 3m 12s master passed
+1 💚 compile 0m 18s master passed
+1 💚 javadoc 0m 14s master passed
+1 💚 shadedjars 5m 54s branch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 3m 5s the patch passed
+1 💚 compile 0m 19s the patch passed
+1 💚 javac 0m 19s the patch passed
+1 💚 javadoc 0m 13s the patch passed
+1 💚 shadedjars 5m 50s patch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚 unit 11m 0s hbase-backup in the patch passed.
31m 38s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6533/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR #6533
Optional Tests javac javadoc unit shadedjars compile
uname Linux a9a2acc68419 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 master / 4100b6d
Default Java Eclipse Adoptium-17.0.11+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6533/6/testReport/
Max. process+thread count 3717 (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-6533/6/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.

@DieterDP-ng
Copy link
Contributor Author

@ndimiduk @rmdmattingly - this PR seems to have been forgotten.

@ndimiduk
Copy link
Member

Mea culpa.

@ndimiduk ndimiduk merged commit 64c582f into apache:master Jun 13, 2025
1 check passed
ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Jun 13, 2025
@DieterDP-ng @ndimiduk
HBASE-29029 Refactor BackupHFileCleaner + fix test (apache#6533)
209b5f5 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Jun 13, 2025
@DieterDP-ng @ndimiduk
HBASE-29029 Refactor BackupHFileCleaner + fix test (apache#6533)
26d60d8 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Jun 13, 2025
@DieterDP-ng @ndimiduk
HBASE-29029 Refactor BackupHFileCleaner + fix test (apache#6533)
96ad214 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
ndimiduk pushed a commit that referenced this pull request Jun 16, 2025
@DieterDP-ng @ndimiduk
HBASE-29029 Refactor BackupHFileCleaner + fix test (#6533)
fb60ebf 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
ndimiduk pushed a commit that referenced this pull request Jun 16, 2025
@DieterDP-ng @ndimiduk
HBASE-29029 Refactor BackupHFileCleaner + fix test (#6533)
ad50d5c 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
ndimiduk pushed a commit that referenced this pull request Jun 16, 2025
@DieterDP-ng @ndimiduk
HBASE-29029 Refactor BackupHFileCleaner + fix test (#6533)
a65bb51 
The TestBackupHFileCleaner test was broken, as it used
a different API to register bulk loads than the API that
is actually used to register bulk loads during backups.
The test also incorrectly closed the FS of the HBaseTestingUtil,
causing this test to block for about 5 minutes during shutdown.

Both the test and BackupHFileCleaner itself were overly convoluted
and are cleaned up. Methods in BackupSystemTable that could lead
to incorrect use have been removed or deprecated (to fix their
use case in HBASE-28715).

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndimiduk ndimiduk ndimiduk approved these changes

@rmdmattingly rmdmattingly Awaiting requested review from rmdmattingly

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DieterDP-ng @Apache-HBase @ndimiduk