Skip to content

Commit

Permalink
Do not delete backups if only the compression type differs. (#1103)
Browse files Browse the repository at this point in the history 
* Change default compression transition time to be far in the future rather than the epoch. This implies that just changing the compression mode will not immediately lead to a dangerous situation in which recent backups could be deleted erroneously.

* Remove LZ4 compression type. It is not supported in practice.

* Preliminary style tweak:  Rename variable to make it cleaner when we add a method in a few commits.

* Preliminary style tweak:  Use for-each iteration for readability.

* Compare files in meta and S3 in a compression-agnostic way.

* Add unit tests to test removing compression from remote paths. Change the approach accordingly.
  • Loading branch information
@mattl-netflix
mattl-netflix authored Oct 8, 2024
1 parent 7433fe8 commit 4f3172a
Show file tree
Hide file tree
Showing 8 changed files with 187 additions and 45 deletions.
8 changes: 0 additions & 8 deletions priam/src/main/java/com/netflix/priam/backup/AbstractBackupPath.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,6 @@ public enum BackupFileType {
public static boolean isDataFile(BackupFileType type) {
return DATA_FILE_TYPES.contains(type);
}

public static BackupFileType fromString(String s) throws BackupRestoreException {
try {
return BackupFileType.valueOf(s);
} catch (IllegalArgumentException e) {
throw new BackupRestoreException(String.format("Unknown BackupFileType %s", s));
}
}
}

protected BackupFileType type;
Expand Down
82 changes: 54 additions & 28 deletions priam/src/main/java/com/netflix/priam/backupv2/BackupTTLTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@

package com.netflix.priam.backupv2;

import com.netflix.priam.backup.*;
import com.google.api.client.util.Lists;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.netflix.priam.backup.AbstractBackupPath;
import com.netflix.priam.backup.BackupRestoreException;
import com.netflix.priam.backup.IBackupFileSystem;
import com.netflix.priam.backup.Status;
import com.netflix.priam.compress.CompressionType;
import com.netflix.priam.config.IBackupRestoreConfig;
import com.netflix.priam.config.IConfiguration;
import com.netflix.priam.health.InstanceState;
Expand All @@ -26,6 +34,15 @@
import com.netflix.priam.scheduler.Task;
import com.netflix.priam.scheduler.TaskTimer;
import com.netflix.priam.utils.DateUtil;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.math.Fraction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
Expand All @@ -34,14 +51,6 @@
import java.util.*;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.math.Fraction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This class is used to TTL or delete the SSTable components from the backups after they are not
Expand Down Expand Up @@ -69,6 +78,9 @@ public class BackupTTLTask extends Task {
private final int BATCH_SIZE = 1000;
private final Instant start_of_feature = DateUtil.parseInstant("201801010000");
private final int maxWaitMillis;
private static final int BACKUP_TYPE_INDEX = 3;
private static final Splitter PATH_SPLITTER = Splitter.on('/');
private static final Joiner PATH_JOINER = Joiner.on('/');

@Inject
public BackupTTLTask(
Expand Down Expand Up @@ -188,24 +200,24 @@ Since this is not the case, the TTL may end up deleting this file even though th
config.getGracePeriodDaysForCompaction());

while (remoteFileLocations.hasNext()) {
AbstractBackupPath abstractBackupPath = abstractBackupPathProvider.get();
abstractBackupPath.parseRemote(remoteFileLocations.next());
AbstractBackupPath backupPath = abstractBackupPathProvider.get();
backupPath.parseRemote(remoteFileLocations.next());
// If lastModifiedTime is after the dateToTTL, we should get out of this loop as
// remote file systems always give locations which are sorted.
if (abstractBackupPath.getLastModified().isAfter(dateToTtl)) {
if (backupPath.getLastModified().isAfter(dateToTtl)) {
logger.info(
"Breaking from TTL. Got a key which is after the TTL time: {}",
abstractBackupPath.getRemotePath());
backupPath.getRemotePath());
break;
}

if (!filesInMeta.containsKey(abstractBackupPath.getRemotePath())) {
deleteFile(abstractBackupPath, false);
if (!filesInMeta.containsKey(removeCompressionPart(backupPath.getRemotePath()))) {
deleteFile(backupPath, false);
} else {
if (logger.isDebugEnabled())
logger.debug(
"Not deleting this key as it is referenced in backups: {}",
abstractBackupPath.getRemotePath());
backupPath.getRemotePath());
}
}

Expand Down Expand Up @@ -260,18 +272,32 @@ public static TaskTimer getTimer(
private class MetaFileWalker extends MetaFileReader {
@Override
public void process(ColumnFamilyResult columnfamilyResult) {
columnfamilyResult
.getSstables()
.forEach(
ssTableResult ->
ssTableResult
.getSstableComponents()
.forEach(
fileUploadResult ->
filesInMeta.put(
fileUploadResult
.getBackupPath(),
null)));
for (ColumnFamilyResult.SSTableResult sstable : columnfamilyResult.getSstables()) {
for (FileUploadResult component : sstable.getSstableComponents()) {
filesInMeta.put(removeCompressionPart(component.getBackupPath()), null);
}
}
}
}

static String removeCompressionPart(String backupPath) {
List<String> parts = Lists.newArrayList(PATH_SPLITTER.split(backupPath));
AbstractBackupPath.BackupFileType fileType =
AbstractBackupPath.BackupFileType.valueOf(parts.get(BACKUP_TYPE_INDEX));
String compressionType;
if (fileType == AbstractBackupPath.BackupFileType.SST_V2) {
compressionType = parts.remove(7);
} else if (fileType == AbstractBackupPath.BackupFileType.SECONDARY_INDEX_V2) {
compressionType = parts.remove(8);
} else {
throw new IllegalStateException(
String.format("only %s, and %s, are supported, saw %s",
AbstractBackupPath.BackupFileType.SST_V2.name(),
AbstractBackupPath.BackupFileType.SECONDARY_INDEX_V2.name(),
fileType));
}
// check compressionType validity
CompressionType.valueOf(compressionType);
return PATH_JOINER.join(parts);
}
}
1 change: 1 addition & 0 deletions priam/src/main/java/com/netflix/priam/backupv2/MetaFileReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.priam.backupv2;

import com.google.gson.stream.JsonReader;
import com.netflix.priam.backup.BackupRestoreException;
import com.netflix.priam.utils.DateUtil;
import com.netflix.priam.utils.GsonJsonSerializer;
import java.io.FileNotFoundException;
Expand Down
1 change: 0 additions & 1 deletion priam/src/main/java/com/netflix/priam/compress/CompressionType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@

public enum CompressionType {
SNAPPY,
LZ4,
NONE
}
2 changes: 1 addition & 1 deletion priam/src/main/java/com/netflix/priam/config/IConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ default boolean addMD5ToBackupUploads() {
* @return the milliseconds since the epoch of the transition time.
*/
default long getCompressionTransitionEpochMillis() {
return 0L;
return Long.MAX_VALUE;
}

/** @return whether to enable auto_snapshot */
Expand Down
4 changes: 2 additions & 2 deletions priam/src/main/java/com/netflix/priam/notification/BackupNotificationMgr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ private Set<AbstractBackupPath.BackupFileType> getUpdatedNotifiedBackupFileTypes
for (String s : this.notifiedBackupFileTypes.split(",")) {
try {
AbstractBackupPath.BackupFileType backupFileType =
AbstractBackupPath.BackupFileType.fromString(s.trim());
AbstractBackupPath.BackupFileType.valueOf(s.trim());
notifiedBackupFileTypesSet.add(backupFileType);
} catch (BackupRestoreException ignored) {
} catch (IllegalArgumentException ignored) {
}
}
}
Expand Down
129 changes: 124 additions & 5 deletions priam/src/test/java/com/netflix/priam/backupv2/TestBackupTTLTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@

package com.netflix.priam.backupv2;

import com.google.common.collect.ImmutableList;
import com.google.common.truth.Truth;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.netflix.priam.backup.AbstractBackupPath;
import com.netflix.priam.backup.BRTestModule;
import com.netflix.priam.backup.FakeBackupFileSystem;
import com.netflix.priam.backup.Status;
import com.netflix.priam.backup.*;
import com.netflix.priam.config.BackupsToCompress;
import com.netflix.priam.config.FakeConfiguration;
import com.netflix.priam.config.IConfiguration;
import com.netflix.priam.health.InstanceState;
import com.netflix.priam.utils.BackupFileUtils;
Expand All @@ -43,6 +44,7 @@
import org.junit.After;
import org.junit.Assert;
import org.junit.Test;
import org.junit.jupiter.api.Assertions;

/** Created by aagrawal on 12/17/18. */
public class TestBackupTTLTask {
Expand Down Expand Up @@ -105,7 +107,6 @@ public void prepTest(int daysForSnapshot) throws Exception {
testBackupUtils.createFile("mc-7-Data.db", time.plus(20, ChronoUnit.MINUTES));
list.clear();
list.add(getRemoteFromLocal(file4));
// list.add(getRemoteFromLocal(file6));
list.add(getRemoteFromLocal(file7));
metas[2] = testBackupUtils.createMeta(list, time.plus(40, ChronoUnit.MINUTES));
allFiles.add(getRemoteFromLocal(file5));
Expand Down Expand Up @@ -206,4 +207,122 @@ public void testRestoreMode(@Mocked InstanceState state) throws Exception {
};
backupTTLService.execute();
}

@Test
public void testFileIsNotDeletedIfOnlyCompressionIsDifferent() throws Exception {
BackupFileUtils.cleanupDir(Paths.get(configuration.getDataFileLocation()));
int lookback =
configuration.getBackupRetentionDays() + configuration.getGracePeriodDaysForCompaction() + 1;
Instant time = DateUtil.getInstant().minus(lookback, ChronoUnit.DAYS);
String file = testBackupUtils.createFile("mc-1-Data.db", time);
time =
time.plus(configuration.getGracePeriodDaysForCompaction() + 1, ChronoUnit.DAYS)
.plus(20, ChronoUnit.MINUTES);
Path metapath = testBackupUtils.createMeta(ImmutableList.of(getRemoteFromLocal(file)), time);
AbstractBackupPath meta = pathProvider.get();
meta.parseLocal(metapath.toFile(), AbstractBackupPath.BackupFileType.META_V2);
((FakeConfiguration) configuration).setBackupsToCompress(BackupsToCompress.NONE);
String data = getRemoteFromLocal(file);
backupFileSystem.setupTest(ImmutableList.of(data, meta.getRemotePath()));

backupTTLService.execute();

Assert.assertTrue(getAllFiles().contains(data));
}

@Test
public void testRemoveCompressionPart_sst_NONE() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/table-9f5c6374d48532299a0a5094af9ad1e3/NONE/PLAINTEXT/nb-10-big-Statistics.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/table-9f5c6374d48532299a0a5094af9ad1e3/PLAINTEXT/nb-10-big-Statistics.db");
}

@Test
public void testRemoveCompressionPart_sst_SNAPPY() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/table-9f5c6374d48532299a0a5094af9ad1e3/SNAPPY/PLAINTEXT/nb-10-big-Statistics.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/table-9f5c6374d48532299a0a5094af9ad1e3/PLAINTEXT/nb-10-big-Statistics.db");
}

@Test
public void testRemoveCompressionPart_sst_unsupportedCompression() {
Assertions.assertThrows(IllegalArgumentException.class, () -> BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/table-9f5c6374d48532299a0a5094af9ad1e3/LZ4/PLAINTEXT/nb-10-big-Statistics.db"));
}

@Test
public void testRemoveCompressionPart_sst_SNAPPYInKeyspace() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/SNAPPY/table-9f5c6374d48532299a0a5094af9ad1e3/SNAPPY/PLAINTEXT/nb-10-big-Statistics.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/SNAPPY/table-9f5c6374d48532299a0a5094af9ad1e3/PLAINTEXT/nb-10-big-Statistics.db");
}

@Test
public void testRemoveCompressionPart_sst_NONEInKeyspace() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/NONE/table-9f5c6374d48532299a0a5094af9ad1e3/NONE/PLAINTEXT/nb-10-big-Statistics.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/NONE/table-9f5c6374d48532299a0a5094af9ad1e3/PLAINTEXT/nb-10-big-Statistics.db");
}

