From 3a98c854f3c16fce77a211da993d8d5a1cc16ae0 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 20:33:55 -0700 Subject: [PATCH 1/8] Remove unused code. --- .../java/com/netflix/priam/backup/BackupRestoreUtil.java | 8 -------- .../java/com/netflix/priam/restore/AbstractRestore.java | 4 ---- 2 files changed, 12 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java index 776a3d9f3..dc2c06bbe 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java @@ -40,7 +40,6 @@ public class BackupRestoreUtil { private Map> includeFilter; private Map> excludeFilter; - public static final List FILTER_KEYSPACE = Collections.singletonList("OpsCenter"); private static final Map> FILTER_COLUMN_FAMILY = ImmutableMap.of( "system", @@ -49,15 +48,8 @@ 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 getLatestValidMetaPath( diff --git a/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java b/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java index 60f69d14b..89b263ec8 100644 --- a/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java +++ b/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java @@ -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> download( Iterator fsIterator, boolean waitForCompletion) throws Exception { List> futureList = new ArrayList<>(); From 352359b66b299c9d00a1439897a9dcff01072cd5 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 20:41:32 -0700 Subject: [PATCH 2/8] Remove redundant comments and vertical whitespace. --- .../priam/backup/BackupRestoreUtil.java | 41 ++++--------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java index dc2c06bbe..7a83fa306 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java @@ -33,7 +33,7 @@ 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(".\\.."); @@ -54,18 +54,11 @@ public BackupRestoreUtil(String configIncludeFilter, String configExcludeFilter) public static Optional getLatestValidMetaPath( IMetaProxy metaProxy, DateUtil.DateRange dateRange) { - // Get a list of manifest files. - List 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 getAllFiles( @@ -74,9 +67,7 @@ public static List getAllFiles( IMetaProxy metaProxy, Provider 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 allFiles = metaProxy .getSSTFilesFromMeta(metaFile) @@ -88,14 +79,11 @@ public static List getAllFiles( return path; }) .collect(Collectors.toList()); - FileUtils.deleteQuietly(metaFile.toFile()); - // Download incremental SSTables after the snapshot meta file. Instant snapshotTime; if (metaProxy instanceof MetaV2Proxy) snapshotTime = latestValidMetaFile.getLastModified(); else snapshotTime = latestValidMetaFile.getTime().toInstant(); - DateUtil.DateRange incrementalDateRange = new DateUtil.DateRange(snapshotTime, dateRange.getEndTime()); Iterator incremental = metaProxy.getIncrementals(incrementalDateRange); @@ -107,27 +95,19 @@ public static List getAllFiles( public static final Map> getFilter(String inputFilter) throws IllegalArgumentException { if (StringUtils.isEmpty(inputFilter)) return null; - - final Map> columnFamilyFilter = - new HashMap<>(); // key: keyspace, value: a list of CFs within the keyspace - + final Map> 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 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: " @@ -146,12 +126,9 @@ public static final Map> 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() @@ -162,7 +139,6 @@ public final boolean isFiltered(String keyspace, String columnFamilyDir) { columnFamilyName); return true; } - if (includeFilter != null) if (!(includeFilter.containsKey(keyspace) && (includeFilter.get(keyspace).isEmpty() @@ -173,7 +149,6 @@ public final boolean isFiltered(String keyspace, String columnFamilyDir) { columnFamilyName); return true; } - return false; } } From a8e54b3a01e82719312e66b9d1824c1a47ff9230 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 20:45:34 -0700 Subject: [PATCH 3/8] Remove debug comments and now-redundant logger, simplify if-else and tighten error message for code style. --- .../priam/backup/BackupRestoreUtil.java | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java index 7a83fa306..e1aaf3d04 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java @@ -30,12 +30,9 @@ import javax.inject.Provider; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** 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> includeFilter; private Map> excludeFilter; @@ -110,8 +107,7 @@ public static final Map> getFilter(String inputFilter) 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; @@ -133,22 +129,12 @@ public final boolean isFiltered(String keyspace, String columnFamilyDir) { 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; } } From 1000b869e05e3843d96366e9f95f69173d14e179 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 20:47:28 -0700 Subject: [PATCH 4/8] Use final where applicable and remove it where redundant. --- .../java/com/netflix/priam/backup/BackupRestoreUtil.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java index e1aaf3d04..5780234c6 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java @@ -34,8 +34,8 @@ /** Helper methods applicable to both backup and restore */ public class BackupRestoreUtil { private static final Pattern columnFamilyFilterPattern = Pattern.compile(".\\.."); - private Map> includeFilter; - private Map> excludeFilter; + private final Map> includeFilter; + private final Map> excludeFilter; private static final Map> FILTER_COLUMN_FAMILY = ImmutableMap.of( @@ -89,7 +89,7 @@ public static List getAllFiles( return allFiles; } - public static final Map> getFilter(String inputFilter) + public static Map> getFilter(String inputFilter) throws IllegalArgumentException { if (StringUtils.isEmpty(inputFilter)) return null; final Map> columnFamilyFilter = new HashMap<>(); From fad61c85aa18e389dee61267b836d00ba4cc4d01 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 21:00:19 -0700 Subject: [PATCH 5/8] Remove redundant BackupRestoreException from getIncrementals method signature. --- .../src/main/java/com/netflix/priam/backupv2/IMetaProxy.java | 4 +--- .../src/main/java/com/netflix/priam/backupv2/MetaV1Proxy.java | 3 +-- .../src/main/java/com/netflix/priam/backupv2/MetaV2Proxy.java | 3 +-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backupv2/IMetaProxy.java b/priam/src/main/java/com/netflix/priam/backupv2/IMetaProxy.java index 99c52abb2..2f79852b2 100644 --- a/priam/src/main/java/com/netflix/priam/backupv2/IMetaProxy.java +++ b/priam/src/main/java/com/netflix/priam/backupv2/IMetaProxy.java @@ -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 getIncrementals(DateUtil.DateRange dateRange) - throws BackupRestoreException; + Iterator getIncrementals(DateUtil.DateRange dateRange); /** * Validate that all the files mentioned in the meta file actually exists on remote file system. diff --git a/priam/src/main/java/com/netflix/priam/backupv2/MetaV1Proxy.java b/priam/src/main/java/com/netflix/priam/backupv2/MetaV1Proxy.java index 348311918..3ed499336 100644 --- a/priam/src/main/java/com/netflix/priam/backupv2/MetaV1Proxy.java +++ b/priam/src/main/java/com/netflix/priam/backupv2/MetaV1Proxy.java @@ -169,8 +169,7 @@ public List getSSTFilesFromMeta(Path localMetaPath) throws Exception { } @Override - public Iterator getIncrementals(DateUtil.DateRange dateRange) - throws BackupRestoreException { + public Iterator getIncrementals(DateUtil.DateRange dateRange) { String prefix = fs.getPrefix().toString(); Iterator iterator = fs.list( diff --git a/priam/src/main/java/com/netflix/priam/backupv2/MetaV2Proxy.java b/priam/src/main/java/com/netflix/priam/backupv2/MetaV2Proxy.java index 1a810f476..288e644b1 100644 --- a/priam/src/main/java/com/netflix/priam/backupv2/MetaV2Proxy.java +++ b/priam/src/main/java/com/netflix/priam/backupv2/MetaV2Proxy.java @@ -79,8 +79,7 @@ private String getMatch( } @Override - public Iterator getIncrementals(DateUtil.DateRange dateRange) - throws BackupRestoreException { + public Iterator getIncrementals(DateUtil.DateRange dateRange) { String incrementalPrefix = getMatch(dateRange, AbstractBackupPath.BackupFileType.SST_V2); String marker = getMatch( From 15535f28a9f05affe3b4b837faec0354a94c43f3 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 21:00:56 -0700 Subject: [PATCH 6/8] Split getting incremental files and snapshot files into separate methods. --- .../netflix/priam/backup/BackupRestoreUtil.java | 17 ++++++++++------- .../priam/resources/BackupServletV2.java | 7 +++++-- .../netflix/priam/restore/AbstractRestore.java | 7 +++++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java index 5780234c6..687855d9e 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java @@ -17,6 +17,7 @@ package com.netflix.priam.backup; +import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableMap; import com.netflix.priam.backupv2.IMetaProxy; import com.netflix.priam.backupv2.MetaV2Proxy; @@ -58,14 +59,13 @@ public static Optional getLatestValidMetaPath( .findFirst(); } - public static List getAllFiles( + public static List getMostRecentSnapshotPaths( AbstractBackupPath latestValidMetaFile, - DateUtil.DateRange dateRange, IMetaProxy metaProxy, Provider pathProvider) throws Exception { Path metaFile = metaProxy.downloadMetaFile(latestValidMetaFile); - List allFiles = + List snapshotPaths = metaProxy .getSSTFilesFromMeta(metaFile) .stream() @@ -77,16 +77,19 @@ public static List getAllFiles( }) .collect(Collectors.toList()); FileUtils.deleteQuietly(metaFile.toFile()); + return snapshotPaths; + } + public static List 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 incremental = metaProxy.getIncrementals(incrementalDateRange); - while (incremental.hasNext()) allFiles.add(incremental.next()); - - return allFiles; + return Lists.newArrayList(metaProxy.getIncrementals(incrementalDateRange)); } public static Map> getFilter(String inputFilter) diff --git a/priam/src/main/java/com/netflix/priam/resources/BackupServletV2.java b/priam/src/main/java/com/netflix/priam/resources/BackupServletV2.java index 7b11807d5..b55a1edef 100644 --- a/priam/src/main/java/com/netflix/priam/resources/BackupServletV2.java +++ b/priam/src/main/java/com/netflix/priam/resources/BackupServletV2.java @@ -147,8 +147,11 @@ public Response list(@PathParam("daterange") String daterange) throws Exception return Response.ok("No valid meta found!").build(); } List 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() diff --git a/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java b/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java index 89b263ec8..4dc671833 100644 --- a/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java +++ b/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java @@ -243,8 +243,11 @@ public void restore(DateUtil.DateRange dateRange) throws Exception { .setSnapshotMetaFile(latestValidMetaFile.get().getRemotePath()); List allFiles = - BackupRestoreUtil.getAllFiles( - latestValidMetaFile.get(), dateRange, metaProxy, pathProvider); + BackupRestoreUtil.getMostRecentSnapshotPaths( + latestValidMetaFile.get(), metaProxy, pathProvider); + allFiles.addAll( + BackupRestoreUtil.getIncrementalPaths( + latestValidMetaFile.get(), dateRange, metaProxy)); // Download snapshot which is listed in the meta file. List> futureList = new ArrayList<>(); From 9c3baf0785d2ca570de010f3f23ef36dfb3f2d55 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Mon, 27 Mar 2023 21:03:38 -0700 Subject: [PATCH 7/8] Reveal hook to allow operators to restore to the last valid snapshot. --- .../java/com/netflix/priam/config/IConfiguration.java | 5 +++++ .../java/com/netflix/priam/restore/AbstractRestore.java | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/config/IConfiguration.java b/priam/src/main/java/com/netflix/priam/config/IConfiguration.java index 5b2576973..fcb03d29d 100644 --- a/priam/src/main/java/com/netflix/priam/config/IConfiguration.java +++ b/priam/src/main/java/com/netflix/priam/config/IConfiguration.java @@ -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 diff --git a/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java b/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java index 4dc671833..1074206e5 100644 --- a/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java +++ b/priam/src/main/java/com/netflix/priam/restore/AbstractRestore.java @@ -245,9 +245,11 @@ public void restore(DateUtil.DateRange dateRange) throws Exception { List allFiles = BackupRestoreUtil.getMostRecentSnapshotPaths( latestValidMetaFile.get(), metaProxy, pathProvider); - allFiles.addAll( - BackupRestoreUtil.getIncrementalPaths( - latestValidMetaFile.get(), dateRange, metaProxy)); + if (!config.skipIncrementalRestore()) { + allFiles.addAll( + BackupRestoreUtil.getIncrementalPaths( + latestValidMetaFile.get(), dateRange, metaProxy)); + } // Download snapshot which is listed in the meta file. List> futureList = new ArrayList<>(); From e3e371e03775881228ee153bfb0b141c1cc28e43 Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Wed, 29 Mar 2023 13:39:59 -0700 Subject: [PATCH 8/8] Remove added non-shaded Guava dependency pursuant to review comments. --- .../java/com/netflix/priam/backup/BackupRestoreUtil.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java index 687855d9e..c97ba72e3 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupRestoreUtil.java @@ -17,7 +17,6 @@ package com.netflix.priam.backup; -import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableMap; import com.netflix.priam.backupv2.IMetaProxy; import com.netflix.priam.backupv2.MetaV2Proxy; @@ -89,7 +88,9 @@ public static List getIncrementalPaths( else snapshotTime = latestValidMetaFile.getTime().toInstant(); DateUtil.DateRange incrementalDateRange = new DateUtil.DateRange(snapshotTime, dateRange.getEndTime()); - return Lists.newArrayList(metaProxy.getIncrementals(incrementalDateRange)); + List incrementalPaths = new ArrayList<>(); + metaProxy.getIncrementals(incrementalDateRange).forEachRemaining(incrementalPaths::add); + return incrementalPaths; } public static Map> getFilter(String inputFilter)