Skip to content

Commit 0dca45b

Browse files
author
Hernan Gelaf-Romer
committed
Log filtering in IncrementalBackupManager can lead to data loss
1 parent 7d0d73b commit 0dca45b

8 files changed

Lines changed: 1035 additions & 20 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: 76 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,64 @@ private void performSnapshots(Admin admin) throws IOException {
248263
}
249264
}
250265

266+
/**
267+
* Computes the per-host log boundaries stored by this full backup, using
268+
* {@link BackupUtils#computeLogBoundaries}. A host that took part in this backup's log roll gets
269+
* its roll result: WALs created after the roll are included in the next incremental backup, even
270+
* though some of their edits may already be in the snapshot, which is safe because deletes are
271+
* replayed along with the puts. For any other host, its archived WALs and the WALs of dead
272+
* servers are covered by the snapshot, so the next incremental backup does not replay them. The
273+
* WALs of a live server that did not take part in the roll (for example one that started during
274+
* it) are pending, because they can still receive edits that are not in the snapshot. The live
275+
* servers are read only after the WAL directories are listed: a region server creates its WAL
276+
* directory only after the master registers it, so every live server whose directory was listed
277+
* is found. It must run right after the log roll, before the snapshot is taken, so that a host
278+
* starting later gets no boundary and has all of its WALs included in the next incremental
279+
* backup.
280+
*/
281+
@RestrictedApi(
282+
explanation = "Package-private for test visibility only. Do not use outside tests.",
283+
link = "",
284+
allowedOnPath = "(.*/src/test/.*|.*/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java)")
285+
static Map<String, Long> computeLogBoundaries(FileSystem fs, Path walRootDir,
286+
Map<String, Long> rolledHosts, Admin admin) throws IOException {
287+
Path logDir = new Path(walRootDir, HConstants.HREGION_LOGDIR_NAME);
288+
Path oldLogDir = new Path(walRootDir, HConstants.HREGION_OLDLOGDIR_NAME);
289+
290+
Map<ServerName, List<String>> logsByServer = new HashMap<>();
291+
for (FileStatus serverLogDir : fs.listStatus(logDir)) {
292+
ServerName serverName =
293+
AbstractFSWALProvider.getServerNameFromWALDirectoryName(serverLogDir.getPath());
294+
if (serverName == null) {
295+
continue;
296+
}
297+
List<String> logs = logsByServer.computeIfAbsent(serverName, k -> new ArrayList<>());
298+
for (FileStatus log : fs.listStatus(serverLogDir.getPath())) {
299+
if (!AbstractFSWALProvider.isMetaFile(log.getPath())) {
300+
logs.add(log.getPath().toString());
301+
}
302+
}
303+
}
304+
305+
Set<ServerName> live = new HashSet<>(admin.getRegionServers());
306+
List<String> coveredLogs = new ArrayList<>();
307+
List<String> pendingLogs = new ArrayList<>();
308+
for (Map.Entry<ServerName, List<String>> entry : logsByServer.entrySet()) {
309+
if (live.contains(entry.getKey())) {
310+
pendingLogs.addAll(entry.getValue());
311+
} else {
312+
coveredLogs.addAll(entry.getValue());
313+
}
314+
}
315+
316+
if (fs.exists(oldLogDir)) {
317+
BackupUtils.getFiles(fs, oldLogDir, coveredLogs,
318+
path -> !AbstractFSWALProvider.isMetaFile(path));
319+
}
320+
321+
return BackupUtils.computeLogBoundaries(rolledHosts, coveredLogs, pendingLogs);
322+
}
323+
251324
private void updateBackupMetadata() throws IOException {
252325
// The table list in backupInfo is good for both full backup and incremental backup.
253326
// 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: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,13 @@ public IncrementalBackupManager(Connection conn, Configuration conf) throws IOEx
5454
/**
5555
* Obtain the list of logs that need to be copied out for this incremental backup. The list is set
5656
* in BackupInfo.
57-
* @return The new HashMap of RS log time stamps after the log roll for this incremental backup.
57+
* @return The new map of RS log time stamps for this incremental backup, as computed by
58+
* {@link BackupUtils#computeLogBoundaries}: the included logs are covered, and the logs
59+
* held back for a later backup (for example, logs still being split) are pending.
5860
* @throws IOException exception
5961
*/
6062
public Map<String, Long> getIncrBackupLogFileMap() throws IOException {
6163
List<String> logList;
62-
Map<String, Long> newTimestamps;
6364
Map<String, Long> previousTimestampMins =
6465
BackupUtils.getRSLogTimestampMins(readLogTimestampMap());
6566

@@ -69,18 +70,27 @@ public Map<String, Long> getIncrBackupLogFileMap() throws IOException {
6970
+ "In order to create an incremental backup, at least one full backup is needed.");
7071
}
7172

73+
Map<String, Long> previousLogRollByHost = readRegionServerLastLogRollResult();
7274
LOG.info("Execute roll log procedure for incremental backup ...");
7375
BackupUtils.logRoll(conn, backupInfo.getBackupRootDir(), conf);
7476

75-
newTimestamps = readRegionServerLastLogRollResult();
77+
Map<String, Long> rolledHosts =
78+
BackupUtils.getRolledHosts(previousLogRollByHost, readRegionServerLastLogRollResult());
7679

77-
logList = getLogFilesForNewBackup(previousTimestampMins, newTimestamps, conf);
78-
logList = excludeProcV2WALs(logList);
80+
LogFileSelection selection = getLogFilesForNewBackup(previousTimestampMins, rolledHosts, conf);
81+
logList = excludeProcV2WALs(selection.included());
7982
backupInfo.setIncrBackupFileList(logList);
8083

84+
Map<String, Long> newTimestamps =
85+
BackupUtils.computeLogBoundaries(rolledHosts, logList, selection.heldBack());
86+
LOG.debug("Log boundaries for incremental backup {}: {}", backupInfo.getBackupId(),
87+
newTimestamps);
8188
return newTimestamps;
8289
}
8390

