Skip to content

Commit

Permalink
Backports to get 3.x aligned with 3.11 (#1046)
Browse files Browse the repository at this point in the history 
* Fix backup verification race condition causing missing notifications (#1034)

* Remove metaproxy validation it is never null in practice.

* Remove DateRange validation. It is never null in practice.

* Remove debug logging.

* Remove latest backup metadata validation. It is never null in practice.

* Consolidate repeated code into private verifyBackup.

* Change method names to better reflect what they do.

* Update latestResult wherever possible.

* Rewrite logic in findLatestVerfiedBackup to make it look more like verifyBackupsInRange.

* Change signature of BackupNotificationMgr.notify to not depend on BackupVerificationResult.

* Return all verified BackupMetadata instead of BackupVerificationResult when verifying en masse. It has enough information to skip the call to find the most recently verified backup.

Also, fix some tests that broke in this process: remove the check for the snapshot time in TestBackupVerification that only makes sense when the Path is for a file that does not exist. Also, mock the appropriate functions in MockBackupVerification in TestBackupVerificationTask.

* Rename findLatestVerifiedBackup responding to review comments.

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

* Remove unused code.

* Remove redundant comments and vertical whitespace.

* Remove debug comments and now-redundant logger, simplify if-else and tighten error message for code style.

* Use final where applicable and remove it where redundant.

* Remove redundant BackupRestoreException from getIncrementals method signature.

* Split getting incremental files and snapshot files into separate methods.

* Reveal hook to allow operators to restore to the last valid snapshot.

* Remove added non-shaded Guava dependency pursuant to review comments.

* minor code modifications to simplify the nfpriam spring boot migration

* make the constructor public

* remove the instance info from the DI (#1042)

* Always TTL backups. (#1038)

* Fix Github CI by explicitly creating necessary directories. (#1045)

---------

Co-authored-by: Cheng Wang <chengw@netflix.com>
Co-authored-by: Cheng Wang <107727158+chengw-netflix@users.noreply.github.com>
  • Loading branch information
@mattl-netflix @chengw-netflix
3 people committed Apr 18, 2023
1 parent b3d4dd9 commit 257b13c
Show file tree
Hide file tree
Showing 25 changed files with 208 additions and 288 deletions.
6 changes: 3 additions & 3 deletions priam/src/main/java/com/netflix/priam/aws/auth/EC2RoleAssumptionCredential.java
View file
Expand Up @@ -17,6 +17,7 @@
import com.amazonaws.auth.STSAssumeRoleSessionCredentialsProvider;
import com.netflix.priam.config.IConfiguration;
import com.netflix.priam.cred.ICredential;
import com.netflix.priam.identity.config.AWSInstanceInfo;
import com.netflix.priam.identity.config.InstanceInfo;
import javax.inject.Inject;
import org.apache.commons.lang3.StringUtils;
Expand All @@ -29,11 +30,10 @@ public class EC2RoleAssumptionCredential implements ICredential {
private AWSCredentialsProvider stsSessionCredentialsProvider;

@Inject
public EC2RoleAssumptionCredential(
ICredential cred, IConfiguration config, InstanceInfo instanceInfo) {
public EC2RoleAssumptionCredential(ICredential cred, IConfiguration config) {
this.cred = cred;
this.config = config;
this.instanceInfo = instanceInfo;
this.instanceInfo = new AWSInstanceInfo(cred);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion priam/src/main/java/com/netflix/priam/backup/BackupDynamicRateLimiter.java
View file
Expand Up @@ -16,7 +16,7 @@ public class BackupDynamicRateLimiter implements DynamicRateLimiter {
private final RateLimiter rateLimiter;

@Inject
BackupDynamicRateLimiter(IConfiguration config, Clock clock, DirectorySize dirSize) {
public BackupDynamicRateLimiter(IConfiguration config, Clock clock, DirectorySize dirSize) {
this.clock = clock;
this.config = config;
this.dirSize = dirSize;
Expand Down
4 changes: 2 additions & 2 deletions priam/src/main/java/com/netflix/priam/backup/BackupMetadata.java
View file
Expand Up @@ -34,9 +34,9 @@ public final class BackupMetadata implements Serializable {
private BackupVersion backupVersion;
private String snapshotLocation;

public BackupMetadata(BackupVersion backupVersion, String token, Date start) throws Exception {
public BackupMetadata(BackupVersion backupVersion, String token, Date start) {
if (start == null || token == null || StringUtils.isEmpty(token))
throw new Exception(
throw new IllegalArgumentException(
String.format(
"Invalid Input: Token: %s or start date: %s is null or empty.",
token, start));
Expand Down
93 changes: 25 additions & 68 deletions priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java
View file
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();
}

public static List<AbstractBackupPath> getAllFiles(
public static List<AbstractBackupPath> getMostRecentSnapshotPaths(
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(
"Skipping: keyspace: {}, CF: {} is part of exclude list.",
keyspace,
columnFamilyName);
return true;
}

if (includeFilter != null)
if (!(includeFilter.containsKey(keyspace)
return !(includeFilter.containsKey(keyspace)
&& (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;
}
}
122 changes: 48 additions & 74 deletions priam/src/main/java/com/netflix/priam/backup/BackupVerification.java
View file
Expand Up @@ -14,13 +14,15 @@
package com.netflix.priam.backup;

import com.netflix.priam.backupv2.IMetaProxy;
import com.netflix.priam.scheduler.UnsupportedTypeException;
import com.netflix.priam.utils.DateUtil;
import com.netflix.priam.utils.DateUtil.DateRange;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Instant;
import java.util.*;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.Optional;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
Expand All @@ -44,7 +46,7 @@ public class BackupVerification {
private BackupVerificationResult latestResult;

@Inject
BackupVerification(
public BackupVerification(
@Named("v1") IMetaProxy metaV1Proxy,
@Named("v2") IMetaProxy metaV2Proxy,
IBackupStatusMgr backupStatusMgr,
Expand All @@ -66,100 +68,72 @@ public IMetaProxy getMetaProxy(BackupVersion backupVersion) {
return null;
}

public Optional<BackupVerificationResult> verifyBackup(
public Optional<BackupVerificationResult> verifyLatestBackup(
BackupVersion backupVersion, boolean force, DateRange dateRange)
throws UnsupportedTypeException, IllegalArgumentException {
throws IllegalArgumentException {
IMetaProxy metaProxy = getMetaProxy(backupVersion);
if (metaProxy == null) {
throw new UnsupportedTypeException(
"BackupVersion type: " + backupVersion + " is not supported");
}

if (dateRange == null) {
throw new IllegalArgumentException("dateRange provided is null");
}

List<BackupMetadata> metadata =
backupStatusMgr.getLatestBackupMetadata(backupVersion, dateRange);
if (metadata == null || metadata.isEmpty()) return Optional.empty();
for (BackupMetadata backupMetadata : metadata) {
if (backupMetadata.getLastValidated() != null && !force) {
// Backup is already validated. Nothing to do.
latestResult = new BackupVerificationResult();
latestResult.valid = true;
latestResult.manifestAvailable = true;
latestResult.snapshotInstant = backupMetadata.getStart().toInstant();
Path snapshotLocation = Paths.get(backupMetadata.getSnapshotLocation());
latestResult.remotePath =
snapshotLocation.subpath(1, snapshotLocation.getNameCount()).toString();
for (BackupMetadata backupMetadata :
backupStatusMgr.getLatestBackupMetadata(backupVersion, dateRange)) {
if (backupMetadata.getLastValidated() == null || force) {
Optional<BackupVerificationResult> result = verifyBackup(metaProxy, backupMetadata);
if (result.isPresent()) {
return result;
}
} else {
updateLatestResult(backupMetadata);
return Optional.of(latestResult);
}
BackupVerificationResult backupVerificationResult =
verifyBackup(metaProxy, backupMetadata);
if (logger.isDebugEnabled())
logger.debug(
"BackupVerification: metadata: {}, result: {}",
backupMetadata,
backupVerificationResult);
if (backupVerificationResult.valid) {
backupMetadata.setLastValidated(new Date(DateUtil.getInstant().toEpochMilli()));
backupStatusMgr.update(backupMetadata);
latestResult = backupVerificationResult;
return Optional.of(backupVerificationResult);
}
}
latestResult = null;
return Optional.empty();
}

public List<BackupVerificationResult> verifyAllBackups(
BackupVersion backupVersion, DateRange dateRange)
throws UnsupportedTypeException, IllegalArgumentException {
public List<BackupMetadata> verifyBackupsInRange(
BackupVersion backupVersion, DateRange dateRange) throws IllegalArgumentException {
IMetaProxy metaProxy = getMetaProxy(backupVersion);
if (metaProxy == null) {
throw new UnsupportedTypeException(
"BackupVersion type: " + backupVersion + " is not supported");
}

if (dateRange == null) {
throw new IllegalArgumentException("dateRange provided is null");
}

List<BackupVerificationResult> result = new ArrayList<>();

List<BackupMetadata> metadata =
backupStatusMgr.getLatestBackupMetadata(backupVersion, dateRange);
if (metadata == null || metadata.isEmpty()) return result;
for (BackupMetadata backupMetadata : metadata) {
if (backupMetadata.getLastValidated() == null) {
BackupVerificationResult backupVerificationResult =
verifyBackup(metaProxy, backupMetadata);
if (logger.isDebugEnabled())
logger.debug(
"BackupVerification: metadata: {}, result: {}",
backupMetadata,
backupVerificationResult);
if (backupVerificationResult.valid) {
backupMetadata.setLastValidated(new Date(DateUtil.getInstant().toEpochMilli()));
backupStatusMgr.update(backupMetadata);
result.add(backupVerificationResult);
}
List<BackupMetadata> results = new ArrayList<>();
for (BackupMetadata backupMetadata :
backupStatusMgr.getLatestBackupMetadata(backupVersion, dateRange)) {
if (backupMetadata.getLastValidated() != null
|| verifyBackup(metaProxy, backupMetadata).isPresent()) {
results.add(backupMetadata);
}
}
return result;
return results;
}

/** returns the latest valid backup verification result if we have found one within the SLO * */
public Optional<Instant> getLatestVerfifiedBackupTime() {
return latestResult == null ? Optional.empty() : Optional.of(latestResult.snapshotInstant);
}

private BackupVerificationResult verifyBackup(
private Optional<BackupVerificationResult> verifyBackup(
IMetaProxy metaProxy, BackupMetadata latestBackupMetaData) {
Path metadataLocation = Paths.get(latestBackupMetaData.getSnapshotLocation());
metadataLocation = metadataLocation.subpath(1, metadataLocation.getNameCount());
AbstractBackupPath abstractBackupPath = abstractBackupPathProvider.get();
abstractBackupPath.parseRemote(metadataLocation.toString());
return metaProxy.isMetaFileValid(abstractBackupPath);
BackupVerificationResult result = metaProxy.isMetaFileValid(abstractBackupPath);
if (result.valid) {
updateLatestResult(latestBackupMetaData);
Date now = new Date(DateUtil.getInstant().toEpochMilli());
latestBackupMetaData.setLastValidated(now);
backupStatusMgr.update(latestBackupMetaData);
return Optional.of(result);
}
return Optional.empty();
}

private void updateLatestResult(BackupMetadata backupMetadata) {
Instant snapshotInstant = backupMetadata.getStart().toInstant();
if (latestResult == null || latestResult.snapshotInstant.isBefore(snapshotInstant)) {
latestResult = new BackupVerificationResult();
latestResult.valid = true;
latestResult.manifestAvailable = true;
latestResult.snapshotInstant = backupMetadata.getStart().toInstant();
Path snapshotLocation = Paths.get(backupMetadata.getSnapshotLocation());
latestResult.remotePath =
snapshotLocation.subpath(1, snapshotLocation.getNameCount()).toString();
}
}
}

0 comments on commit 257b13c

Please sign in to comment.