subject:"Re\: \[PR\] HBASE\-28990 Modify Incremental Backup for Continuous Backup \[hbase\]"

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-20 Thread via GitHub



taklwu merged PR #6788:
URL: https://github.com/apache/hbase/pull/6788


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-20 Thread via GitHub



taklwu commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2992465467

   @ankitsol before merging , can you double check 
`TestIncrementalBackup#TestIncBackupRestore` is passing at least in your local 
setup ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-18 Thread via GitHub



vinayakphegde commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2155028027


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   Makes sense — thanks for the discussion, folks! 👍



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-17 Thread via GitHub



taklwu commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2153681156


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   so, maybe asking differently, is this a one-way approach for continuous 
backup if we couple with the optimization of HBASE-29003 that reduce the 
additional bulkload HFiles of the source cluster? 
   
   and without this change, or as @Kota-SH pointed about the if the source 
cluster/directory is not accessible, which backup can we use ? especially can 
we still have incremental recovery? 
   
   My two cents on this approach, building on top of HBASE-29003, is that it 
seems reasonable. at least incremental backup already has this code change that 
uses the source cluster/storage. I was wondered the feedback on the design docs 
is also suggesting us to work closer with the logic of incremental backup, and 
such we could avoid introducing similar logic but in fact is serving the same 
thing. 
   
   Meanwhile, it's worth thinking of 
   a. the original plan that copies all the bulkloaded HFiles in between a 
incremental backup was too slow, other than this approach, do we have any 
alternative? 
   b. are we 100% against the continuous backup reads HFiles/bulkload HFiles 
from the source storage? HBASE-29003 made a very good point about storage 
usage, especially the HDFS use cases with 3 replicas. 
   
   ---
   
   point 3 and 4, I assumed @ankitsol  already answered, so I don't have 
comments.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-17 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2152621143


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   @Kota-SH  Post bulkload operation, user is advised to take a backup, as part 
of continuous backup and PITR design (I think this is not yet documented, I 
will add a comment in design to add this part)
   
   Earlier we implemented WALPlayer with bulkload restore functionality but 
that could have resulted in timeouts or performance issues, so we dropped 
WALPlayer implementation and decided to ask user to take a backup (ideally 
incremental backup) post a bulkload operation so restore is fast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-17 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2152638968


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   @Kota-SH Regarding compaction, there is preCommitStoreFile() hook which 
registers these bulkloaded files in backup system table and avoids deletion 
during compaction



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-17 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2152621143


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   @Kota-SH  Post bulkload operation, user is advised to take a backup, as part 
of continuous backup and PITR design (I think this is not yet documented, I 
will add a comment in design to add this part)
   
   Earlier we implemented WALPlayer with bulkload restore functionality but 
that could have resulted in timeouts or performance issues, so we dropped this 
WALPlayer modification and decided to ask user to take a backup (ideally 
incremental backup) post a bulkload operation so restore is fast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-17 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2152621143


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   @Kota-SH  Post bulkload operation, user is advised to take a backup, as part 
of contiuous backup and pitr design (I think this is not yet documented, I will 
add a comment in design to add this part)
   
   Earlier we implemented WALPlayer with bulkload restore functionality but 
that could have resulted in timeouts or performance issues, so we dropped 
WALPlayer implementation and decided to ask user to take a backup (ideally 
incremental backup) post a bulkload operation so restore is fast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-17 Thread via GitHub



Kota-SH commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2152443777


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   I think the question is what happens if the source cluster/storage is gone 
and users need a DR?
   We could still get back data from snapshots and WALs from the external 
storage. Is there a way to restore the bulkloads in such a situation?
   Also, what happens if those bulkload hfiles are compacted? Do we track it 
somewhere?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-16 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149561312


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   For 1) & 2) points I see 2 advantages with the coded approach in this PR. 
First, this behaviour would be same for both continuous and non-continuous 
incremental backup (ie using bulkload files from source cluster). Second, using 
source cluster hfiles wrt bulkload operation instead of backup hfiles would 
reduce processing time during backup and cost of storage space of backup area. 
Backing up bulkload operation would also delay backup of WALs since backing up 
WALs and bulkload files are serial in execution.
   
   Backing bulkload files idea was necessary when we were planning to use 
WALPlayer with bulkload restore capability. Now I don't see any advantage of 
backing up of bulkload files
   
   3) BackupObserver.preCommitStoreFile() in invoked only for bulkload 
operation so for bulkloaded hfiles only one time copy happens.
   
   4) This code actually resolves a bug for properly handling of bulkload 
operation, no modification of core logic although it might seem like.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-16 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149561312


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   For 1) & 2) points I see 2 advantages with the coded approach in this PR. 
First, this behaviour would be same for both continuous and non-continuous 
incremental backup (ie using bulkload files from source cluster). Second, using 
source cluster hfiles wrt bulkload operation instead of backup hfiles would 
reduce processing time during backup and cost of storage space of backup area. 
Backing up bulkload operation would also delay backup of WALs since backing up 
WALs and bulkload files are serial in execution.
   
   Backing bulkload files idea was necessary when we were planning to use 
WALPlayer with bulkload restore capability. Now I don't see any advantage of 
backing up of bulkload files
   
   3) BackupObserver.preCommitStoreFile() in invoked only for bulkload 
operation so for bulkloaded hfiles only one time copy happens.
   
   4) This code actually resolves a bug for properly handling of bulkload 
operation, no modification of core logic although it might like.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-16 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149561312


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   For 1) & 2) points I see 2 advantages with the coded approach in this PR. 
First, this behaviour would be same for both continuous and non-continuous 
incremental backup (ie using bulkload files from source cluster). Second, using 
source cluster hfiles wrt bulkload operation instead of backup hfiles would 
reduce processing time during backup and cost of storage space of backup area. 
Backing up bulkload operation would also delay backup of WALs since backing up 
WALs and bulkload files are serial in execution.
   
   Backing bulkload files idea was necessary when we were planning to use 
WALPlayer with bulkload restore capability. Now I don't see any advantage of 
backing up of bulkload files
   
   3) BackupObserver.preCommitStoreFile() in invoked only for bulkload 
operation so for bulkloaded hfiles only one time copy happens.
   
   4) This code actually resolves a bug for properly handling of bulkload 
operation, no modification of core logic.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-16 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149561312


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   For 1) & 2) points I see 2 advantages with the coded approach in this PR. 
First, this behaviour would be same for both continuous and non-continuous 
incremental backup (ie using bulkload files from source cluster). Second, using 
source cluster hfiles wrt bulkload operation instead of backup hfiles would 
reduce processing time and cost of storage space of backup area. Backing up 
bulkload operation would also delay backup of WALs since backing up WALs and 
bulkload files are serial in execution.
   
   Backing bulkload files idea was necessary when we were planning to use 
WALPlayer with bulkload restore capability. Now I don't see any advantage of 
backing up of bulkload files
   
   3) BackupObserver.preCommitStoreFile() in invoked only for bulkload 
operation so for bulkloaded hfiles only one time copy happens.
   
   4) This code actually resolves a bug for properly handling of bulkload 
