Skip to content

(Not yet upstreamed) HBASE-29895: BackupBoundaries global coverage can allow premature WAL deletion when backup roots have different host coverage #259

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
hgromer merged 2 commits into from
Aug 17, 2026
Merged

(Not yet upstreamed) HBASE-29895: BackupBoundaries global coverage can allow premature WAL deletion when backup roots have different host coverage #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4003ae
HBASE-29895: BackupBoundaries global coverage can allow premature WAL…
Aug 17, 2026
e848888
retrigger CI
Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -124,11 +123,7 @@ protected static BackupBoundaries calculatePreservationBoundary(List<BackupInfo>
BackupBoundaries boundaries = builder.build();

if (LOG.isDebugEnabled()) {
LOG.debug("Boundaries defaultBoundary: {}", boundaries.getDefaultBoundary());
for (Map.Entry<Address, Long> entry : boundaries.getBoundaries().entrySet()) {
LOG.debug("Server: {}, WAL cleanup boundary: {}", entry.getKey().getHostName(),
entry.getValue());
}
boundaries.logAllBoundaries();
}

return boundaries;
Expand Down
140 changes: 78 additions & 62 deletions hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Address, Long> boundaries;
private final Map<String, BoundaryInfo> 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<Address, Long> boundaries, long defaultBoundary) {
this.boundaries = boundaries;
this.defaultBoundary = defaultBoundary;
private BackupBoundaries(Map<String, BoundaryInfo> rootBoundaries) {
this.rootBoundaries = rootBoundaries;
}

public boolean isDeletable(Path walLogPath) {
Expand All @@ -66,104 +60,126 @@ 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<String, BoundaryInfo> 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);
return false;
}
}

public Map<Address, Long> getBoundaries() {
return boundaries;
}

public long getDefaultBoundary() {
return defaultBoundary;
public void logAllBoundaries() {
for (Map.Entry<String, BoundaryInfo> 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<Address, Long> boundaries;
private final long defaultBoundary;

private BoundaryInfo(Map<Address, Long> 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<Address, Long> entry : boundaries.entrySet()) {
LOG.debug("Backup root: {}, Server: {}, WAL cleanup boundary: {}", rootDir,
entry.getKey().getHostName(), entry.getValue());
}
}
}

public static class BackupBoundariesBuilder {
private final Map<Address, Long> boundaries = new HashMap<>();
private final Map<String, PerRootState> 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<String, Long> 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());
}
}

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<String, BoundaryInfo> rootBoundaries = new HashMap<>();
for (Map.Entry<String, PerRootState> 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<Address, Long> boundaries = new HashMap<>();
long oldestStartTs = Long.MAX_VALUE;
long oldestRollTs = Long.MAX_VALUE;
}
}
}
54 changes: 54 additions & 0 deletions hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand All @@ -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"),
Expand All @@ -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<String, Long> 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");
Expand Down
1 change: 1 addition & 0 deletions hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestOperationInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

@hgromer hgromer Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Just ran spotless apply

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
Expand Down
4 changes: 2 additions & 2 deletions ...c/test/java/org/apache/hadoop/hbase/master/balancer/TestUnattainableBalancerCostGoal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

@hgromer hgromer Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Just ran spotless apply

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.ServerName;
Expand All @@ -33,14 +32,15 @@
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;
import org.junit.experimental.categories.Category;
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
Expand Down
Loading