91+
private record LogFileSelection(List<String> included, List<String> heldBack) {
92+
}
93+
8494
private List<String> excludeProcV2WALs(List<String> logList) {
8595
List<String> list = new ArrayList<>();
8696
for (int i = 0; i < logList.size(); i++) {
@@ -97,15 +107,16 @@ private List<String> excludeProcV2WALs(List<String> logList) {
97107
}
98108

99109
/**
100-
* For each region server: get all log files newer than the last timestamps but not newer than the
101-
* newest timestamps.
110+
* Gather all log files that either: 1) are newer than the older timestamps, but not newer than
111+
* the newest timestamps, or 2) are archived logs whose host name does not occur in the newest
112+
* timestamps.
102113
* @param olderTimestamps the timestamp for each region server of the last backup.
103114
* @param newestTimestamps the timestamp for each region server that the backup should lead to.
104115
* @param conf the Hadoop and Hbase configuration
105-
* @return a list of log files to be backed up
116+
* @return the log files to be backed up, and the log files held back for a later backup
106117
* @throws IOException exception
107118
*/
108-
private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
119+
private LogFileSelection getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
109120
Map<String, Long> newestTimestamps, Configuration conf) throws IOException {
110121
LOG.debug("In getLogFilesForNewBackup()\n" + "olderTimestamps: " + olderTimestamps
111122
+ "\n newestTimestamps: " + newestTimestamps);
@@ -217,18 +228,14 @@ private List<String> getLogFilesForNewBackup(Map<String, Long> olderTimestamps,
217228
resultLogFiles.add(currentLogFile);
218229
}
219230

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.
224231
Long newTimestamp = newestTimestamps.get(host);
225-
if (newTimestamp == null || currentLogTS > newTimestamp) {
232+
if (newTimestamp != null && currentLogTS > newTimestamp) {
226233
newestLogs.add(currentLogFile);
227234
}
228235
}
229236
// remove newest log per host because they are still in use
230237
resultLogFiles.removeAll(newestLogs);
231-
return resultLogFiles;
238+
return new LogFileSelection(resultLogFiles, newestLogs);
232239
}
233240

234241
static class NewestLogFilter implements PathFilter {

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.text.SimpleDateFormat;
3232
import java.time.ZoneOffset;
3333
import java.util.ArrayList;
34+
import java.util.Collection;
3435
import java.util.Collections;
3536
import java.util.Comparator;
3637
import java.util.Date;
@@ -206,6 +207,62 @@ public static String parseHostNameFromLogFile(Path p) {
206207
}
207208
}
208209

210+
/**
211+
* Returns the log roll result of every region server that took part in the log roll that happened
212+
* between reading {@code previousLogRolls} and {@code latestLogRolls}. A region server that did
213+
* not take part (for example because it is offline) keeps its old roll result, which is never
214+
* removed, so it is recognized by its roll result not changing.
215+
* @param previousLogRolls roll results by host, read before the log roll
216+
* @param latestLogRolls roll results by host, read after the log roll
217+
* @return roll results by host, for the hosts that took part in the log roll
218+
*/
219+
public static Map<String, Long> getRolledHosts(Map<String, Long> previousLogRolls,
220+
Map<String, Long> latestLogRolls) {
221+
return latestLogRolls.entrySet().stream()
222+
.filter(entry -> !entry.getValue().equals(previousLogRolls.get(entry.getKey())))
223+
.collect(Collectors.toMap(Entry::getKey, Entry::getValue));
224+
}
225+
226+
/**
227+
* Computes the per-host log boundaries stored by a backup. Everything up to a host's boundary is
228+
* covered by backups, so later incremental backups only include that host's logs that are newer.
229+
* <ul>
230+
* <li>A host that took part in the backup's log roll gets its roll result.</li>
231+
* <li>Any other host gets the creation time of its newest covered log, capped to just below its
232+
* oldest pending log, so that pending logs are included in a later backup. A host without covered
233+
* or pending logs gets no boundary.</li>
234+
* </ul>
235+
* Capping is needed because a host's logs can still be pending while some of its newer logs are
236+
* already covered, for example when a dead server's logs are split and archived out of order. A
237+
* host is also kept while any of its logs are pending, even if none are covered, because without
238+
* a boundary the next backup would treat it as unknown and skip its older logs.
239+
* @param rolledHosts roll results of the hosts that took part in the log roll
240+
* @param coveredLogs logs whose edits are covered by this backup
241+
* @param pendingLogs logs whose edits might not be covered by this backup
242+
* @return boundaries by host
243+
*/
244+
public static Map<String, Long> computeLogBoundaries(Map<String, Long> rolledHosts,
245+
Collection<String> coveredLogs, 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+
return boundaries;
264+
}
265+
209266
/**
210267
* Returns WAL file name
211268
* @param walFileName WAL file name

0 commit comments

Comments
 (0)