Skip to content

Commit dd8a875

Browse files
authored
HBASE-29776 Log filtering in IncrementalBackupManager can lead to data loss (#8694) (#8775)
1 parent d941d53 commit dd8a875

8 files changed

Lines changed: 1190 additions & 28 deletions

File tree

‎hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,8 +639,9 @@ public List<BackupInfo> getBackupHistory(Order order, int n, BackupInfo.Filter..
639639

640640
/**
641641
* Write the current timestamps for each regionserver to backup system table after a successful
642-
* full or incremental backup. The saved timestamp is of the last log file that was backed up
643-
* already.
642+
* full or incremental backup. For a region server that took part in the backup's log roll, the
643+
* saved timestamp is the result of that roll. For a region server that did not (offline or
644+
* decommissioned), it is the creation time of the newest of its log files that was backed up.
644645
* @param tables tables
645646
* @param newTimestamps timestamps
646647
* @param backupRoot root directory path to backup

‎hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,20 @@
2323
import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.DEFAULT_BACKUP_MAX_ATTEMPTS;
2424
import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.JOB_NAME_CONF_KEY;
2525

26+
import com.google.errorprone.annotations.RestrictedApi;
2627
import java.io.IOException;
2728
import java.util.ArrayList;
29+
import java.util.HashMap;
30+
import java.util.HashSet;
2831
import java.util.List;
2932
import java.util.Map;
33+
import java.util.Set;
34+
import java.util.stream.Collectors;
35+
import org.apache.hadoop.fs.FileStatus;
36+
import org.apache.hadoop.fs.FileSystem;
37+
import org.apache.hadoop.fs.Path;
38+
import org.apache.hadoop.hbase.HConstants;
39+
import org.apache.hadoop.hbase.ServerName;
3040
import org.apache.hadoop.hbase.TableName;
3141
import org.apache.hadoop.hbase.backup.BackupCopyJob;
3242
import org.apache.hadoop.hbase.backup.BackupInfo;
@@ -38,7 +48,9 @@
3848
import org.apache.hadoop.hbase.backup.util.BackupUtils;
3949
import org.apache.hadoop.hbase.client.Admin;
4050
import org.apache.hadoop.hbase.client.Connection;
51+
import org.apache.hadoop.hbase.util.CommonFSUtils;
4152
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
53+
import org.apache.hadoop.hbase.wal.AbstractFSWALProvider;
4254
import org.apache.yetus.audience.InterfaceAudience;
4355
import org.slf4j.Logger;
4456
import org.slf4j.LoggerFactory;
@@ -146,9 +158,13 @@ public void execute() throws IOException {
146158
// snapshots for the same reason as the log rolls.
147159
List<BulkLoad> bulkLoadsToDelete = backupManager.readBulkloadRows(tableList);
148160

161+
Map<String, Long> previousLogRollsByHost = backupManager.readRegionServerLastLogRollResult();
149162
BackupUtils.logRoll(conn, backupInfo.getBackupRootDir(), conf);
150163

151-
newTimestamps = backupManager.readRegionServerLastLogRollResult();
164+
Map<String, Long> latestLogRollsByHost = backupManager.readRegionServerLastLogRollResult();
165+
Path walRootDir = CommonFSUtils.getWALRootDir(conf);
166+
newTimestamps = computeLogBoundaries(walRootDir.getFileSystem(conf), walRootDir,
167+
BackupUtils.getRolledHosts(previousLogRollsByHost, latestLogRollsByHost), admin);
152168

153169
// SNAPSHOT_TABLES:
154170
backupInfo.setPhase(BackupPhase.SNAPSHOT);
@@ -216,4 +232,80 @@ protected void snapshotTable(Admin admin, TableName tableName, String snapshotNa
216232
}
217233
throw new IOException("Failed to snapshot table " + tableName);
218234
}
235+
236+
/**
237+
* Computes the per-host log boundaries stored by this full backup, so that the next incremental
238+
* backup includes exactly the WALs whose edits may be missing from the snapshot. It must run
239+
* right after the log roll, before the snapshot is taken, so that a host starting later gets no
240+
* boundary and has all of its WALs included in the next incremental backup.
241+
* <p>
242+
* Boundaries are computed by {@link BackupUtils#computeLogBoundaries}:
243+
* <ul>
244+
* <li>A host that took part in this backup's log roll gets its roll result. WALs created after
245+
* the roll will be included in the next incremental backup, even though some of their edits may
246+
* already be in the snapshot, which is safe because deletes are replayed along with the
247+
* puts.</li>
248+
* <li>For any other host, its archived WALs and the WALs of dead servers are covered by the
249+
* snapshot, so the next incremental backup does not replay them.</li>
250+
* <li>The WALs of a live server that did not take part in the roll (for example one that started
251+
* during it) are pending, because they can still receive edits that are not in the snapshot.</li>
252+
* </ul>
253+
* If the master's server list is incomplete (for example right after a master failover), the
254+
* backup fails instead of treating live servers as dead.
255+
*/
256+
@RestrictedApi(
257+
explanation = "Package-private for test visibility only. Do not use outside tests.",
258+
link = "",
259+
allowedOnPath = "(.*/src/test/.*|.*/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java)")
260+
static Map<String, Long> computeLogBoundaries(FileSystem fs, Path walRootDir,
261+
Map<String, Long> rolledHosts, Admin admin) throws IOException {
262+
Path logDir = new Path(walRootDir, HConstants.HREGION_LOGDIR_NAME);
263+
Path oldLogDir = new Path(walRootDir, HConstants.HREGION_OLDLOGDIR_NAME);
264+
265+
Map<ServerName, List<String>> logsByServer = new HashMap<>();
266+
for (FileStatus serverLogDir : fs.listStatus(logDir)) {
267+
ServerName serverName =
268+
AbstractFSWALProvider.getServerNameFromWALDirectoryName(serverLogDir.getPath());
269+
if (serverName == null) {
270+
continue;
271+
}
272+
List<String> logs = logsByServer.computeIfAbsent(serverName, k -> new ArrayList<>());
273+
for (FileStatus log : fs.listStatus(serverLogDir.getPath())) {
274+
if (!AbstractFSWALProvider.isMetaFile(log.getPath())) {
275+
logs.add(log.getPath().toString());
276+
}
277+
}
278+
}
279+
280+
// The live servers are read only after the WAL directories are listed: a region server creates
281+
// its WAL directory only after the master registers it, so every live server whose directory
282+
// was listed is found. If the live servers do not include every host that took part in the
283+
// roll, the master's server list is incomplete, or a host stopped after the roll.
284+
Set<ServerName> live = new HashSet<>(admin.getRegionServers());
285+
Set<String> liveAddresses =
286+
live.stream().map(sn -> sn.getAddress().toString()).collect(Collectors.toSet());
287+
if (!liveAddresses.containsAll(rolledHosts.keySet())) {
288+
throw new IOException("Live region servers " + live + " do not include every host that took"
289+
+ " part in this backup's log roll " + rolledHosts.keySet() + ". The master's server list"
290+
+ " may be incomplete, for example during a master failover, or a region server stopped"
291+
+ " after the log roll, so the backup is failed to be retried.");
292+
}
293+
294+
List<String> coveredLogs = new ArrayList<>();
295+
List<String> pendingLogs = new ArrayList<>();
296+
for (Map.Entry<ServerName, List<String>> entry : logsByServer.entrySet()) {
297+
if (live.contains(entry.getKey())) {
298+
pendingLogs.addAll(entry.getValue());
299+
} else {
300+
coveredLogs.addAll(entry.getValue());
301+
}
302+
}
303+
304+
if (fs.exists(oldLogDir)) {
305+
BackupUtils.getFiles(fs, oldLogDir, coveredLogs,
306+
path -> !AbstractFSWALProvider.isMetaFile(path));
307+
}
308+
309+
return BackupUtils.computeLogBoundaries(rolledHosts, coveredLogs, pendingLogs);
310+
}
219311
}

‎hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalBackupManager.java‎

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import java.io.IOException;
2121
import java.util.ArrayList;
2222
import java.util.Collections;
23+
import java.util.HashSet;
2324
import java.util.List;
2425
import java.util.Map;
26+
import java.util.Set;
2527
import org.apache.hadoop.conf.Configuration;
2628
import org.apache.hadoop.fs.FileStatus;
2729
import org.apache.hadoop.fs.FileSystem;
@@ -54,12 +56,14 @@ public IncrementalBackupManager(Connection conn, Configuration conf) throws IOEx
5456
/**
5557
* Obtain the list of logs that need to be copied out for this incremental backup. The list is set
5658
* in BackupInfo.
57-
* @return The new HashMap of RS log time stamps after the log roll for this incremental backup.
59+
* @return The new map of RS log time stamps for this incremental backup, as computed by
60+
* {@link BackupUtils#computeLogBoundaries}: the included logs are covered, the logs held
61+
* back for a later backup (for example, logs still being split) are pending, and a host
62+
* that still has logs keeps its previous boundary if this backup gives it no new one.
5863
* @throws IOException exception
5964
*/
6065
public Map<String, Long> getIncrBackupLogFileMap() throws IOException {
6166
List<String> logList;
62-
Map<String, Long> newTimestamps;
6367
Map<String, Long> previousTimestampMins =
6468
BackupUtils.getRSLogTimestampMins(readLogTimestampMap());
6569

@@ -69,18 +73,28 @@ public Map<String, Long> getIncrBackupLogFileMap() throws IOException {
6973
+ "In order to create an incremental backup, at least one full backup is needed.");
7074
}
7175

76+
Map<String, Long> previousLogRollByHost = readRegionServerLastLogRollResult();
7277
LOG.info("Execute roll log procedure for incremental backup ...");
7378
BackupUtils.logRoll(conn, backupInfo.getBackupRootDir(), conf);
7479

75-
newTimestamps = readRegionServerLastLogRollResult();
80+
Map<String, Long> rolledHosts =
81+
BackupUtils.getRolledHosts(previousLogRollByHost, readRegionServerLastLogRollResult());
7682

77-
logList = getLogFilesForNewBackup(previousTimestampMins, newTimestamps, conf);
78-
logList = excludeProcV2WALs(logList);
83+
LogFileSelection selection = getLogFilesForNewBackup(previousTimestampMins, rolledHosts, conf);
84+
logList = excludeProcV2WALs(selection.included());
7985
backupInfo.setIncrBackupFileList(logList);
8086

87+
Map<String, Long> newTimestamps = BackupUtils.computeLogBoundaries(rolledHosts,
88+
previousTimestampMins, selection.hostsWithLogs(), logList, selection.heldBack());
89+
LOG.debug("Log boundaries for incremental backup {}: {}", backupInfo.getBackupId(),
90+
newTimestamps);
8191
return newTimestamps;
8292
}
8393