operation, no modification of core logic.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-16 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149561312


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   1) & 2) I see 2 advantages with the coded approach in this PR. First, this 
behaviour would be same for both continuous and non-continuous incremental 
backup (ie using bulkload files from source cluster). Second, using source 
cluster hfiles wrt bulkload operation instead of backup hfiles would reduce 
processing time and cost of storage space of backup area. Backing up bulkload 
operation would also delay backup of WALs since backing up WALs and bulkload 
files are serial in execution.
   
   Backing bulkload files idea was necessary when we were planning to use 
WALPlayer with bulkload restore capability. Now I don't see any advantage of 
backing up of bulkload files
   
   3) BackupObserver.preCommitStoreFile() in invoked only for bulkload 
operation so for bulkloaded hfiles only one time copy happens.
   
   4) This code actually resolves a bug for properly handling of bulkload 
operation, no modification of core logic.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-16 Thread via GitHub



vinayakphegde commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149351693


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   Okay, I have a few questions:
   1. Why is this needed? Can’t we get the bulk loaded files from the backup 
location, just like we do with WAL files? Why not handle bulkload files the 
same way?
   2. Also, one thing I noticed — by default, when backup is enabled in the 
cluster, we start accumulating all HFiles (both store files and bulkloaded 
files) using BackupObserver. These files are registered under bulkLoadTableName 
to prevent cleaner chores from deleting them during incremental backups, etc. 
But in our case, we don’t want this behavior, right? The whole idea is to back 
up files into the backup location, not retain them on the source cluster. I 
think we should enable BackupObserver only for the traditional (non-continuous) 
backup path. Otherwise, we’re unnecessarily accumulating files in the source 
cluster.
   3. I also noticed this:
   - In [this 
code](https://github.com/apache/hbase/blob/master/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java#L290-L292),
 we read all WAL files since the last backup, convert them to HFiles, and store 
them in the backup location.
   - Again, in [this 
line](https://github.com/apache/hbase/blob/master/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java#L319),
 we’re reading bulkloaded files from the table and backing them up.
   - However, the list of bulkloaded files also includes regular store files — 
as seen 
[here](https://github.com/apache/hbase/blob/28c757d45aa1797bbee6892433aec16a460e765e/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java#L75)
 — which means we’re potentially processing normal HFiles twice?
   4. Regarding: “I added here because it was necessary for the bulkload test I 
added”. -- ideally, we shouldn’t modify core logic only to support a specific 
test case. It might be better to adapt the test to the intended behavior.
   
   @anmolnar @kgeisz @taklwu — please share your thoughts as well.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-15 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2149007153


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   This is needed so subsequent incremental backup (just after this full 
backup) captures only bulkload from this full backup, so fullbackup deletes 
existing bulkload entries from system table
   
   So an incremental backup post a full backup will capture only bulkload 
entries from fullbackup
   
   This change is actually part of 
https://github.com/apache/hbase/pull/6506/files#diff-18c753bdf4ce717d55d98646bf46723970d2bd4a470a815eb9512c7d94398274
 which is merged, I added here because it was necessary for bulkload test I 
added



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-12 Thread via GitHub



vinayakphegde commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2143289259


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   I couldn’t quite figure out why this step is needed. Could you please 
explain the reasoning behind it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-11 Thread via GitHub



kgeisz commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2141129199


##
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupWithContinuous.java:
##
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import static 
org.apache.hadoop.hbase.replication.regionserver.ReplicationMarkerChore.REPLICATION_MARKER_ENABLED_DEFAULT;
+import static 
org.apache.hadoop.hbase.replication.regionserver.ReplicationMarkerChore.REPLICATION_MARKER_ENABLED_KEY;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.impl.BackupAdminImpl;
+import org.apache.hadoop.hbase.backup.impl.BackupManifest;
+import org.apache.hadoop.hbase.backup.impl.BackupSystemTable;
+import org.apache.hadoop.hbase.backup.impl.BulkLoad;
+import org.apache.hadoop.hbase.backup.util.BackupUtils;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.tool.BulkLoadHFiles;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.CommonFSUtils;
+import org.apache.hadoop.hbase.util.HFileTestUtil;
+import org.apache.hadoop.util.ToolRunner;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
+
+@Category(LargeTests.class)
+public class TestIncrementalBackupWithContinuous extends TestContinuousBackup {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestIncrementalBackupWithContinuous.class);
+
+  private static final Logger LOG =
+LoggerFactory.getLogger(TestIncrementalBackupWithContinuous.class);
+
+  private byte[] ROW = Bytes.toBytes("row1");
+  private final byte[] FAMILY = Bytes.toBytes("family");
+  private final byte[] COLUMN = Bytes.toBytes("col");
+  private static final int ROWS_IN_BULK_LOAD = 100;
+
+  @Test
+  public void testContinuousBackupWithIncrementalBackupSuccess() throws 
Exception {
+LOG.info("Testing incremental backup with continuous backup");
+conf1.setBoolean(REPLICATION_MARKER_ENABLED_KEY, true);
+String methodName = 
Thread.currentThread().getStackTrace()[1].getMethodName();
+TableName tableName = TableName.valueOf("table_" + methodName);
+Table t1 = TEST_UTIL.createTable(tableName, FAMILY);
+
+try (BackupSystemTable table = new 
BackupSystemTable(TEST_UTIL.getConnection())) {
+  int before = table.getBackupHistory().size();
+
+  // Run continuous backup
+  String[] args = buildBackupArgs("full", new TableName[] { tableName }, 
true);
+  int ret = ToolRunner.run(conf1, new BackupDriver(), args);
+  assertEquals("Full Backup should succeed", 0, ret);
+
+  // Verify backup history increased and all the backups are succeeded
+  LOG.info("Verify backup history increased and all the backups are 
succeeded");
+  List backups = table.getBackupHistory();
+  assertEquals("Backup history should increase", before + 1, 
backups.size());
+  for (BackupInfo data : List.of(backups.get(0))) {
+String backupId = data.getBackupId();
+assertTrue(checkSucceeded(backupId));
+  }
+
+  // Verify backup manifest contains the correct tables
+  LOG.info("Verify backup manifest contains the correct tables");
+  BackupManifest manifest = getLatest

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-06 Thread via GitHub



ankitsol commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2948849540

   > can you check those failing tests of `TestIncrementalBackup` ?
   
   @taklwu TestIncrementalBackup has 2 flaky tests 
(TestIncBackupRestoreWithOriginalSplits and TestIncBackupRestore), there are 
other JIRAs tracking this test like 
[HBASE-28461](https://issues.apache.org/jira/browse/HBASE-28461)
   
   I have ran these test without my change with same failure


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2947674305

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 45s |  |  Docker mode activated.  |
   | -0 :warning: |  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 :green_heart: |  mvninstall  |   3m 37s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  shadedjars  |   7m  1s |  |  branch has no errors when 
building our shaded downstream artifacts.  |
   | -0 :warning: |  patch  |   7m 11s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 53s |  |  patch has no errors when 
building our shaded downstream artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  |  23m 10s | 
[/patch-unit-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   |  |   |  47m 46s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux bbb94d8941b5 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 / 64fd7b82065725eaa59bb23cdcc510bc5fa677a7 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/testReport/
 |
   | Max. process+thread count | 3762 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2947652567

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 39s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any 
anti-patterns.  |
    _ HBASE-28957 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 32s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  |  HBASE-28957 passed  |
   | -0 :warning: |  checkstyle  |   0m 16s | 
[/buildtool-branch-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/artifact/yetus-general-check/output/buildtool-branch-checkstyle-hbase-backup.txt)
 |  The patch fails to run checkstyle in hbase-backup  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotless  |   1m  2s |  |  branch has no errors when 
running spotless:check.  |
   | -0 :warning: |  patch  |   1m 11s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 22s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 41s |  |  the patch passed  |
   | -0 :warning: |  javac  |   0m 41s | 
[/results-compile-javac-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/artifact/yetus-general-check/output/results-compile-javac-hbase-backup.txt)
 |  hbase-backup generated 2 new + 109 unchanged - 0 fixed = 111 total (was 
109)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m 12s | 
[/buildtool-patch-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/artifact/yetus-general-check/output/buildtool-patch-checkstyle-hbase-backup.txt)
 |  The patch fails to run checkstyle in hbase-backup  |
   | +1 :green_heart: |  spotbugs  |   0m 58s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  16m 42s |  |  Patch does not cause any 
errors with Hadoop 3.3.6 3.4.0.  |
   | +1 :green_heart: |  spotless  |   1m 11s |  |  patch has no errors when 
running spotless:check.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   |  42m 32s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/artifact/yetus-general-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell 
detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 13822d3ad071 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 / 64fd7b82065725eaa59bb23cdcc510bc5fa677a7 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 83 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/8/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



taklwu commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2945908145

   can you check those failing tests of `TestIncrementalBackup` ?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2129214025


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -262,9 +273,18 @@ public void execute() throws IOException, 
ColumnFamilyMismatchException {
   // case PREPARE_INCREMENTAL:
   beginBackup(backupManager, backupInfo);
   backupInfo.setPhase(BackupPhase.PREPARE_INCREMENTAL);
-  LOG.debug("For incremental backup, current table set is "
-+ backupManager.getIncrementalBackupTableSet());
-  newTimestamps = ((IncrementalBackupManager) 
backupManager).getIncrBackupLogFileMap();
+  // Non-continuous Backup incremental backup is controlled by 
'incremental backup table set'
+  // and not by user provided backup table list. This is an optimization 
to avoid copying
+  // the same set of WALs for incremental backups of different tables at 
different time
+  // HBASE-14038
+  // Continuous-incremental backup backs up user provided table list/set
+  if (backupInfo.isContinuousBackupEnabled()) {
+LOG.debug("For incremental backup, current table set is " + 
backupInfo.getTables());
+  } else {
+LOG.debug("For incremental backup, current table set is "
+  + backupManager.getIncrementalBackupTableSet());
+newTimestamps = ((IncrementalBackupManager) 
backupManager).getIncrBackupLogFileMap();
+  }

Review Comment:
   Fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2129211091


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -362,23 +378,86 @@ protected void deleteBulkLoadDirectory() throws 
IOException {
   }
 
   protected void convertWALsToHFiles() throws IOException {
-// get incremental backup file list and prepare parameters for DistCp
-List incrBackupFileList = backupInfo.getIncrBackupFileList();
-// Get list of tables in incremental backup set
-Set tableSet = backupManager.getIncrementalBackupTableSet();
-// filter missing files out (they have been copied by previous backups)
-incrBackupFileList = filterMissingFiles(incrBackupFileList);
-List tableList = new ArrayList();
-for (TableName table : tableSet) {
-  // Check if table exists
-  if (tableExists(table, conn)) {
-tableList.add(table.getNameAsString());
-  } else {
-LOG.warn("Table " + table + " does not exists. Skipping in WAL 
converter");
+if (backupInfo.isContinuousBackupEnabled()) {
+  Set tableSet = backupInfo.getTables();
+  List backupInfos = backupManager.getBackupHistory(true);
+  for (TableName table : tableSet) {
+for (BackupInfo backup : backupInfos) {
+  // find previous backup for this table
+  if (backup.getTables().contains(table)) {
+LOG.info("Found previous backup of type {} with id {} for table 
{}", backup.getType(),
+  backup.getBackupId(), table.getNameAsString());
+List walBackupFileList;
+if (backup.getType() == BackupType.FULL) {
+  walBackupFileList = getBackupLogs(backup.getStartTs());
+} else {
+  walBackupFileList = 
getBackupLogs(backup.getIncrCommittedWalTs());
+}
+walToHFiles(walBackupFileList, 
Arrays.asList(table.getNameAsString()));
+break;
+  }
+}
+  }
+} else {
+  // get incremental backup file list and prepare parameters for DistCp
+  List incrBackupFileList = backupInfo.getIncrBackupFileList();
+  // Get list of tables in incremental backup set
+  Set tableSet = backupManager.getIncrementalBackupTableSet();
+  // filter missing files out (they have been copied by previous backups)
+  incrBackupFileList = filterMissingFiles(incrBackupFileList);
+  List tableList = new ArrayList();
+  for (TableName table : tableSet) {
+// Check if table exists
+if (tableExists(table, conn)) {
+  tableList.add(table.getNameAsString());
+} else {
+  LOG.warn("Table " + table + " does not exists. Skipping in WAL 
converter");
+}
   }
+  walToHFiles(incrBackupFileList, tableList);
 }
-walToHFiles(incrBackupFileList, tableList);
+  }
+
+  private List getBackupLogs(long startTs) throws IOException {
+// get log files from backup dir
+String walBackupDir = conf.get(CONF_CONTINUOUS_BACKUP_WAL_DIR);
+if (walBackupDir == null || walBackupDir.isEmpty()) {
+  throw new IOException(
+"Incremental backup requires the WAL backup directory " + 
CONF_CONTINUOUS_BACKUP_WAL_DIR);
+}
+List resultLogFiles = new ArrayList<>();
+Path walBackupPath = new Path(walBackupDir);
+FileSystem backupFs = FileSystem.get(walBackupPath.toUri(), conf);
+FileStatus[] dayDirs = backupFs.listStatus(new Path(walBackupDir, 
WALS_DIR));
+SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT);
+
+for (FileStatus dayDir : dayDirs) {
+  if (!dayDir.isDirectory()) {
+continue; // Skip files, only process directories
+  }
 
+  String dirName = dayDir.getPath().getName();
+  try {
+Date dirDate = dateFormat.parse(dirName);
+long dirStartTime = dirDate.getTime(); // Start of that day (00:00:00)
+long dirEndTime = dirStartTime + ONE_DAY_IN_MILLISECONDS - 1; // End 
time of the day
+// (23:59:59)
+
+if (dirEndTime >= startTs) {
+  Path dirPath = dayDir.getPath();
+  FileStatus[] logs = backupFs.listStatus(dirPath);
+  ;
+  for (FileStatus log : logs) {
+String filepath = log.getPath().toString();
+LOG.debug("currentLogFile: " + filepath);
+resultLogFiles.add(filepath);
+  }
+}

Review Comment:
   Right, good point



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2129210287


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -362,23 +378,86 @@ protected void deleteBulkLoadDirectory() throws 
IOException {
   }
 
   protected void convertWALsToHFiles() throws IOException {
-// get incremental backup file list and prepare parameters for DistCp
-List incrBackupFileList = backupInfo.getIncrBackupFileList();
-// Get list of tables in incremental backup set
-Set tableSet = backupManager.getIncrementalBackupTableSet();
-// filter missing files out (they have been copied by previous backups)
-incrBackupFileList = filterMissingFiles(incrBackupFileList);
-List tableList = new ArrayList();
-for (TableName table : tableSet) {
-  // Check if table exists
-  if (tableExists(table, conn)) {
-tableList.add(table.getNameAsString());
-  } else {
-LOG.warn("Table " + table + " does not exists. Skipping in WAL 
converter");
+if (backupInfo.isContinuousBackupEnabled()) {
+  Set tableSet = backupInfo.getTables();
+  List backupInfos = backupManager.getBackupHistory(true);
+  for (TableName table : tableSet) {
+for (BackupInfo backup : backupInfos) {
+  // find previous backup for this table
+  if (backup.getTables().contains(table)) {
+LOG.info("Found previous backup of type {} with id {} for table 
{}", backup.getType(),
+  backup.getBackupId(), table.getNameAsString());
+List walBackupFileList;
+if (backup.getType() == BackupType.FULL) {
+  walBackupFileList = getBackupLogs(backup.getStartTs());
+} else {
+  walBackupFileList = 
getBackupLogs(backup.getIncrCommittedWalTs());
+}
+walToHFiles(walBackupFileList, 
Arrays.asList(table.getNameAsString()));
+break;
+  }
+}
+  }
+} else {
+  // get incremental backup file list and prepare parameters for DistCp
+  List incrBackupFileList = backupInfo.getIncrBackupFileList();
+  // Get list of tables in incremental backup set
+  Set tableSet = backupManager.getIncrementalBackupTableSet();
+  // filter missing files out (they have been copied by previous backups)
+  incrBackupFileList = filterMissingFiles(incrBackupFileList);
+  List tableList = new ArrayList();
+  for (TableName table : tableSet) {
+// Check if table exists
+if (tableExists(table, conn)) {
+  tableList.add(table.getNameAsString());
+} else {
+  LOG.warn("Table " + table + " does not exists. Skipping in WAL 
converter");
+}
   }
+  walToHFiles(incrBackupFileList, tableList);
 }
-walToHFiles(incrBackupFileList, tableList);
+  }
+
+  private List getBackupLogs(long startTs) throws IOException {
+// get log files from backup dir
+String walBackupDir = conf.get(CONF_CONTINUOUS_BACKUP_WAL_DIR);
+if (walBackupDir == null || walBackupDir.isEmpty()) {
+  throw new IOException(
+"Incremental backup requires the WAL backup directory " + 
CONF_CONTINUOUS_BACKUP_WAL_DIR);
+}
+List resultLogFiles = new ArrayList<>();
+Path walBackupPath = new Path(walBackupDir);
+FileSystem backupFs = FileSystem.get(walBackupPath.toUri(), conf);
+FileStatus[] dayDirs = backupFs.listStatus(new Path(walBackupDir, 
WALS_DIR));
+SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT);

Review Comment:
   Yes, here I have fixed, we need to do same at other places



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2129209021


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -148,13 +148,16 @@ protected void snapshotCopy(BackupInfo backupInfo) throws 
IOException {
   public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
+  initializeBackupStartCode(backupManager);
+  performLogRoll(admin);

Review Comment:
   Not needed, removed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2129207952


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java:
##
@@ -565,7 +584,7 @@ public String backupTables(BackupRequest request) throws 
IOException {
 }
   }
   if (nonExistingTableList != null) {
-if (type == BackupType.INCREMENTAL) {
+if (type == BackupType.INCREMENTAL && 
!request.isContinuousBackupEnabled()) {

Review Comment:
   Sure



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-05 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2129206236


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/TableBackupClient.java:
##
@@ -112,7 +113,9 @@ protected void beginBackup(BackupManager backupManager, 
BackupInfo backupInfo)
 backupManager.setBackupInfo(backupInfo);
 // set the start timestamp of the overall backup
 long startTs = EnvironmentEdgeManager.currentTime();
+long committedWALsTs = BackupUtils.getReplicationCheckpoint(conn);

Review Comment:
   Makes sense, skipped



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-04 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2942705412

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 44s |  |  Docker mode activated.  |
   | -0 :warning: |  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 :green_heart: |  mvninstall  |   4m 12s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  shadedjars  |   6m  4s |  |  branch has no errors when 
building our shaded downstream artifacts.  |
   | -0 :warning: |  patch  |   6m 12s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 10s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 19s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 59s |  |  patch has no errors when 
building our shaded downstream artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  |  22m 18s | 
[/patch-unit-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   |  |   |  44m 58s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 6880c3f19d15 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 / 64fd7b82065725eaa59bb23cdcc510bc5fa677a7 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/testReport/
 |
   | Max. process+thread count | 3734 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-06-04 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2942684399

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 30s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any 
anti-patterns.  |
    _ HBASE-28957 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 53s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  |  HBASE-28957 passed  |
   | -0 :warning: |  checkstyle  |   0m 10s | 
[/buildtool-branch-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/artifact/yetus-general-check/output/buildtool-branch-checkstyle-hbase-backup.txt)
 |  The patch fails to run checkstyle in hbase-backup  |
   | +1 :green_heart: |  spotbugs  |   0m 34s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotless  |   0m 47s |  |  branch has no errors when 
running spotless:check.  |
   | -0 :warning: |  patch  |   0m 54s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m  2s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  |  the patch passed  |
   | -0 :warning: |  javac  |   0m 29s | 
[/results-compile-javac-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/artifact/yetus-general-check/output/results-compile-javac-hbase-backup.txt)
 |  hbase-backup generated 2 new + 109 unchanged - 0 fixed = 111 total (was 
109)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m  9s | 
[/buildtool-patch-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/artifact/yetus-general-check/output/buildtool-patch-checkstyle-hbase-backup.txt)
 |  The patch fails to run checkstyle in hbase-backup  |
   | +1 :green_heart: |  spotbugs  |   0m 35s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m  9s |  |  Patch does not cause any 
errors with Hadoop 3.3.6 3.4.0.  |
   | +1 :green_heart: |  spotless  |   0m 44s |  |  patch has no errors when 
running spotless:check.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m  9s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   |  31m 33s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/7/artifact/yetus-general-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell 
detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 4f904cfc4259 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 / 64fd7b82065725eaa59bb23cdcc510bc5fa677a7 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 81 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-31 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2117693576


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java:
##
@@ -517,28 +517,47 @@ public String backupTables(BackupRequest request) throws 
IOException {
 
 String backupId = BackupRestoreConstants.BACKUPID_PREFIX + 
EnvironmentEdgeManager.currentTime();
 if (type == BackupType.INCREMENTAL) {
-  Set incrTableSet;
-  try (BackupSystemTable table = new BackupSystemTable(conn)) {
-incrTableSet = table.getIncrementalBackupTableSet(targetRootDir);
-  }
+  if (request.isContinuousBackupEnabled()) {
+Set continuousBackupTableSet;
+try (BackupSystemTable table = new BackupSystemTable(conn)) {
+  continuousBackupTableSet = 
table.getContinuousBackupTableSet().keySet();
+}
+if (continuousBackupTableSet.isEmpty()) {
+  String msg = "Continuous backup table set contains no tables. "
++ "You need to run Continuous backup first "
++ (tableList != null ? "on " + StringUtils.join(tableList, ",") : 
"");
+  throw new IOException(msg);
+}
+if (!continuousBackupTableSet.containsAll(tableList)) {
+  String extraTables = StringUtils.join(tableList, ",");
+  String msg = "Some tables (" + extraTables + ") haven't gone through 
Continuous backup. "
++ "Perform Continuous backup on " + extraTables + " first, " + 
"then retry the command";
+  throw new IOException(msg);
+}
+  } else {
+Set incrTableSet;
+try (BackupSystemTable table = new BackupSystemTable(conn)) {
+  incrTableSet = table.getIncrementalBackupTableSet(targetRootDir);
+}
 
-  if (incrTableSet.isEmpty()) {
-String msg =
-  "Incremental backup table set contains no tables. " + "You need to 
run full backup first "
+if (incrTableSet.isEmpty()) {
+  String msg = "Incremental backup table set contains no tables. "
++ "You need to run full backup first "
 + (tableList != null ? "on " + StringUtils.join(tableList, ",") : 
"");
 
-throw new IOException(msg);
-  }
-  if (tableList != null) {
-tableList.removeAll(incrTableSet);
-if (!tableList.isEmpty()) {
-  String extraTables = StringUtils.join(tableList, ",");
-  String msg = "Some tables (" + extraTables + ") haven't gone through 
full backup. "
-+ "Perform full backup on " + extraTables + " first, " + "then 
retry the command";
   throw new IOException(msg);
 }
+if (tableList != null) {
+  tableList.removeAll(incrTableSet);
+  if (!tableList.isEmpty()) {
+String extraTables = StringUtils.join(tableList, ",");
+String msg = "Some tables (" + extraTables + ") haven't gone 
through full backup. "
+  + "Perform full backup on " + extraTables + " first, " + "then 
retry the command";
+throw new IOException(msg);
+  }
+}
+tableList = Lists.newArrayList(incrTableSet);

Review Comment:
   Reason: [HBASE-14038] The incremental backup is controlled by the 
'incremental backup table set'.
   For example, if the table set contains (table1, table2, table3). Incremental 
backup will back up the WALs, which cover all the tables in the table set.
   It is to avoid copying the same set of WALs, which would the likely case if 
you backup up table1, then backup table2.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-31 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2117693722


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -262,9 +271,13 @@ public void execute() throws IOException, 
ColumnFamilyMismatchException {
   // case PREPARE_INCREMENTAL:
   beginBackup(backupManager, backupInfo);
   backupInfo.setPhase(BackupPhase.PREPARE_INCREMENTAL);
-  LOG.debug("For incremental backup, current table set is "
-+ backupManager.getIncrementalBackupTableSet());
-  newTimestamps = ((IncrementalBackupManager) 
backupManager).getIncrBackupLogFileMap();
+  if (backupInfo.isContinuousBackupEnabled()) {
+LOG.debug("For incremental backup, current table set is " + 
backupInfo.getTables());
+  } else {
+LOG.debug("For incremental backup, current table set is "
+  + backupManager.getIncrementalBackupTableSet());
+newTimestamps = ((IncrementalBackupManager) 
backupManager).getIncrBackupLogFileMap();
+  }

Review Comment:
   Reason: [[HBASE-14038](https://issues.apache.org/jira/browse/HBASE-14038)] 
The incremental backup is controlled by the 'incremental backup table set'.
   For example, if the table set contains (table1, table2, table3). Incremental 
backup will back up the WALs, which cover all the tables in the table set.
   It is to avoid copying the same set of WALs, which would the likely case if 
you backup up table1, then backup table2.
   
   Added in comment in code too



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-30 Thread via GitHub



taklwu commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2116373980


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -149,12 +149,20 @@ public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
 
+  // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
+  // will be part of the snapshot being taken). We gather this list before 
taking the actual
+  // snapshots for the same reason as the log rolls.
+  List bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);

Review Comment:
   nit: without this steps, will the continuous backup fail? or just have more 
data leftover ? also can we have a unit tests for it? 



##
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupWithContinuous.java:
##
@@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.*;
+import static 
org.apache.hadoop.hbase.backup.replication.ContinuousBackupReplicationEndpoint.CONF_STAGED_WAL_FLUSH_INITIAL_DELAY;
+import static 
org.apache.hadoop.hbase.backup.replication.ContinuousBackupReplicationEndpoint.CONF_STAGED_WAL_FLUSH_INTERVAL;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.*;

Review Comment:
   ditto for `*` , please change your IDE setup



##
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupWithContinuous.java:
##
@@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.*;
+import static 
org.apache.hadoop.hbase.backup.replication.ContinuousBackupReplicationEndpoint.CONF_STAGED_WAL_FLUSH_INITIAL_DELAY;
+import static 
org.apache.hadoop.hbase.backup.replication.ContinuousBackupReplicationEndpoint.CONF_STAGED_WAL_FLUSH_INTERVAL;
+import static org.junit.Assert.*;

Review Comment:
   ditto for `*` , please change your IDE setup



##
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupWithContinuous.java:
##
@@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.*;
+import static 
org.apache.hadoop.hbase.backup.replication.ContinuousBackupReplicationEnd

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-28 Thread via GitHub



kgeisz commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2112940251


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -362,23 +385,89 @@ protected void deleteBulkLoadDirectory() throws 
IOException {
   }
 
   protected void convertWALsToHFiles() throws IOException {
-// get incremental backup file list and prepare parameters for DistCp
-List incrBackupFileList = backupInfo.getIncrBackupFileList();
-// Get list of tables in incremental backup set
-Set tableSet = backupManager.getIncrementalBackupTableSet();
-// filter missing files out (they have been copied by previous backups)
-incrBackupFileList = filterMissingFiles(incrBackupFileList);
-List tableList = new ArrayList();
-for (TableName table : tableSet) {
-  // Check if table exists
-  if (tableExists(table, conn)) {
-tableList.add(table.getNameAsString());
-  } else {
-LOG.warn("Table " + table + " does not exists. Skipping in WAL 
converter");
+long previousBackupTs = 0L;
+if (backupInfo.isContinuousBackupEnabled()) {
+  Set tableSet = backupInfo.getTables();
+  List backupInfos = backupManager.getBackupHistory(true);
+  for (TableName table : tableSet) {
+for (BackupInfo backup : backupInfos) {
+  // find previous backup for this table
+  if (backup.getTables().contains(table)) {
+LOG.info("Found previous backup of type {} with id {} for table 
{}", backup.getType(),
+  backup.getBackupId(), table.getNameAsString());
+List walBackupFileList;
+if (backup.getType() == BackupType.FULL) {
+  previousBackupTs = backup.getStartTs();
+} else {
+  previousBackupTs = backup.getIncrCommittedWalTs();
+}
+walBackupFileList = getBackupLogs(previousBackupTs);
+walToHFiles(walBackupFileList, 
Arrays.asList(table.getNameAsString()),
+  previousBackupTs);
+break;
+  }
+}
   }
+} else {
+  // get incremental backup file list and prepare parameters for DistCp
+  List incrBackupFileList = backupInfo.getIncrBackupFileList();
+  // Get list of tables in incremental backup set
+  Set tableSet = backupManager.getIncrementalBackupTableSet();
+  // filter missing files out (they have been copied by previous backups)
+  incrBackupFileList = filterMissingFiles(incrBackupFileList);
+  List tableList = new ArrayList();
+  for (TableName table : tableSet) {
+// Check if table exists
+if (tableExists(table, conn)) {
+  tableList.add(table.getNameAsString());
+} else {
+  LOG.warn("Table " + table + " does not exists. Skipping in WAL 
converter");
+}
+  }
+  walToHFiles(incrBackupFileList, tableList, previousBackupTs);
 }
-walToHFiles(incrBackupFileList, tableList);
+  }
 
+  private List getBackupLogs(long startTs) throws IOException {
+// get log files from backup dir
+String walBackupDir = conf.get(CONF_CONTINUOUS_BACKUP_WAL_DIR);
+if (walBackupDir == null || walBackupDir.isEmpty()) {
+  throw new IOException(
+"Incremental backup requires the WAL backup directory " + 
CONF_CONTINUOUS_BACKUP_WAL_DIR);
+}
+List resultLogFiles = new ArrayList<>();
+Path walBackupPath = new Path(walBackupDir);
+FileSystem backupFs = FileSystem.get(walBackupPath.toUri(), conf);
+FileStatus[] dayDirs = backupFs.listStatus(new Path(walBackupDir, 
WALS_DIR));
+SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT);
+dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
+
+for (FileStatus dayDir : dayDirs) {
+  if (!dayDir.isDirectory()) {
+continue; // Skip files, only process directories
+  }
+
+  String dirName = dayDir.getPath().getName();
+  try {
+Date dirDate = dateFormat.parse(dirName);
+long dirStartTime = dirDate.getTime(); // Start of that day (00:00:00)
+long dirEndTime = dirStartTime + ONE_DAY_IN_MILLISECONDS - 1; // End 
time of the day
+// (23:59:59)

Review Comment:
   This is comment a little odd.  I imagine `mvn spotless:apply` is what put it 
on multiple lines.  What if you shorten the comment a little?
   
   ```suggestion
   long dirEndTime = dirStartTime + ONE_DAY_IN_MILLISECONDS - 1; // End 
time of day (23:59:59)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-28 Thread via GitHub



kgeisz commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2112770649


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java:
##
@@ -517,28 +517,47 @@ public String backupTables(BackupRequest request) throws 
IOException {
 
 String backupId = BackupRestoreConstants.BACKUPID_PREFIX + 
EnvironmentEdgeManager.currentTime();
 if (type == BackupType.INCREMENTAL) {
-  Set incrTableSet;
-  try (BackupSystemTable table = new BackupSystemTable(conn)) {
-incrTableSet = table.getIncrementalBackupTableSet(targetRootDir);
-  }
+  if (request.isContinuousBackupEnabled()) {
+Set continuousBackupTableSet;
+try (BackupSystemTable table = new BackupSystemTable(conn)) {
+  continuousBackupTableSet = 
table.getContinuousBackupTableSet().keySet();
+}
+if (continuousBackupTableSet.isEmpty()) {
+  String msg = "Continuous backup table set contains no tables. "
++ "You need to run Continuous backup first "
++ (tableList != null ? "on " + StringUtils.join(tableList, ",") : 
"");
+  throw new IOException(msg);
+}
+if (!continuousBackupTableSet.containsAll(tableList)) {
+  String extraTables = StringUtils.join(tableList, ",");
+  String msg = "Some tables (" + extraTables + ") haven't gone through 
Continuous backup. "
++ "Perform Continuous backup on " + extraTables + " first, " + 
"then retry the command";

Review Comment:
   nit
   ```suggestion
   + "Perform Continuous backup on " + extraTables + " first, then 
retry the command";
   ```



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java:
##
@@ -517,28 +517,47 @@ public String backupTables(BackupRequest request) throws 
IOException {
 
 String backupId = BackupRestoreConstants.BACKUPID_PREFIX + 
EnvironmentEdgeManager.currentTime();
 if (type == BackupType.INCREMENTAL) {
-  Set incrTableSet;
-  try (BackupSystemTable table = new BackupSystemTable(conn)) {
-incrTableSet = table.getIncrementalBackupTableSet(targetRootDir);
-  }
+  if (request.isContinuousBackupEnabled()) {
+Set continuousBackupTableSet;
+try (BackupSystemTable table = new BackupSystemTable(conn)) {
+  continuousBackupTableSet = 
table.getContinuousBackupTableSet().keySet();
+}
+if (continuousBackupTableSet.isEmpty()) {
+  String msg = "Continuous backup table set contains no tables. "
++ "You need to run Continuous backup first "
++ (tableList != null ? "on " + StringUtils.join(tableList, ",") : 
"");
+  throw new IOException(msg);
+}
+if (!continuousBackupTableSet.containsAll(tableList)) {
+  String extraTables = StringUtils.join(tableList, ",");
+  String msg = "Some tables (" + extraTables + ") haven't gone through 
Continuous backup. "
++ "Perform Continuous backup on " + extraTables + " first, " + 
"then retry the command";
+  throw new IOException(msg);
+}
+  } else {
+Set incrTableSet;
+try (BackupSystemTable table = new BackupSystemTable(conn)) {
+  incrTableSet = table.getIncrementalBackupTableSet(targetRootDir);
+}
 
-  if (incrTableSet.isEmpty()) {
-String msg =
-  "Incremental backup table set contains no tables. " + "You need to 
run full backup first "
+if (incrTableSet.isEmpty()) {
+  String msg = "Incremental backup table set contains no tables. "
++ "You need to run full backup first "
 + (tableList != null ? "on " + StringUtils.join(tableList, ",") : 
"");
 
-throw new IOException(msg);
-  }
-  if (tableList != null) {
-tableList.removeAll(incrTableSet);
-if (!tableList.isEmpty()) {
-  String extraTables = StringUtils.join(tableList, ",");
-  String msg = "Some tables (" + extraTables + ") haven't gone through 
full backup. "
-+ "Perform full backup on " + extraTables + " first, " + "then 
retry the command";
   throw new IOException(msg);
 }
+if (tableList != null) {
+  tableList.removeAll(incrTableSet);
+  if (!tableList.isEmpty()) {
+String extraTables = StringUtils.join(tableList, ",");
+String msg = "Some tables (" + extraTables + ") haven't gone 
through full backup. "
+  + "Perform full backup on " + extraTables + " first, " + "then 
retry the command";

Review Comment:
   nit
   ```suggestion
 + "Perform full backup on " + extraTables + " first, then 
retry the command";
   ```



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -262,9 +273,18 @@ public void execute() throws IOException, 
ColumnF

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-11 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2870971883

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 30s |  |  Docker mode activated.  |
   | -0 :warning: |  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 :green_heart: |  mvninstall  |   3m 50s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 59s |  |  branch has no errors when 
building our shaded downstream artifacts.  |
   | -0 :warning: |  patch  |   6m  7s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m  4s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 19s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 58s |  |  patch has no errors when 
building our shaded downstream artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  |  22m 44s | 
[/patch-unit-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   |  |   |  44m 12s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 06b26e8ba4da 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 / 39a4f8ce6587df70e4429112b277f867aadc8b66 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/testReport/
 |
   | Max. process+thread count | 3663 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-11 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2870948474

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 31s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any 
anti-patterns.  |
    _ HBASE-28957 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 14s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  checkstyle  |   0m 11s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotbugs  |   0m 31s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotless  |   0m 47s |  |  branch has no errors when 
running spotless:check.  |
   | -0 :warning: |  patch  |   0m 54s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m  3s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  |  the patch passed  |
   | -0 :warning: |  javac  |   0m 29s | 
[/results-compile-javac-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/artifact/yetus-general-check/output/results-compile-javac-hbase-backup.txt)
 |  hbase-backup generated 5 new + 108 unchanged - 0 fixed = 113 total (was 
108)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m  9s | 
[/results-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/artifact/yetus-general-check/output/results-checkstyle-hbase-backup.txt)
 |  hbase-backup: The patch generated 4 new + 1 unchanged - 0 fixed = 5 total 
(was 1)  |
   | +1 :green_heart: |  spotbugs  |   0m 36s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m 35s |  |  Patch does not cause any 
errors with Hadoop 3.3.6 3.4.0.  |
   | +1 :green_heart: |  spotless  |   0m 44s |  |  patch has no errors when 
running spotless:check.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 10s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   |  31m 21s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/artifact/yetus-general-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell 
detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 998ad25e3ceb 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 / 39a4f8ce6587df70e4429112b277f867aadc8b66 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 82 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/6/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-05-06 Thread via GitHub



vinayakphegde commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2076784304


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/TableBackupClient.java:
##
@@ -112,7 +113,9 @@ protected void beginBackup(BackupManager backupManager, 
BackupInfo backupInfo)
 backupManager.setBackupInfo(backupInfo);
 // set the start timestamp of the overall backup
 long startTs = EnvironmentEdgeManager.currentTime();
+long committedWALsTs = BackupUtils.getReplicationCheckpoint(conn);

Review Comment:
   Then let's skip the calculation. Below, you're setting `committedWALsTs` 
using `backupInfo.setIncrCommittedWalTs(committedWALsTs);`
   In the case of a non-continuous backup, `committedWALsTs` should be 0, 
right? So we shouldn't set it here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-28 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2834464500

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 32s |  |  Docker mode activated.  |
   | -0 :warning: |  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 :green_heart: |  mvninstall  |   4m 23s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  shadedjars  |   7m  3s |  |  branch has no errors when 
building our shaded downstream artifacts.  |
   | -0 :warning: |  patch  |   7m 12s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 26s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m  0s |  |  patch has no errors when 
building our shaded downstream artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  |  23m  2s | 
[/patch-unit-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   |  |   |  48m 29s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 8d32c15ae0ea 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 / 7b26cb03cbbb37f97e2b38c1f2028ab06e9f9e34 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/testReport/
 |
   | Max. process+thread count | 3750 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-28 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2834405500

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 53s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any 
anti-patterns.  |
    _ HBASE-28957 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  checkstyle  |   0m 12s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotbugs  |   0m 35s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotless  |   0m 52s |  |  branch has no errors when 
running spotless:check.  |
   | -0 :warning: |  patch  |   0m 59s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 14s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 31s |  |  the patch passed  |
   | -0 :warning: |  javac  |   0m 31s | 
[/results-compile-javac-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/artifact/yetus-general-check/output/results-compile-javac-hbase-backup.txt)
 |  hbase-backup generated 6 new + 108 unchanged - 0 fixed = 114 total (was 
108)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m 11s | 
[/results-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/artifact/yetus-general-check/output/results-checkstyle-hbase-backup.txt)
 |  hbase-backup: The patch generated 4 new + 0 unchanged - 0 fixed = 4 total 
(was 0)  |
   | +1 :green_heart: |  spotbugs  |   0m 37s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m  4s |  |  Patch does not cause any 
errors with Hadoop 3.3.6 3.4.0.  |
   | +1 :green_heart: |  spotless  |   0m 45s |  |  patch has no errors when 
running spotless:check.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 10s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   |  32m 37s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/artifact/yetus-general-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell 
detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux a8d0439162fa 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 / 7b26cb03cbbb37f97e2b38c1f2028ab06e9f9e34 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 84 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/5/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-27 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2062827276


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java:
##
@@ -124,6 +124,11 @@ public enum BackupPhase {
*/
   private long completeTs;
 
+  /**
+   * Committed WAL timestamp for incremental backup
+   */
+  private long incrCommittedWalTs;

Review Comment:
   0



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-27 Thread via GitHub



ankitsol commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2062827182


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/TableBackupClient.java:
##
@@ -112,7 +113,9 @@ protected void beginBackup(BackupManager backupManager, 
BackupInfo backupInfo)
 backupManager.setBackupInfo(backupInfo);
 // set the start timestamp of the overall backup
 long startTs = EnvironmentEdgeManager.currentTime();
+long committedWALsTs = BackupUtils.getReplicationCheckpoint(conn);

Review Comment:
   Yes, not applicable



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-24 Thread via GitHub



vinayakphegde commented on code in PR #6788:
URL: https://github.com/apache/hbase/pull/6788#discussion_r2056431707


##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupManager.java:
##
@@ -434,4 +434,15 @@ public void addContinuousBackupTableSet(Set 
tables, long startTimesta
 throws IOException {
 systemTable.addContinuousBackupTableSet(tables, startTimestamp);
   }
+
+  /**
+   * Retrieves the current set of tables covered by continuous backup along 
with the timestamp
+   * indicating when continuous backup started for each table.
+   * @return a map where the key is the table name and the value is the 
timestamp representing the
+   * start time of continuous backup for that table.
+   * @throws IOException if an I/O error occurs while accessing the backup 
system table.
+   */
+  public Map getContinuousBackupTableSet() throws IOException 
{

Review Comment:
   where are we using this method? 



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java:
##
@@ -565,7 +584,7 @@ public String backupTables(BackupRequest request) throws 
IOException {
 }
   }
   if (nonExistingTableList != null) {
-if (type == BackupType.INCREMENTAL) {
+if (type == BackupType.INCREMENTAL && 
!request.isContinuousBackupEnabled()) {

Review Comment:
   maybe we can put some comment to explain what is happening here? why it is 
okay for normal incremental backup to go through the process even if some 
tables are not existing and why it is not okay in case of continuous backup 
enabled incremental backup.



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##
@@ -262,9 +271,13 @@ public void execute() throws IOException, 
ColumnFamilyMismatchException {
   // case PREPARE_INCREMENTAL:
   beginBackup(backupManager, backupInfo);
   backupInfo.setPhase(BackupPhase.PREPARE_INCREMENTAL);
-  LOG.debug("For incremental backup, current table set is "
-+ backupManager.getIncrementalBackupTableSet());
-  newTimestamps = ((IncrementalBackupManager) 
backupManager).getIncrBackupLogFileMap();
+  if (backupInfo.isContinuousBackupEnabled()) {
+LOG.debug("For incremental backup, current table set is " + 
backupInfo.getTables());
+  } else {
+LOG.debug("For incremental backup, current table set is "
+  + backupManager.getIncrementalBackupTableSet());
+newTimestamps = ((IncrementalBackupManager) 
backupManager).getIncrBackupLogFileMap();
+  }

Review Comment:
   We should clarify this point in the comment or elsewhere in the code:
   
   - When continuous backup is enabled, incremental backups only consider the 
tables explicitly specified by the user.
   - For regular incremental backups, we consider all tables marked for 
incremental backup under the given backup root.
   
   We should also investigate the reasoning behind this behavior to gain better 
clarity.



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java:
##
@@ -124,6 +124,11 @@ public enum BackupPhase {
*/
   private long completeTs;
 
+  /**
+   * Committed WAL timestamp for incremental backup
+   */
+  private long incrCommittedWalTs;

Review Comment:
   what will be the value in case of normal incremental backup?



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/TableBackupClient.java:
##
@@ -112,7 +113,9 @@ protected void beginBackup(BackupManager backupManager, 
BackupInfo backupInfo)
 backupManager.setBackupInfo(backupInfo);
 // set the start timestamp of the overall backup
 long startTs = EnvironmentEdgeManager.currentTime();
+long committedWALsTs = BackupUtils.getReplicationCheckpoint(conn);

Review Comment:
   this will not be applicable in normal incremental backup, right?



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/replication/ContinuousBackupReplicationEndpoint.java:
##
@@ -282,8 +283,9 @@ private void backupWalEntries(long day, List 
walEntries) throws IOExc
 
   private FSHLogProvider.Writer createWalWriter(long dayInMillis) {
 // Convert dayInMillis to "-MM-dd" format
-SimpleDateFormat dateFormat = new SimpleDateFormat(DATE_FORMAT);

Review Comment:
   won't just `dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));` work?



##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##
@@ -148,13 +148,16 @@ protected void snapshotCopy(BackupInfo backupInfo) throws 
IOException {
   public void execute() throws IOException {
 try (Admin admin = conn.getAdmin()) {
   beginBackup(backupManager, backupInfo);
+  initializeBackupStartCode(backupManager);
+  performLogRoll(admin);

Review Comment:
   Why are we doing these steps again here? 
   is it also needed for continuous backup enabled full

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-21 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2820054852

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 29s |  |  Docker mode activated.  |
   | -0 :warning: |  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 :green_heart: |  mvninstall  |   3m 33s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  shadedjars  |   6m 21s |  |  branch has no errors when 
building our shaded downstream artifacts.  |
   | -0 :warning: |  patch  |   6m 29s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 35s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 26s |  |  patch has no errors when 
building our shaded downstream artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  |  23m  2s | 
[/patch-unit-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   |  |   |  45m 39s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 4877413ba229 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 / 8e1d0331de3698713527bd6184be748533cd3984 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/testReport/
 |
   | Max. process+thread count | 3771 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-21 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2819997241

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 32s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any 
anti-patterns.  |
    _ HBASE-28957 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 47s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotbugs  |   0m 38s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotless  |   0m 51s |  |  branch has no errors when 
running spotless:check.  |
   | -0 :warning: |  patch  |   0m 59s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 32s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  |  the patch passed  |
   | -0 :warning: |  javac  |   0m 34s | 
[/results-compile-javac-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/artifact/yetus-general-check/output/results-compile-javac-hbase-backup.txt)
 |  hbase-backup generated 5 new + 108 unchanged - 0 fixed = 113 total (was 
108)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m 10s | 
[/results-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/artifact/yetus-general-check/output/results-checkstyle-hbase-backup.txt)
 |  hbase-backup: The patch generated 5 new + 1 unchanged - 0 fixed = 6 total 
(was 1)  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m 20s |  |  Patch does not cause any 
errors with Hadoop 3.3.6 3.4.0.  |
   | +1 :green_heart: |  spotless  |   0m 50s |  |  patch has no errors when 
running spotless:check.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 10s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   |  32m 37s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/artifact/yetus-general-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell 
detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 156e6abd495c 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 / 8e1d0331de3698713527bd6184be748533cd3984 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 83 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/4/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-01 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2771264479

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 31s |  |  Docker mode activated.  |
   | -0 :warning: |  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 :green_heart: |  mvninstall  |   3m 20s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 19s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  shadedjars  |   5m 58s |  |  branch has no errors when 
building our shaded downstream artifacts.  |
   | -0 :warning: |  patch  |   6m  6s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   3m  0s | 
[/patch-mvninstall-root.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-jdk17-hadoop3-check/output/patch-mvninstall-root.txt)
 |  root in the patch failed.  |
   | -1 :x: |  compile  |   0m 19s | 
[/patch-compile-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-jdk17-hadoop3-check/output/patch-compile-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   | -0 :warning: |  javac  |   0m 19s | 
[/patch-compile-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-jdk17-hadoop3-check/output/patch-compile-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 56s |  |  patch has no errors when 
building our shaded downstream artifacts.  |
    _ Other Tests _ |
   | -1 :x: |  unit  |   0m 19s | 
[/patch-unit-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   |  |   |  21m 12s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | javac javadoc unit compile shadedjars |
   | uname | Linux 4c6730c8e30b 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 / dc809b3acd390bc501dd67357e46eecc18651dae |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/testReport/
 |
   | Max. process+thread count | 82 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/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.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] HBASE-28990 Modify Incremental Backup for Continuous Backup [hbase]

2025-04-01 Thread via GitHub



Apache-HBase commented on PR #6788:
URL: https://github.com/apache/hbase/pull/6788#issuecomment-2771261172

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 30s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any 
anti-patterns.  |
    _ HBASE-28957 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 19s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  checkstyle  |   0m 10s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotbugs  |   0m 32s |  |  HBASE-28957 passed  |
   | +1 :green_heart: |  spotless  |   0m 46s |  |  branch has no errors when 
running spotless:check.  |
   | -0 :warning: |  patch  |   0m 52s |  |  Used diff version of patch file. 
Binary files and potentially other changes not applied. Please rebase and 
squash commits if necessary.  |
    _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   3m  3s | 
[/patch-mvninstall-root.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-mvninstall-root.txt)
 |  root in the patch failed.  |
   | -1 :x: |  compile  |   0m 25s | 
[/patch-compile-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-compile-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   | -0 :warning: |  javac  |   0m 25s | 
[/patch-compile-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-compile-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m  9s | 
[/results-checkstyle-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/results-checkstyle-hbase-backup.txt)
 |  hbase-backup: The patch generated 9 new + 1 unchanged - 0 fixed = 10 total 
(was 1)  |
   | -1 :x: |  spotbugs  |   0m 17s | 
[/patch-spotbugs-hbase-backup.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-spotbugs-hbase-backup.txt)
 |  hbase-backup in the patch failed.  |
   | -1 :x: |  hadoopcheck  |   2m 46s |  |  The patch causes 15 errors with 
Hadoop v3.3.6.  |
   | -1 :x: |  hadoopcheck  |   5m 30s |  |  The patch causes 15 errors with 
Hadoop v3.4.0.  |
   | -1 :x: |  spotless  |   0m 41s |  |  patch has 31 errors when running 
spotless:check, run spotless:apply to fix.  |
    _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m  9s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   |  17m 35s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hbase/pull/6788 |
   | Optional Tests | dupname asflicense javac spotbugs checkstyle codespell 
detsecrets compile hadoopcheck hbaseanti spotless |
   | uname | Linux 34b29038fe1c 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 / dc809b3acd390bc501dd67357e46eecc18651dae |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | hadoopcheck | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-javac-3.3.6.txt
 |
   | hadoopcheck | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-javac-3.4.0.txt
 |
   | spotless | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/artifact/yetus-general-check/output/patch-spotless.txt
 |
   | Max. process+thread count | 83 (vs. ulimit of 3) |
   | modules | C: hbase-backup U: hbase-backup |
   | Console output | 
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6788/3/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.
   
   


-- 
Th

47 matches

Mail list logo