Skip to content
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.

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

Reveal hook to allow operators to restore just to the most recent snapshot #1035

Merged
merged 8 commits into from
Mar 29, 2023
93 changes: 25 additions & 68 deletions priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,13 @@
import javax.inject.Provider;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** Created by aagrawal on 8/14/17. */
/** Helper methods applicable to both backup and restore */
public class BackupRestoreUtil {
private static final Logger logger = LoggerFactory.getLogger(BackupRestoreUtil.class);
private static final Pattern columnFamilyFilterPattern = Pattern.compile(".\\..");
private Map<String, List<String>> includeFilter;
private Map<String, List<String>> excludeFilter;
private final Map<String, List<String>> includeFilter;
private final Map<String, List<String>> excludeFilter;

public static final List<String> FILTER_KEYSPACE = Collections.singletonList("OpsCenter");
private static final Map<String, List<String>> FILTER_COLUMN_FAMILY =
ImmutableMap.of(
"system",
Expand All @@ -49,43 +45,26 @@ public class BackupRestoreUtil {

@Inject
public BackupRestoreUtil(String configIncludeFilter, String configExcludeFilter) {
setFilters(configIncludeFilter, configExcludeFilter);
}

public BackupRestoreUtil setFilters(String configIncludeFilter, String configExcludeFilter) {
includeFilter = getFilter(configIncludeFilter);
excludeFilter = getFilter(configExcludeFilter);
logger.info("Exclude filter set: {}", configExcludeFilter);
logger.info("Include filter set: {}", configIncludeFilter);
return this;
}

public static Optional<AbstractBackupPath> getLatestValidMetaPath(
IMetaProxy metaProxy, DateUtil.DateRange dateRange) {
// Get a list of manifest files.
List<AbstractBackupPath> metas = metaProxy.findMetaFiles(dateRange);

// Find a valid manifest file.
for (AbstractBackupPath meta : metas) {
BackupVerificationResult result = metaProxy.isMetaFileValid(meta);
if (result.valid) {
return Optional.of(meta);
}
}

return Optional.empty();
return metaProxy
.findMetaFiles(dateRange)
.stream()
.filter(meta -> metaProxy.isMetaFileValid(meta).valid)
.findFirst();
Copy link
Contributor

Choose a reason for hiding this comment

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

beautiful

}

public static List<AbstractBackupPath> getAllFiles(
public static List<AbstractBackupPath> getMostRecentSnapshotPaths(
Copy link
Contributor

Choose a reason for hiding this comment

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

renaming the method was a great idea, makes it more precise (now that it's different) and also makes it impossible to forget to update a callsite

AbstractBackupPath latestValidMetaFile,
DateUtil.DateRange dateRange,
IMetaProxy metaProxy,
Provider<AbstractBackupPath> pathProvider)
throws Exception {
// Download the meta.json file.
Path metaFile = metaProxy.downloadMetaFile(latestValidMetaFile);
// Parse meta.json file to find the files required to download from this snapshot.
List<AbstractBackupPath> allFiles =
List<AbstractBackupPath> snapshotPaths =
metaProxy
.getSSTFilesFromMeta(metaFile)
.stream()
Expand All @@ -96,50 +75,43 @@ public static List<AbstractBackupPath> getAllFiles(
return path;
})
.collect(Collectors.toList());

FileUtils.deleteQuietly(metaFile.toFile());
return snapshotPaths;
}

// Download incremental SSTables after the snapshot meta file.
public static List<AbstractBackupPath> getIncrementalPaths(
AbstractBackupPath latestValidMetaFile,
DateUtil.DateRange dateRange,
IMetaProxy metaProxy) {
Instant snapshotTime;
if (metaProxy instanceof MetaV2Proxy) snapshotTime = latestValidMetaFile.getLastModified();
else snapshotTime = latestValidMetaFile.getTime().toInstant();

DateUtil.DateRange incrementalDateRange =
new DateUtil.DateRange(snapshotTime, dateRange.getEndTime());
Iterator<AbstractBackupPath> incremental = metaProxy.getIncrementals(incrementalDateRange);
while (incremental.hasNext()) allFiles.add(incremental.next());

return allFiles;
List<AbstractBackupPath> incrementalPaths = new ArrayList<>();
metaProxy.getIncrementals(incrementalDateRange).forEachRemaining(incrementalPaths::add);
return incrementalPaths;
}

public static final Map<String, List<String>> getFilter(String inputFilter)
public static Map<String, List<String>> getFilter(String inputFilter)
throws IllegalArgumentException {
if (StringUtils.isEmpty(inputFilter)) return null;

final Map<String, List<String>> columnFamilyFilter =
new HashMap<>(); // key: keyspace, value: a list of CFs within the keyspace

final Map<String, List<String>> columnFamilyFilter = new HashMap<>();
String[] filters = inputFilter.split(",");
for (String cfFilter :
filters) { // process filter of form keyspace.* or keyspace.columnfamily
for (String cfFilter : filters) {
if (columnFamilyFilterPattern.matcher(cfFilter).find()) {

String[] filter = cfFilter.split("\\.");
String keyspaceName = filter[0];
String columnFamilyName = filter[1];

if (columnFamilyName.contains("-"))
columnFamilyName = columnFamilyName.substring(0, columnFamilyName.indexOf("-"));

List<String> existingCfs =
columnFamilyFilter.getOrDefault(keyspaceName, new ArrayList<>());
if (!columnFamilyName.equalsIgnoreCase("*")) existingCfs.add(columnFamilyName);
columnFamilyFilter.put(keyspaceName, existingCfs);

} else {
throw new IllegalArgumentException(
"Column family filter format is not valid. Format needs to be \"keyspace.columnfamily\". Invalid input: "
+ cfFilter);
"Invalid format: " + cfFilter + ". \"keyspace.columnfamily\" is required.");
}
}
return columnFamilyFilter;
Expand All @@ -154,34 +126,19 @@ public static final Map<String, List<String>> getFilter(String inputFilter)
*/
public final boolean isFiltered(String keyspace, String columnFamilyDir) {
if (StringUtils.isEmpty(keyspace) || StringUtils.isEmpty(columnFamilyDir)) return false;

String columnFamilyName = columnFamilyDir.split("-")[0];
// column family is in list of global CF filter
if (FILTER_COLUMN_FAMILY.containsKey(keyspace)
&& FILTER_COLUMN_FAMILY.get(keyspace).contains(columnFamilyName)) return true;

if (excludeFilter != null)
if (excludeFilter.containsKey(keyspace)
&& (excludeFilter.get(keyspace).isEmpty()
|| excludeFilter.get(keyspace).contains(columnFamilyName))) {
logger.debug(
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose it's fine and you have a better understanding around whether or not we ever need those log messages but in general I'm not opposed to debug logs that I can enable to help debug an issue

"Skipping: keyspace: {}, CF: {} is part of exclude list.",
keyspace,
columnFamilyName);
return true;
}

if (includeFilter != null)
if (!(includeFilter.containsKey(keyspace)
return !(includeFilter.containsKey(keyspace)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: could collapse this further, return includeFilter != null && !includeFilter.containsKey(keyspace) && foo || bar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good point. However, I wanted to leave code in methods unrelated to the goal of the PR unchanged. I accepted this because it was done automatically by Intellij. If I change this I'd need to add the appropriate tests and I think that is beyond scope and best left to a follow-up.

&& (includeFilter.get(keyspace).isEmpty()
|| includeFilter.get(keyspace).contains(columnFamilyName)))) {
logger.debug(
"Skipping: keyspace: {}, CF: {} is not part of include list.",
keyspace,
columnFamilyName);
return true;
}

|| includeFilter.get(keyspace).contains(columnFamilyName)));
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,8 @@ public interface IMetaProxy {
*
* @param dateRange the time period to scan in the remote file system for incremental files.
* @return iterator containing the list of path on the remote file system satisfying criteria.
* @throws BackupRestoreException if there is an issue contacting remote file system.
*/
Iterator<AbstractBackupPath> getIncrementals(DateUtil.DateRange dateRange)
throws BackupRestoreException;
Iterator<AbstractBackupPath> getIncrementals(DateUtil.DateRange dateRange);

/**
* Validate that all the files mentioned in the meta file actually exists on remote file system.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ public List<String> getSSTFilesFromMeta(Path localMetaPath) throws Exception {
}

@Override
public Iterator<AbstractBackupPath> getIncrementals(DateUtil.DateRange dateRange)
throws BackupRestoreException {
public Iterator<AbstractBackupPath> getIncrementals(DateUtil.DateRange dateRange) {
String prefix = fs.getPrefix().toString();
Iterator<AbstractBackupPath> iterator =
fs.list(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ private String getMatch(
}

@Override
public Iterator<AbstractBackupPath> getIncrementals(DateUtil.DateRange dateRange)
throws BackupRestoreException {
public Iterator<AbstractBackupPath> getIncrementals(DateUtil.DateRange dateRange) {
String incrementalPrefix = getMatch(dateRange, AbstractBackupPath.BackupFileType.SST_V2);
String marker =
getMatch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1178,6 +1178,11 @@ default long getCompressionTransitionEpochMillis() {
/** @return whether to enable auto_snapshot */
boolean getAutoSnapshot();

/** @return whether incremental backups should be skipped in a restore */
default boolean skipIncrementalRestore() {
return false;
}

/**
* Escape hatch for getting any arbitrary property by key This is useful so we don't have to
* keep adding methods to this interface for every single configuration option ever. Also
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,11 @@ public Response list(@PathParam("daterange") String daterange) throws Exception
return Response.ok("No valid meta found!").build();
}
List<AbstractBackupPath> allFiles =
BackupRestoreUtil.getAllFiles(
latestValidMetaFile.get(), dateRange, metaProxy, pathProvider);
BackupRestoreUtil.getMostRecentSnapshotPaths(
latestValidMetaFile.get(), metaProxy, pathProvider);
allFiles.addAll(
BackupRestoreUtil.getIncrementalPaths(
latestValidMetaFile.get(), dateRange, metaProxy));

return Response.ok(
GsonJsonSerializer.getGson()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,6 @@ public static final boolean isRestoreEnabled(IConfiguration conf, InstanceInfo i
return (isRestoreMode && isBackedupRac);
}

public void setRestoreConfiguration(String restoreIncludeCFList, String restoreExcludeCFList) {
backupRestoreUtil.setFilters(restoreIncludeCFList, restoreExcludeCFList);
}

private List<Future<Path>> download(
Iterator<AbstractBackupPath> fsIterator, boolean waitForCompletion) throws Exception {
List<Future<Path>> futureList = new ArrayList<>();
Expand Down Expand Up @@ -247,8 +243,13 @@ public void restore(DateUtil.DateRange dateRange) throws Exception {
.setSnapshotMetaFile(latestValidMetaFile.get().getRemotePath());

List<AbstractBackupPath> allFiles =
BackupRestoreUtil.getAllFiles(
latestValidMetaFile.get(), dateRange, metaProxy, pathProvider);
BackupRestoreUtil.getMostRecentSnapshotPaths(
latestValidMetaFile.get(), metaProxy, pathProvider);
if (!config.skipIncrementalRestore()) {
allFiles.addAll(
BackupRestoreUtil.getIncrementalPaths(
latestValidMetaFile.get(), dateRange, metaProxy));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I take it we don't have to update the callsite in BackupServletV2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it there as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused - do we need to use config.skipIncrementalRestore there too? I didn't see that there in this PR, perhaps it isn't required?


// Download snapshot which is listed in the meta file.
List<Future<Path>> futureList = new ArrayList<>();
Expand Down