@Test
public void testRemoveCompressionPart_sst_SNAPPYInTable() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/SNAPPY-9f5c6374d48532299a0a5094af9ad1e3/SNAPPY/PLAINTEXT/nb-10-big-Statistics.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/SNAPPY-9f5c6374d48532299a0a5094af9ad1e3/PLAINTEXT/nb-10-big-Statistics.db");
}

@Test
public void testRemoveCompressionPart_sst_NONEInTable() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/NONE-9f5c6374d48532299a0a5094af9ad1e3/NONE/PLAINTEXT/nb-10-big-Statistics.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SST_V2/1724992379000/keyspace/NONE-9f5c6374d48532299a0a5094af9ad1e3/PLAINTEXT/nb-10-big-Statistics.db");
}

@Test
public void testRemoveCompressionPart_si_NONE() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/NONE/PLAINTEXT/keyspace-table.table_field_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/PLAINTEXT/keyspace-table.table_field_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_SNAPPY() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/SNAPPY/PLAINTEXT/keyspace-table.table_field_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/PLAINTEXT/keyspace-table.table_field_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_unsupportedCompression() {
Assertions.assertThrows(
IllegalArgumentException.class,
() -> BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/LZ4/PLAINTEXT/keyspace-table.table_field_idx-ka-1-CRC.db"));
}

@Test
public void testRemoveCompressionPart_si_SNAPPYInKeyspace() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/SNAPPY/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/SNAPPY/PLAINTEXT/SNAPPY-table.table_field_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/SNAPPY/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/PLAINTEXT/SNAPPY-table.table_field_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_NONEInKeyspace() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/NONE/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/NONE/PLAINTEXT/NONE-table.table_field_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/NONE/table-672cc1f038a311e68d78cd36d8a9052a/.table_field_idx/PLAINTEXT/NONE-table.table_field_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_SNAPPYInTable() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/SNAPPY-672cc1f038a311e68d78cd36d8a9052a/.SNAPPY_field_idx/SNAPPY/PLAINTEXT/keyspace-SNAPPY.SNAPPY_field_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/SNAPPY-672cc1f038a311e68d78cd36d8a9052a/.SNAPPY_field_idx/PLAINTEXT/keyspace-SNAPPY.SNAPPY_field_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_NONEInTable() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/NONE-672cc1f038a311e68d78cd36d8a9052a/.NONE_field_idx/NONE/PLAINTEXT/keyspace-NONE.NONE_field_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/NONE-672cc1f038a311e68d78cd36d8a9052a/.NONE_field_idx/PLAINTEXT/keyspace-NONE.NONE_field_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_SNAPPYInIndex() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_SNAPPY_idx/SNAPPY/PLAINTEXT/keyspace-table.table_SNAPPY_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_SNAPPY_idx/PLAINTEXT/keyspace-table.table_SNAPPY_idx-ka-1-CRC.db");
}

@Test
public void testRemoveCompressionPart_si_NONEInIndex() {
Truth.assertThat(BackupTTLTask.removeCompressionPart("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_NONE_idx/NONE/PLAINTEXT/keyspace-table.table_NONE_idx-ka-1-CRC.db"))
.isEqualTo("test_backup/1234_cass_foo/-4419532432517671141/SECONDARY_INDEX_V2/1724992379000/keyspace/table-672cc1f038a311e68d78cd36d8a9052a/.table_NONE_idx/PLAINTEXT/keyspace-table.table_NONE_idx-ka-1-CRC.db");
}
}
5 changes: 5 additions & 0 deletions priam/src/test/java/com/netflix/priam/config/FakeConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ public void setCreateNewToken(boolean mayCreateNewToken) {
this.mayCreateNewToken = mayCreateNewToken;
}

public FakeConfiguration setBackupsToCompress(BackupsToCompress backupsToCompress) {
setFakeConfig("Priam.backupsToCompress", backupsToCompress);
return this;
}

public BackupsToCompress getBackupsToCompress() {
return (BackupsToCompress)
fakeConfig.getOrDefault("Priam.backupsToCompress", BackupsToCompress.ALL);
Expand Down

0 comments on commit 4f3172a

Please sign in to comment.