Skip to content

HBASE-29003 Proper bulk load tracking - #6506

Merged
rmdmattingly merged 1 commit into
apache:masterfrom
DieterDP-ng:HBASE-29003
Apr 21, 2025
Merged

HBASE-29003 Proper bulk load tracking#6506
rmdmattingly merged 1 commit into
apache:masterfrom
DieterDP-ng:HBASE-29003

Conversation

@DieterDP-ng

@DieterDP-ngDieterDP-ng commented Dec 2, 2024

Copy link
Copy Markdown
Contributor

Note that this build on top of my older PR: #6134

@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 commented Jan 8, 2025

Copy link
Copy Markdown
Member

Do you mind fixing this broken checkstyle while you're in there?

[WARNING] The requested profile "test-patch" could not be activated because it does not exist.
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:checkstyle (default-cli) on project hbase-backup: An error has occurred in Checkstyle report generation. Failed during checkstyle configuration: Exception was thrown while processing /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-6506/yetus-general-check/src/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BulkLoad.java: IllegalStateException occurred while parsing file /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-6506/yetus-general-check/src/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BulkLoad.java. /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-6506/yetus-general-check/src/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BulkLoad.java:72:33: expecting RPAREN, found 'that' -> [Help 1]

@DieterDP-ng
DieterDP-ngforce-pushed the HBASE-29003 branch 2 times, most recently from 15fdfe7 to 01b6b87CompareFebruary 7, 2025 12:40
@DieterDP-ng

DieterDP-ng commented Feb 7, 2025

Copy link
Copy Markdown
ContributorAuthor

Fixed the checkstyle error.

@ndimiduk@rmdmattingly the discussion on Jira about the best approach stopped. Should I wait for further feedback?

Personally, I think the best solution would be to track in the backup system table whether a full backup is required or not. This tracking would have to occur through an Observer as is done in this ticket. Given that this PR solves a real issue, I'd suggest to include it, and leave the switch to the tracking system to a later commit.

Note that users should update their config to include the new observer. Not sure if this counts as a migration since the backup is still considered experimental. Should be mentioned somewhere though, but unsure where.

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@ndimiduk

ndimiduk commented Feb 26, 2025

Copy link
Copy Markdown
Member

FYI @hgromer

@ndimiduk

ndimiduk commented Feb 26, 2025

Copy link
Copy Markdown
Member

Note that users should update their config to include the new observer. Not sure if this counts as a migration since the backup is still considered experimental. Should be mentioned somewhere though, but unsure where.

This kind of change does demand a mention in the release notes. We have such a field on our Jira ticket schema, and it automatically gets populated at release time.

@hgromer

hgromer commented Feb 26, 2025

Copy link
Copy Markdown
Contributor

I had actually spun up a Jira to this effect, didn't realize there was already an open PR

@DieterDP-ng

DieterDP-ng commented Mar 6, 2025

Copy link
Copy Markdown
ContributorAuthor

Any advice on how to progress this ticket?

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@DieterDP-ng

DieterDP-ng commented Mar 7, 2025

Copy link
Copy Markdown
ContributorAuthor

Test failure looks unrelated to this PR, flaky test?

@rmdmattingly

rmdmattingly commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

Hey @DieterDP-ng, thanks for the work here and sorry that I've been slow to respond. I will take a look here today

rmdmattingly
rmdmattingly approved these changes Mar 10, 2025
View reviewed changes

@rmdmattinglyrmdmattingly left a comment

Copy link
Copy Markdown
Contributor

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. @ndimiduk or @hgromer, any thoughts?

