diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java index 45fe961ac25b..f62f93a4ae4f 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java @@ -42,7 +42,6 @@ import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.cleaner.BaseLogCleanerDelegate; import org.apache.hadoop.hbase.master.region.MasterRegionFactory; -import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.procedure2.store.wal.WALProcedureStore; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; @@ -124,11 +123,7 @@ protected static BackupBoundaries calculatePreservationBoundary(List BackupBoundaries boundaries = builder.build(); if (LOG.isDebugEnabled()) { - LOG.debug("Boundaries defaultBoundary: {}", boundaries.getDefaultBoundary()); - for (Map.Entry entry : boundaries.getBoundaries().entrySet()) { - LOG.debug("Server: {}, WAL cleanup boundary: {}", entry.getKey().getHostName(), - entry.getValue()); - } + boundaries.logAllBoundaries(); } return boundaries; diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java index 6853fee78154..20c3996fa44e 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java @@ -30,26 +30,20 @@ import org.slf4j.LoggerFactory; /** - * Tracks time boundaries for WAL file cleanup during backup operations. Maintains the oldest - * timestamp per RegionServer included in any backup, enabling safe determination of which WAL files - * can be deleted without compromising backup integrity. + * Tracks WAL cleanup boundaries separately for each backup root to ensure WALs are only deleted + * when ALL backup roots no longer need them. A WAL file can only be deleted if it is older than the + * boundary for every backup root, protecting WALs needed by any root even when other roots have + * already backed up that host at a later timestamp. */ @InterfaceAudience.Private public class BackupBoundaries { private static final Logger LOG = LoggerFactory.getLogger(BackupBoundaries.class); + private static final BackupBoundaries EMPTY = new BackupBoundaries(Collections.emptyMap()); - // This map tracks, for every RegionServer, the least recent (= oldest / lowest timestamp) - // inclusion in any backup. In other words, it is the timestamp boundary up to which all backup - // roots have included the WAL in their backup. - private final Map boundaries; + private final Map rootBoundaries; - // The fallback cleanup boundary for RegionServers without explicit backup boundaries - // (e.g., servers that joined after backups began can be checked against this boundary) - private final long defaultBoundary; - - private BackupBoundaries(Map boundaries, long defaultBoundary) { - this.boundaries = boundaries; - this.defaultBoundary = defaultBoundary; + private BackupBoundaries(Map rootBoundaries) { + this.rootBoundaries = rootBoundaries; } public boolean isDeletable(Path walLogPath) { @@ -66,32 +60,16 @@ public boolean isDeletable(Path walLogPath) { Address address = Address.fromString(hostname); long pathTs = WAL.getTimestamp(walLogPath.getName()); - if (!boundaries.containsKey(address)) { - boolean isDeletable = pathTs <= defaultBoundary; - if (LOG.isDebugEnabled()) { - LOG.debug( - "Boundary for {} not found. isDeletable = {} based on defaultBoundary = {} and WAL ts of {}", - walLogPath, isDeletable, defaultBoundary, pathTs); - } - return isDeletable; - } - - long backupTs = boundaries.get(address); - if (pathTs <= backupTs) { - if (LOG.isDebugEnabled()) { - LOG.debug( - "WAL cleanup time-boundary found for server {}: {}. Ok to delete older file: {}", - address.getHostName(), pathTs, walLogPath); + for (Map.Entry entry : rootBoundaries.entrySet()) { + if (!entry.getValue().isDeletable(address, pathTs)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Backup root {} preventing deletion of {} with ts {}", entry.getKey(), + walLogPath, pathTs); + } + return false; } - return true; - } - - if (LOG.isDebugEnabled()) { - LOG.debug("WAL cleanup time-boundary found for server {}: {}. Keeping younger file: {}", - address.getHostName(), backupTs, walLogPath); } - - return false; + return true; } catch (Exception e) { LOG.warn("Error occurred while filtering file: {}. Ignoring cleanup of this log", walLogPath, e); @@ -99,56 +77,80 @@ public boolean isDeletable(Path walLogPath) { } } - public Map getBoundaries() { - return boundaries; - } - - public long getDefaultBoundary() { - return defaultBoundary; + public void logAllBoundaries() { + for (Map.Entry entry : rootBoundaries.entrySet()) { + entry.getValue().logBoundaries(entry.getKey()); + } } public static BackupBoundariesBuilder builder(long tsCleanupBuffer) { return new BackupBoundariesBuilder(tsCleanupBuffer); } + public static class BoundaryInfo { + private final Map boundaries; + private final long defaultBoundary; + + private BoundaryInfo(Map boundaries, long defaultBoundary) { + this.boundaries = boundaries; + this.defaultBoundary = defaultBoundary; + } + + public boolean isDeletable(Address address, long pathTs) { + Long boundary = boundaries.get(address); + if (boundary == null) { + return pathTs <= defaultBoundary; + } + return pathTs <= boundary; + } + + public void logBoundaries(String rootDir) { + LOG.debug("Backup root: {}, defaultBoundary: {}", rootDir, defaultBoundary); + for (Map.Entry entry : boundaries.entrySet()) { + LOG.debug("Backup root: {}, Server: {}, WAL cleanup boundary: {}", rootDir, + entry.getKey().getHostName(), entry.getValue()); + } + } + } + public static class BackupBoundariesBuilder { - private final Map boundaries = new HashMap<>(); + private final Map perRootStates = new HashMap<>(); private final long tsCleanupBuffer; - private long oldestStartTs = Long.MAX_VALUE; - private BackupBoundariesBuilder(long tsCleanupBuffer) { this.tsCleanupBuffer = tsCleanupBuffer; } /** - * Updates the boundaries based on the provided backup info. + * Updates the boundaries based on the provided backup info. Boundaries are tracked per backup + * root so that each root independently protects the WALs it still needs. * @param backupInfo the most recent completed backup info for a backup root, or if there is no * such completed backup, the currently running backup. */ public void update(BackupInfo backupInfo) { + PerRootState state = + perRootStates.computeIfAbsent(backupInfo.getBackupRootDir(), k -> new PerRootState()); + switch (backupInfo.getState()) { case COMPLETE: - // If a completed backup exists in the backup root, we want to protect all logs that - // have been created since the log-roll that happened for that backup. for (TableName table : backupInfo.getTableSetTimestampMap().keySet()) { for (Map.Entry entry : backupInfo.getTableSetTimestampMap().get(table) .entrySet()) { Address regionServerAddress = Address.fromString(entry.getKey()); Long logRollTs = entry.getValue(); - Long storedTs = boundaries.get(regionServerAddress); + Long storedTs = state.boundaries.get(regionServerAddress); if (storedTs == null || logRollTs < storedTs) { - boundaries.put(regionServerAddress, logRollTs); + state.boundaries.put(regionServerAddress, logRollTs); + if (logRollTs < state.oldestRollTs) { + state.oldestRollTs = logRollTs; + } } } } break; case RUNNING: - // If there is NO completed backup in the backup root, there are no persisted log-roll - // timestamps available yet. But, we still want to protect all files that have been - // created since the start of the currently running backup. - oldestStartTs = Math.min(oldestStartTs, backupInfo.getStartTs()); + state.oldestStartTs = Math.min(state.oldestStartTs, backupInfo.getStartTs()); break; default: throw new IllegalStateException("Unexpected backupInfo state: " + backupInfo.getState()); @@ -156,14 +158,28 @@ public void update(BackupInfo backupInfo) { } public BackupBoundaries build() { - if (boundaries.isEmpty()) { - long defaultBoundary = oldestStartTs - tsCleanupBuffer; - return new BackupBoundaries(Collections.emptyMap(), defaultBoundary); + if (perRootStates.isEmpty()) { + return EMPTY; } - long oldestRollTs = Collections.min(boundaries.values()); - long defaultBoundary = Math.min(oldestRollTs, oldestStartTs) - tsCleanupBuffer; - return new BackupBoundaries(boundaries, defaultBoundary); + Map rootBoundaries = new HashMap<>(); + for (Map.Entry entry : perRootStates.entrySet()) { + PerRootState state = entry.getValue(); + long defaultBoundary; + if (state.boundaries.isEmpty()) { + defaultBoundary = state.oldestStartTs - tsCleanupBuffer; + } else { + defaultBoundary = Math.min(state.oldestRollTs, state.oldestStartTs) - tsCleanupBuffer; + } + rootBoundaries.put(entry.getKey(), new BoundaryInfo(state.boundaries, defaultBoundary)); + } + return new BackupBoundaries(rootBoundaries); + } + + private static class PerRootState { + final Map boundaries = new HashMap<>(); + long oldestStartTs = Long.MAX_VALUE; + long oldestRollTs = Long.MAX_VALUE; } } } diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java index 3eaa1c633cd3..4ff7ad518d22 100644 --- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java @@ -294,6 +294,7 @@ public void testDoesNotDeleteWALsFromNewServers() throws Exception { public void testCanDeleteFileWithNewServerWALs() { BackupInfo backup = new BackupInfo(); backup.setState(BackupInfo.BackupState.COMPLETE); + backup.setBackupRootDir("s3://backup-root1"); backup.setTableSetTimestampMap(Collections.singletonMap(TableName.valueOf("table1"), Collections.singletonMap("server1:60020", 1000000L))); BackupBoundaries boundaries = @@ -324,6 +325,7 @@ public void testCanDeleteFileWithNewServerWALs() { public void testFirstBackupProtectsFiles() { BackupInfo backup = new BackupInfo(); backup.setBackupId("backup_1"); + backup.setBackupRootDir("s3://backup-root1"); backup.setState(BackupInfo.BackupState.RUNNING); backup.setStartTs(100L); // Running backups have no TableSetTimestampMap @@ -345,6 +347,7 @@ public void testFirstBackupProtectsFiles() { // In this case, a region-server-specific timestamp is available, so the buffer is not used. BackupInfo backup2 = new BackupInfo(); backup2.setBackupId("backup_2"); + backup2.setBackupRootDir("s3://backup-root1"); backup2.setState(BackupInfo.BackupState.COMPLETE); backup2.setStartTs(80L); backup2.setTableSetTimestampMap(Collections.singletonMap(TableName.valueOf("table1"), @@ -360,6 +363,57 @@ public void testFirstBackupProtectsFiles() { assertFalse(BackupLogCleaner.canDeleteFile(boundaries, path)); } + @Test + public void testMultiRootBoundariesProtectsWALsNeededByAnyRoot() { + // Root A has backed up both server1 and server2 + BackupInfo backupA = new BackupInfo(); + backupA.setBackupId("backup_A"); + backupA.setBackupRootDir("s3://root-A"); + backupA.setState(BackupInfo.BackupState.COMPLETE); + Map rootATimestamps = new HashMap<>(); + rootATimestamps.put("server1:60020", 2000L); + rootATimestamps.put("server2:60020", 2000L); + backupA.setTableSetTimestampMap( + Collections.singletonMap(TableName.valueOf("table1"), rootATimestamps)); + + // Root B has only backed up server1, at an earlier timestamp + BackupInfo backupB = new BackupInfo(); + backupB.setBackupId("backup_B"); + backupB.setBackupRootDir("s3://root-B"); + backupB.setState(BackupInfo.BackupState.COMPLETE); + backupB.setTableSetTimestampMap(Collections.singletonMap(TableName.valueOf("table1"), + Collections.singletonMap("server1:60020", 1000L))); + + BackupBoundaries boundaries = + BackupLogCleaner.calculatePreservationBoundary(Arrays.asList(backupA, backupB), 0L); + + // server1 WAL at 500: before both boundaries -> deletable + Path server1Old = new Path("/hbase/oldWALs/server1%2C60020%2C12345.500"); + assertTrue(BackupLogCleaner.canDeleteFile(boundaries, server1Old), + "WAL before both roots' boundaries should be deletable"); + + // server1 WAL at 1500: after root B's boundary (1000) -> NOT deletable + Path server1Between = new Path("/hbase/oldWALs/server1%2C60020%2C12345.1500"); + assertFalse(BackupLogCleaner.canDeleteFile(boundaries, server1Between), + "WAL after root B's boundary should NOT be deletable even though root A allows it"); + + // server2 WAL at 1500: root A has boundary 2000 (ok), but root B has never backed up + // server2 so root B's defaultBoundary (min of its rollTs=1000) should prevent deletion + Path server2Between = new Path("/hbase/oldWALs/server2%2C60020%2C12345.1500"); + assertFalse(BackupLogCleaner.canDeleteFile(boundaries, server2Between), + "WAL for server2 should NOT be deletable because root B hasn't backed up server2"); + + // server2 WAL at 500: before root B's defaultBoundary (1000) -> deletable + Path server2Old = new Path("/hbase/oldWALs/server2%2C60020%2C12345.500"); + assertTrue(BackupLogCleaner.canDeleteFile(boundaries, server2Old), + "WAL before all boundaries should be deletable"); + + // server2 WAL at 2500: after root A's boundary -> NOT deletable + Path server2New = new Path("/hbase/oldWALs/server2%2C60020%2C12345.2500"); + assertFalse(BackupLogCleaner.canDeleteFile(boundaries, server2New), + "WAL after root A's boundary should NOT be deletable"); + } + @Test public void testCleansUpHMasterWal() { Path path = new Path("/hbase/MasterData/WALs/hmaster,60000,1718808578163"); diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestOperationInterceptor.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestOperationInterceptor.java index d2a76817e647..ddafd774f941 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestOperationInterceptor.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestOperationInterceptor.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.fail; + import java.io.IOException; import java.util.ArrayList; import java.util.List; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestUnattainableBalancerCostGoal.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestUnattainableBalancerCostGoal.java index 5e95564b6fee..cf3f241cab89 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestUnattainableBalancerCostGoal.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestUnattainableBalancerCostGoal.java @@ -24,7 +24,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.ServerName; @@ -33,7 +32,6 @@ import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -41,6 +39,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet; + /** * If your minCostNeedsBalance is set too low, then the balancer should still eventually stop making * moves as further cost improvements become impossible, and balancer plan calculation becomes