taklwu commented on code in PR #7090:
URL: https://github.com/apache/hbase/pull/7090#discussion_r2141267031
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java:
##########
@@ -966,6 +988,34 @@ void updateBackupTableStartTimes(BackupSystemTable
sysTable, long cutoffTimestam
}
}
+ private void removeAllTablesFromContinuousBackup(BackupSystemTable
sysTable)
+ throws IOException {
+ Map<TableName, Long> allTables = sysTable.getContinuousBackupTableSet();
+ if (!allTables.isEmpty()) {
+ sysTable.removeContinuousBackupTableSet(allTables.keySet());
+ System.out.println("Removed all tables from continuous backup
metadata.");
+ }
+ }
+
+ private void deleteAllBackupWALFiles(Configuration conf, String
backupWalDir)
+ throws IOException {
+ try {
+ FileSystem fs = FileSystem.get(conf);
+ Path walPath = new Path(backupWalDir);
+ if (fs.exists(walPath)) {
+ FileStatus[] contents = fs.listStatus(walPath);
+ for (FileStatus item : contents) {
+ fs.delete(item.getPath(), true); // recursive delete of each child
+ }
+ System.out.println("Deleted all contents under WAL directory: " +
backupWalDir);
+ }
+ } catch (IOException e) {
+ System.out.println("WARNING: Failed to delete contents under WAL
directory: " + backupWalDir
Review Comment:
nit:
```suggestion
System.err.println("WARNING: Failed to delete contents under WAL
directory: " + backupWalDir
```
##########
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDeleteWithCleanup.java:
##########
@@ -96,6 +119,70 @@ public void testBackupDeleteWithCleanupLogic() throws
Exception {
// Step 5: Verify System Table Update
verifySystemTableUpdate(backupSystemTable, currentTime);
+
+ // Cleanup
+ deleteBackup(anotherBackupId);
+ }
+
+ @Test
+ public void testSingleBackupForceDelete() throws Exception {
+ // Step 1: Setup Backup Folders
+ long currentTime = EnvironmentEdgeManager.getDelegate().currentTime();
+ setupBackupFolders(currentTime);
+
+ // Log the directory structure before cleanup
+ logDirectoryStructure(fs, backupWalDir, "Directory structure BEFORE
cleanup:");
+
+ // Step 2: Simulate Backup Creation
Review Comment:
nit: add a check of `continuousBackupReplicationPeerExists` that replication
is enabled in Step 2 ?
##########
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java:
##########
@@ -414,6 +414,14 @@ protected BackupRequest createBackupRequest(BackupType
type, List<TableName> tab
return request;
}
+ protected BackupRequest createBackupRequest(BackupType type, List<TableName>
tables, String path,
Review Comment:
```suggestion
protected BackupRequest createBackupRequest(BackupType type,
List<TableName> tables, String rootDir,
```
##########
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDeleteWithCleanup.java:
##########
@@ -96,6 +119,70 @@ public void testBackupDeleteWithCleanupLogic() throws
Exception {
// Step 5: Verify System Table Update
verifySystemTableUpdate(backupSystemTable, currentTime);
+
+ // Cleanup
+ deleteBackup(anotherBackupId);
+ }
+
+ @Test
+ public void testSingleBackupForceDelete() throws Exception {
+ // Step 1: Setup Backup Folders
+ long currentTime = EnvironmentEdgeManager.getDelegate().currentTime();
+ setupBackupFolders(currentTime);
+
+ // Log the directory structure before cleanup
+ logDirectoryStructure(fs, backupWalDir, "Directory structure BEFORE
cleanup:");
+
+ // Step 2: Simulate Backup Creation
+ backupSystemTable.addContinuousBackupTableSet(Set.of(table1),
+ currentTime - (2 * ONE_DAY_IN_MILLISECONDS));
+
+ EnvironmentEdgeManager
+ .injectEdge(() -> System.currentTimeMillis() - (2 *
ONE_DAY_IN_MILLISECONDS));
+
+ String backupId = fullTableBackup(Lists.newArrayList(table1));
+ assertTrue(checkSucceeded(backupId));
+
+ // Step 3: Run Delete Command
+ deleteBackup(backupId);
+
+ // Log the directory structure after cleanup
+ logDirectoryStructure(fs, backupWalDir, "Directory structure AFTER
cleanup:");
+
+ // Step 4: Verify CONTINUOUS_BACKUP_REPLICATION_PEER is disabled
+ assertFalse("Backup replication peer should be disabled or removed",
+ continuousBackupReplicationPeerExists());
+
+ // Step 5: Verify that system table is updated to remove all the tables
+ Set<TableName> remainingTables =
backupSystemTable.getContinuousBackupTableSet().keySet();
+ assertTrue("System table should have no tables after force delete",
remainingTables.isEmpty());
Review Comment:
what is `force delete`? maybe change the wording to full backup is clear
```suggestion
assertTrue("System table should have no tables after full backup is
clear", remainingTables.isEmpty());
```
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java:
##########
@@ -1587,7 +1587,7 @@ private Delete createDeleteForIncrBackupTableSet(String
backupRoot) {
private Delete createDeleteForContinuousBackupTableSet(Set<TableName>
tables) {
Delete delete = new Delete(rowkey(CONTINUOUS_BACKUP_SET));
for (TableName tableName : tables) {
- delete.addColumns(META_FAMILY,
Bytes.toBytes(tableName.getNameAsString()));
+ delete.addColumn(META_FAMILY,
Bytes.toBytes(tableName.getNameAsString()));
Review Comment:
nit: the code between `addColumns` and `addColumn` is very similar, why do
we want to change it?
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java:
##########
@@ -944,6 +956,16 @@ long determineWALCleanupCutoffTime(BackupSystemTable
sysTable) throws IOExceptio
return 0;
}
+ private void disableContinuousBackupReplicationPeer(Admin admin) throws
IOException {
+ for (ReplicationPeerDescription peer : admin.listReplicationPeers()) {
+ if (peer.getPeerId().equals(CONTINUOUS_BACKUP_REPLICATION_PEER) &&
peer.isEnabled()) {
+ admin.disableReplicationPeer(CONTINUOUS_BACKUP_REPLICATION_PEER);
+ System.out.println("Disabled replication peer: " +
CONTINUOUS_BACKUP_REPLICATION_PEER);
Review Comment:
the current class does not use any logging interface, please ignore this
comment
--
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]
