Skip to content

Commit 09d4006

Browse files
authored
HBASE-29776 Log filtering in IncrementalBackupManager can lead to data loss (#8694)
1 parent ab49d26 commit 09d4006

8 files changed

Lines changed: 1190 additions & 30 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
@@ -662,8 +662,9 @@ public List<BackupInfo> getBackupHistory(Order order, int n, BackupInfo.Filter..
662662

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

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

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,21 @@
3636
import static org.apache.hadoop.hbase.replication.ReplicationUtils.OFFSET_UPDATE_INTERVAL_MS_KEY;
3737
import static org.apache.hadoop.hbase.replication.ReplicationUtils.OFFSET_UPDATE_SIZE_THRESHOLD_KEY;
3838

39+
import com.google.errorprone.annotations.RestrictedApi;
3940
import java.io.IOException;
4041
import java.util.ArrayList;
4142
import java.util.HashMap;
43+
import java.util.HashSet;
4244
import java.util.List;
4345
import java.util.Map;
46+
import java.util.Set;
4447
import java.util.UUID;
4548
import java.util.stream.Collectors;
49+
import org.apache.hadoop.fs.FileStatus;
50+
import org.apache.hadoop.fs.FileSystem;
51+
import org.apache.hadoop.fs.Path;
52+
import org.apache.hadoop.hbase.HConstants;
53+
import org.apache.hadoop.hbase.ServerName;
4654
import org.apache.hadoop.hbase.TableName;
4755
import org.apache.hadoop.hbase.backup.BackupCopyJob;
4856
import org.apache.hadoop.hbase.backup.BackupInfo;
@@ -60,7 +68,9 @@
6068
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
6169
import org.apache.hadoop.hbase.replication.ReplicationException;
6270
import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
71+
import org.apache.hadoop.hbase.util.CommonFSUtils;
6372
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
73+
import org.apache.hadoop.hbase.wal.AbstractFSWALProvider;
6474
import org.apache.yetus.audience.InterfaceAudience;
6575
import org.slf4j.Logger;
6676
import org.slf4j.LoggerFactory;
@@ -208,7 +218,12 @@ private void handleContinuousBackup(Admin admin) throws IOException {
208218
}
209219

210220
private void handleNonContinuousBackup(Admin admin) throws IOException {
211-
performLogRoll();
221+
Map<String, Long> previousLogRollsByHost = backupManager.readRegionServerLastLogRollResult();
222+
Map<String, Long> latestLogRollsByHost = performLogRoll();
223+
Path walRootDir = CommonFSUtils.getWALRootDir(conf);
224+
newTimestamps = computeLogBoundaries(walRootDir.getFileSystem(conf), walRootDir,
225+
BackupUtils.getRolledHosts(previousLogRollsByHost, latestLogRollsByHost), admin);
226+
212227
performBackupSnapshots(admin);
213228
backupManager.addIncrementalBackupTableSet(backupInfo.getTables());
214229

@@ -219,15 +234,15 @@ private void handleNonContinuousBackup(Admin admin) throws IOException {
219234
updateBackupMetadata();
220235
}
221236

222-
private void performLogRoll() throws IOException {
237+
private Map<String, Long> performLogRoll() throws IOException {
223238
// We roll log here before we do the snapshot. It is possible there is duplicate data
224239
// in the log that is already in the snapshot. But if we do it after the snapshot, we
225240
// could have data loss.
226241
// A better approach is to do the roll log on each RS in the same global procedure as
227242
// the snapshot.
228243
LOG.info("Execute roll log procedure for full backup ...");
229244
BackupUtils.logRoll(conn, backupInfo.getBackupRootDir(), conf);
230-
newTimestamps = backupManager.readRegionServerLastLogRollResult();
245+
return backupManager.readRegionServerLastLogRollResult();
231246
}
232247

233248
private void performBackupSnapshots(Admin admin) throws IOException {
@@ -248,6 +263,82 @@ private void performSnapshots(Admin admin) throws IOException {
248263
}
249264
}
250265

266+
/**
267+
* Computes the per-host log boundaries stored by this full backup, so that the next incremental
268+
* backup includes exactly the WALs whose edits may be missing from the snapshot. It must run
269+
* right after the log roll, before the snapshot is taken, so that a host starting later gets no
270+
* boundary and has all of its WALs included in the next incremental backup.
271+
* <p>
272+
* Boundaries are computed by {@link BackupUtils#computeLogBoundaries}:
273+
* <ul>
274+
* <li>A host that took part in this backup's log roll gets its roll result. WALs created after
275+
* the roll will be included in the next incremental backup, even though some of their edits may
276+
* already be in the snapshot, which is safe because deletes are replayed along with the
277+
* puts.</li>
278+
* <li>For any other host, its archived WALs and the WALs of dead servers are covered by the
279+
* snapshot, so the next incremental backup does not replay them.</li>
280+
* <li>The WALs of a live server that did not take part in the roll (for example one that started
281+
* during it) are pending, because they can still receive edits that are not in the snapshot.</li>
282+
* </ul>
283+
* If the master's server list is incomplete (for example right after a master failover), the
284+
* backup fails instead of treating live servers as dead.
285+
*/
286+
@RestrictedApi(
287+
explanation = "Package-private for test visibility only. Do not use outside tests.",
288+
link = "",
289+
allowedOnPath = "(.*/src/test/.*|.*/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java)")
290+
static Map<String, Long> computeLogBoundaries(FileSystem fs, Path walRootDir,
291+
Map<String, Long> rolledHosts, Admin admin) throws IOException {
292+
Path logDir = new Path(walRootDir, HConstants.HREGION_LOGDIR_NAME);
293+
Path oldLogDir = new Path(walRootDir, HConstants.HREGION_OLDLOGDIR_NAME);
294+
295+
Map<ServerName, List<String>> logsByServer = new HashMap<>();
296+
for (FileStatus serverLogDir : fs.listStatus(logDir)) {
297+
ServerName serverName =
298+
AbstractFSWALProvider.getServerNameFromWALDirectoryName(serverLogDir.getPath());
299+
if (serverName == null) {
300+
continue;
301+
}
302+
List<String> logs = logsByServer.computeIfAbsent(serverName, k -> new ArrayList<>());
303+
for (FileStatus log : fs.listStatus(serverLogDir.getPath())) {
304+
if (!AbstractFSWALProvider.isMetaFile(log.getPath())) {
305+
logs.add(log.getPath().toString());
306+
}
307+
}
308+
}
309+
310+
// The live servers are read only after the WAL directories are listed: a region server creates
311+
// its WAL directory only after the master registers it, so every live server whose directory
312+
// was listed is found. If the live servers do not include every host that took part in the
313+
// roll, the master's server list is incomplete, or a host stopped after the roll.
314+
Set<ServerName> live = new HashSet<>(admin.getRegionServers());
315+
Set<String> liveAddresses =
316+
live.stream().map(sn -> sn.getAddress().toString()).collect(Collectors.toSet());
317+
if (!liveAddresses.containsAll(rolledHosts.keySet())) {
318+
throw new IOException("Live region servers " + live + " do not include every host that took"
319+
+ " part in this backup's log roll " + rolledHosts.keySet() + ". The master's server list"
320+
+ " may be incomplete, for example during a master failover, or a region server stopped"
321+
+ " after the log roll, so the backup is failed to be retried.");
322+
}
323+
324+
List<String> coveredLogs = new ArrayList<>();
325+
List<String> pendingLogs = new ArrayList<>();
326+
for (Map.Entry<ServerName, List<String>> entry : logsByServer.entrySet()) {
327+
if (live.contains(entry.getKey())) {
328+
pendingLogs.addAll(entry.getValue());
329+
} else {
330+
coveredLogs.addAll(entry.getValue());
331+
}
332+
}
333+
334+
if (fs.exists(oldLogDir)) {
335+
BackupUtils.getFiles(fs, oldLogDir, coveredLogs,
336+
path -> !AbstractFSWALProvider.isMetaFile(path));
337+
}
338+
339+
return BackupUtils.computeLogBoundaries(rolledHosts, coveredLogs, pendingLogs);
340+
}
341+
251342
private void updateBackupMetadata() throws IOException {
252343
// The table list in backupInfo is good for both full backup and incremental backup.
253344
// For incremental backup, it contains the incremental backup table set.

‎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 {

0 commit comments

Comments
 (0)