94+
private record LogFileSelection(List<String> included, List<String> heldBack,
95+
Set<String> hostsWithLogs) {
96+
}
97+
8498
private List<String> excludeProcV2WALs(List<String> logList) {
8599
List<String> list = new ArrayList<>();
86100
for (int i = 0; i < logList.size(); i++) {
@@ -97,15 +111,17 @@ private List<String> excludeProcV2WALs(List<String> logList) {
97111
}
98112

99113
/**
100-
* For each region server: get all log files newer than the last timestamps but not newer than the
101-
* newest timestamps.
114+
* Gather all log files that either: 1) are newer than the older timestamps, but not newer than
115+
* the newest timestamps, or 2) are archived logs whose host name does not occur in the newest
116+
* timestamps.
102117
* @param olderTimestamps the timestamp for each region server of the last backup.
103118
* @param newestTimestamps the timestamp for each region server that the backup should lead to.
104119
* @param conf the Hadoop and Hbase configuration
105-
* @return a list of log files to be backed up
120+
* @return the log files to be backed up, the log files held back for a later backup, and the
121+
* hosts that have any log files, including ones not backed up
106122
* @throws IOException exception
107123
*/
108-
private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
124+
private LogFileSelection getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
109125
Map<String, Long> newestTimestamps, Configuration conf) throws IOException {
110126
LOG.debug("In getLogFilesForNewBackup()\n" + "olderTimestamps: " + olderTimestamps
111127
+ "\n newestTimestamps: " + newestTimestamps);
@@ -119,6 +135,7 @@ private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
119135

120136
List<String> resultLogFiles = new ArrayList<>();
121137
List<String> newestLogs = new ArrayList<>();
138+
Set<String> hostsWithLogs = new HashSet<>();
122139

123140
/*
124141
* The old region servers and timestamps info we kept in backup system table may be out of sync
@@ -143,6 +160,7 @@ private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
143160
if (host == null) {
144161
continue;
145162
}
163+
hostsWithLogs.add(host);
146164
FileStatus[] logs;
147165
oldTimeStamp = olderTimestamps.get(host);
148166
// It is possible that there is no old timestamp in backup system table for this host if
@@ -165,17 +183,15 @@ private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
165183
resultLogFiles.add(currentLogFile);
166184
currentLogTS = BackupUtils.getCreationTime(log.getPath());
167185

168-
// If newestTimestamps.get(host) is null, means that
169-
// either RS (host) has been restarted recently with different port number
170-
// or RS is down (was decommisioned). In any case, we treat this
171-
// log file as eligible for inclusion into incremental backup log list
186+
// A host without a roll result did not take part in this backup's log roll: it is offline
187+
// (for example a dead server whose logs are still being split), or it started after the
188+
// roll. Its logs are held back, like logs newer than the roll result of a host that took
189+
// part. BackupUtils#computeLogBoundaries keeps the boundary of the host below held-back
190+
// logs, so a later backup includes them.
172191
Long ts = newestTimestamps.get(host);
173192
if (ts == null) {
174-
LOG.warn("ORPHAN log found: " + log + " host=" + host);
175-
LOG.debug("Known hosts (from newestTimestamps):");
176-
for (String s : newestTimestamps.keySet()) {
177-
LOG.debug(s);
178-
}
193+
LOG.debug("Holding back log {} of host {}, which did not take part in the log roll",
194+
log.getPath(), host);
179195
}
180196
if (ts == null || currentLogTS > ts) {
181197
newestLogs.add(currentLogFile);
@@ -198,6 +214,7 @@ private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
198214
if (host == null) {
199215
continue;
200216
}
217+
hostsWithLogs.add(host);
201218
currentLogTS = BackupUtils.getCreationTime(p);
202219
oldTimeStamp = olderTimestamps.get(host);
203220
/*
@@ -217,18 +234,15 @@ private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
217234
resultLogFiles.add(currentLogFile);
218235
}
219236

220-
// It is possible that a host in .oldlogs is an obsolete region server
221-
// so newestTimestamps.get(host) here can be null.
222-
// Even if these logs belong to a obsolete region server, we still need
223-
// to include they to avoid loss of edits for backup.
224237
Long newTimestamp = newestTimestamps.get(host);
225-
if (newTimestamp == null || currentLogTS > newTimestamp) {
238+
if (newTimestamp != null && currentLogTS > newTimestamp) {
226239
newestLogs.add(currentLogFile);
227240
}
228241
}
229-
// remove newest log per host because they are still in use
242+
// Held-back logs are newer than the roll result of their host, or belong to a host that did not
243+
// take part in the log roll. A later backup includes them.
230244
resultLogFiles.removeAll(newestLogs);
231-
return resultLogFiles;
245+
return new LogFileSelection(resultLogFiles, newestLogs, hostsWithLogs);
232246
}
233247

234248
static class NewestLogFilter implements PathFilter {

‎hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupUtils.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121
import java.io.IOException;
2222
import java.net.URLDecoder;
2323
import java.util.ArrayList;
24+
import java.util.Collection;
2425
import java.util.Collections;
2526
import java.util.Comparator;
2627
import java.util.HashMap;
2728
import java.util.List;
2829
import java.util.Map;
2930
import java.util.Map.Entry;
31+
import java.util.Set;
3032
import java.util.TreeSet;
3133
import java.util.function.Predicate;
3234
import java.util.stream.Collectors;
@@ -186,6 +188,86 @@ public static String parseHostNameFromLogFile(Path p) {
186188
}
187189
}
188190

191+
/**
192+
* Returns the log roll result of every region server that took part in the log roll that happened
193+
* between reading {@code previousLogRolls} and {@code latestLogRolls}. A region server that did
194+
* not take part (for example because it is offline) keeps its old roll result, which is never
195+
* removed, so it is recognized by its roll result not changing.
196+
* @param previousLogRolls roll results by host, read before the log roll
197+
* @param latestLogRolls roll results by host, read after the log roll
198+
* @return roll results by host, for the hosts that took part in the log roll
199+
*/
200+
public static Map<String, Long> getRolledHosts(Map<String, Long> previousLogRolls,
201+
Map<String, Long> latestLogRolls) {
202+
return latestLogRolls.entrySet().stream()
203+
.filter(entry -> !entry.getValue().equals(previousLogRolls.get(entry.getKey())))
204+
.collect(Collectors.toMap(Entry::getKey, Entry::getValue));
205+
}
206+
207+
/**
208+
* Computes the per-host log boundaries stored by a backup that has no previous boundaries to
209+
* carry forward, such as a full backup. See
210+
* {@link #computeLogBoundaries(Map, Map, Set, Collection, Collection)}.
211+
*/
212+
public static Map<String, Long> computeLogBoundaries(Map<String, Long> rolledHosts,
213+
Collection<String> coveredLogs, Collection<String> pendingLogs) throws IOException {
214+
return computeLogBoundaries(rolledHosts, Collections.emptyMap(), Collections.emptySet(),
215+
coveredLogs, pendingLogs);
216+
}
217+
218+
/**
219+
* Computes the per-host log boundaries stored by a backup. Everything up to a host's boundary is
220+
* covered by backups, so later incremental backups only include that host's logs that are newer.
221+
* <ul>
222+
* <li>A host that took part in the backup's log roll gets its roll result.</li>
223+
* <li>Any other host gets the creation time of its newest covered log, capped to just below its
224+
* oldest pending log, so that pending logs are included in a later backup.</li>
225+
* <li>A host without covered or pending logs keeps its previous boundary while it still has logs,
226+
* and otherwise gets no boundary.</li>
227+
* </ul>
228+
* Capping is needed because a host's logs can still be pending while some of its newer logs are
229+
* already covered, for example when a dead server's logs are split and archived out of order. As
230+
* a result, the next backup includes the covered logs that are newer than the cap a second time:
231+
* losing the pending logs is worse than repeating edits that are already backed up. A host is
232+
* also kept while any of its logs are pending, even if none are covered, because without a
233+
* boundary the next backup would treat it as unknown and skip its older logs. For the same reason
234+
* a host keeps its previous boundary while it still has logs: without it, the next backup would
235+
* include its logs that are already covered, which can bring back deleted data.
236+
* @param rolledHosts roll results of the hosts that took part in the log roll
237+
* @param previousBoundaries boundaries stored by the previous backup
238+
* @param hostsWithLogs hosts that still have logs, whether or not this backup includes them
239+
* @param coveredLogs logs whose edits are included in the backup
240+
* @param pendingLogs logs whose edits are not to be included in the backup
241+
* @return boundaries by host
242+
*/
243+
public static Map<String, Long> computeLogBoundaries(Map<String, Long> rolledHosts,
244+
Map<String, Long> previousBoundaries, Set<String> hostsWithLogs, Collection<String> coveredLogs,
245+
Collection<String> pendingLogs) throws IOException {
246+
Map<String, Long> otherHosts = new HashMap<>();
247+
for (String log : coveredLogs) {
248+
Path path = new Path(log);
249+
String host = parseHostNameFromLogFile(path);
250+
if (host != null && !rolledHosts.containsKey(host)) {
251+
otherHosts.merge(host, getCreationTime(path), Math::max);
252+
}
253+
}
254+
for (String log : pendingLogs) {
255+
Path path = new Path(log);
256+
String host = parseHostNameFromLogFile(path);
257+
if (host != null && !rolledHosts.containsKey(host)) {
258+
otherHosts.merge(host, getCreationTime(path) - 1, Math::min);
259+
}
260+
}
261+
Map<String, Long> boundaries = new HashMap<>(rolledHosts);
262+
boundaries.putAll(otherHosts);
263+
for (Entry<String, Long> previous : previousBoundaries.entrySet()) {
264+
if (hostsWithLogs.contains(previous.getKey())) {
265+
boundaries.putIfAbsent(previous.getKey(), previous.getValue());
266+
}
267+
}
268+
return boundaries;
269+
}
270+
189271
/**
190272
* Returns WAL file name
191273
* @param walFileName WAL file name

0 commit comments

Comments
 (0)