Comment threadhbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java Outdated
Predicate<BulkLoad> filter) throws IOException {
try (Connection connection = ConnectionFactory.createConnection(config);
BackupSystemTable tbl = new BackupSystemTable(connection)) {
List<BulkLoad> bulkLoads = tbl.readBulkloadRows(List.of(tableName));

@rmdmattinglyrmdmattinglyMar 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a concern for another time, but it's only a matter of time until the lack of pagination in BackupSystemTable#readBulkloadRows bites us

hgromer
hgromer reviewed Mar 18, 2025
View reviewed changes
Comment threadhbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java
backupManager.writeBackupStartCode(newStartCode);

backupManager
.deleteBulkLoadedRows(bulkLoadsToDelete.stream().map(BulkLoad::getRowKey).toList());

@hgromerhgromerMar 18, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At my company, we have a similar patch applied to our fork, and we've run into issues with batch sizes that causes backup failures. This seems to happen when there are too many rows to delete, you end up with something like

Caused by: org.apache.hbase.thirdparty.com.google.protobuf.ServiceException: Rejecting large batch operation for current batch with firstRegionName: backup:system_bulk,,1739970553683.c3828af81a4b3847aa0f1612bf638713. , Requested Number of Rows: 2048 , Size Threshold: 1500
at org.apache.hadoop.hbase.regionserver.RSRpcServices.checkBatchSizeAndLogLargeSize(RSRpcServices.java:2721)
at org.apache.hadoop.hbase.regionserver.RSRpcServices.multi(RSRpcServices.java:2757)
at org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos$ClientService$2.callBlockingMethod(ClientProtos.java:43520)
at org.apache.ha

It might be worth splitting up this call to delete if they are exceptionally large

@ndimidukndimidukMar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hgromer do you feel that splitting up the delete is required for this patch?

@ndimidukndimidukMar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@DieterDP-ng any concerns about making this change?

@hgromerhgromerMar 31, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be worth splitting this up. Introducing this means we can get into a situation where backups are unable to succeed.

@DieterDP-ngDieterDP-ngApr 2, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

No objection to splitting it. Updating the PR...

@DieterDP-ngDieterDP-ngApr 2, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Rather than doing batching here, I did it at BackupSystemTable.deleteBulkLoadedRows, since that method is used in multiple locations, and I guess they would all benefit from batching. See added commit (can be squashed).

@DieterDP-ngDieterDP-ngApr 17, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

(For future reference: turned out batching was already built-in, see #6506 (comment))

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
hgromer
hgromer reviewed Apr 2, 2025
View reviewed changes
Comment threadhbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java Outdated
public void deleteBulkLoadedRows(List<byte[]> rows) throws IOException {
try (BufferedMutator bufferedMutator = connection.getBufferedMutator(bulkLoadTableName)) {
try (BufferedMutator bufferedMutator = connection
.getBufferedMutator(new BufferedMutatorParams(bulkLoadTableName).setMaxMutations(1000))) {

@hgromerhgromerApr 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be possible to set this to the config value HConstants.BATCH_ROWS_THRESHOLD_NAME, seeing as that's the cluster limit for batches?

The configuration value must then be supplied by the operator launching the backup.

@DieterDP-ngDieterDP-ngApr 3, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This pointer led me to conclude that it's not needed to specify batching at all, because it's already present.

Our method uses Connection#getBufferedMutator, for which the only non-delegating implementation is ConnectionOverAsyncConnection#getBufferedMutator. That one calls AsyncConnectionImpl#getBufferedMutatorBuilder(TableName). In that method we can see it uses a AsyncConnectionConfiguration object that has already extracted a batching value:

AsyncConnectionConfiguration(Configuration conf) {
...
this.bufferedMutatorMaxMutations = conf.getInt(BUFFERED_MUTATOR_MAX_MUTATIONS_KEY,
conf.getInt(HConstants.BATCH_ROWS_THRESHOLD_NAME, BUFFERED_MUTATOR_MAX_MUTATIONS_DEFAULT));
}

So I'll remove my change that introduced the batchsize.

@hgromerhgromerApr 3, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah good to know, so the only thing needed is for the operator to specify the configuration in the Connection object that is passed to the BackupAdminImp

👍

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@rmdmattingly

rmdmattingly commented Apr 8, 2025

Copy link
Copy Markdown
Contributor

Retrying the build

@Apache-HBase

This comment has been minimized.

Sign in to view
@Apache-HBase

This comment has been minimized.

Sign in to view
@DieterDP-ng

DieterDP-ng commented Apr 14, 2025

Copy link
Copy Markdown
ContributorAuthor

The failed test runs OK for me. I guess another flaky test.

DieterDP-ng
DieterDP-ng commented Apr 14, 2025
View reviewed changes
Comment threadhbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupManager.java Outdated
}
(coproc == null ? "" : coproc + ",") + observerClass);

String masterCoProc = conf.get(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY);

@DieterDP-ngDieterDP-ngApr 14, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This probably belongs in the decorateMasterConfiguration method instead.

@rmdmattinglyrmdmattinglyApr 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you want to make this change? If not, I'm happy to ship this PR

@DieterDP-ngDieterDP-ngApr 17, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Updated!

I also extracted all master-related stuff from the BackupObserver to BackupMasterObserver. Did this in the light of HBASE-29240, where there's more master-specific logic being added.

@DieterDP-ng
HBASE-29003 Proper bulk load tracking
9cf3008
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
@Apache-HBase

Apache-HBase commented Apr 17, 2025

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 28sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗codespell0m 0scodespell was not available.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
_ master Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for branch
+1 💚mvninstall3m 10smaster passed
+1 💚compile7m 53smaster passed
+1 💚checkstyle1m 10smaster passed
+1 💚spotbugs9m 17smaster passed
+0 🆗refguide2m 18sbranch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚spotless0m 44sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗mvndep0m 11sMaven dependency ordering for patch
+1 💚mvninstall3m 1sthe patch passed
+1 💚compile7m 45sthe patch passed
+1 💚javac7m 45sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle1m 15sthe patch passed
+1 💚spotbugs9m 41sthe patch passed
+0 🆗refguide2m 4spatch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚hadoopcheck11m 50sPatch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚spotless0m 47spatch has no errors when running spotless:check.
_ Other Tests _
+1 💚asflicense0m 29sThe patch does not generate ASF License warnings.
70m 5s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6506/10/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#6506
Optional Testsdupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless refguide
unameLinux 7f13fbdef710 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 9cf3008
Default JavaEclipse Adoptium-17.0.11+9
refguidehttps://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-6506/10/yetus-general-check/output/branch-site/book.html
refguidehttps://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-6506/10/yetus-general-check/output/patch-site/book.html
Max. process+thread count187 (vs. ulimit of 30000)
modulesC: hbase-server hbase-backup . U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6506/10/console
versionsgit=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Apr 17, 2025

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 27sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed 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 _
+0 🆗mvndep0m 36sMaven dependency ordering for branch
+1 💚mvninstall3m 25smaster passed
+1 💚compile2m 7smaster passed
+1 💚javadoc2m 44smaster passed
+1 💚shadedjars5m 57sbranch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall3m 1sthe patch passed
+1 💚compile2m 11sthe patch passed
+1 💚javac2m 11sthe patch passed
+1 💚javadoc2m 34sthe patch passed
+1 💚shadedjars5m 58spatch has no errors when building our shaded downstream artifacts.
_ Other Tests _
-1 ❌unit317m 34s/patch-unit-root.txtroot in the patch failed.
353m 33s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6506/10/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#6506
Optional Testsjavac javadoc unit compile shadedjars
unameLinux d5a0bbc52f6e 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 9cf3008
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6506/10/testReport/
Max. process+thread count5112 (vs. ulimit of 30000)
modulesC: hbase-server hbase-backup . U: .
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6506/10/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@rmdmattingly
rmdmattingly merged commit 65a6d8a into apache:masterApr 21, 2025
rmdmattingly pushed a commit that referenced this pull request Apr 21, 2025
@DieterDP-ngRay Mattingly
HBASE-29003Proper bulk load tracking (#6506)
3b9e96b
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
rmdmattingly pushed a commit that referenced this pull request Apr 21, 2025
@DieterDP-ngRay Mattingly
HBASE-29003Proper bulk load tracking (#6506)
81b2c9e
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
rmdmattingly pushed a commit that referenced this pull request Apr 22, 2025
@DieterDP-ngRay Mattingly
HBASE-29003Proper bulk load tracking (#6506)
5274a25
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
rmdmattingly added a commit that referenced this pull request Apr 22, 2025
@rmdmattingly@DieterDP-ng
HBASE-29003Proper bulk load tracking (#6506) (#6920)
f60ecb6
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
Co-authored-by: DieterDP <90392398+DieterDP-ng@users.noreply.github.com>
rmdmattingly pushed a commit that referenced this pull request Apr 23, 2025
@DieterDP-ngRay Mattingly
HBASE-29003Proper bulk load tracking (#6506)
b872757
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
rmdmattingly added a commit that referenced this pull request Apr 23, 2025
@rmdmattingly@DieterDP-ng
HBASE-29003Proper bulk load tracking (#6506) (#6921)
148dd9e
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
Co-authored-by: DieterDP <90392398+DieterDP-ng@users.noreply.github.com>
rmdmattingly added a commit that referenced this pull request Apr 23, 2025
@rmdmattingly@DieterDP-ngRay Mattingly
HBASE-29003Proper bulk load tracking (#6506) (#6921)
f24a09d
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
Co-authored-by: DieterDP <90392398+DieterDP-ng@users.noreply.github.com>
rmdmattingly added a commit that referenced this pull request Apr 25, 2025
@rmdmattingly@DieterDP-ngRay Mattingly
HBASE-29003Proper bulk load tracking (#6506) (#6921)
43afb37
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
Co-authored-by: DieterDP <90392398+DieterDP-ng@users.noreply.github.com>
rmdmattingly added a commit that referenced this pull request Apr 27, 2025
@rmdmattingly@DieterDP-ng
HBASE-29003Proper bulk load tracking (#6506) (#6921) (#6927)
18a28f8
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
Co-authored-by: DieterDP <90392398+DieterDP-ng@users.noreply.github.com>
mokai87 pushed a commit to mokai87/hbase that referenced this pull request Aug 7, 2025
@rmdmattingly@DieterDP-ng@mokai87
HBASE-29003 Proper bulk load tracking (apache#6506) (apache#6921)
32139b6
Signed-off-by: Ray Mattingly <rmattingly@apache.org>
The HBase backup mechanism keeps track of which HFiles
were bulk loaded, so they can be included in incremental
backups.
Before this ticket, these bulk load records were only
deleted when an incremental backup is created. This
commit adds 2 more locations:
1) after a full backup. Since a full backup already
captures all data, this meant that unnecessary HFiles
were being included in the next incremental backup.
2) after a table delete/truncate/CF-deletion. Previously,
if an HFile was loaded before a table was cleared, the next
incremental backup would effectively still include the
HFile. This lead to incorrect data being restored.
This commit also completely refactors & simplifies the
test for this functionality.
Co-authored-by: DieterDP <90392398+DieterDP-ng@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rmdmattinglyrmdmattinglyrmdmattingly approved these changes
@hgromerhgromerhgromer approved these changes
@ndimidukndimidukAwaiting requested review from ndimiduk

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@DieterDP-ng@Apache-HBase@ndimiduk@hgromer@rmdmattingly