From 444d20ebd7d5404a8ba8561034809d2e486195ad Mon Sep 17 00:00:00 2001 From: Matt Lehman Date: Sun, 2 Apr 2023 12:07:39 -0700 Subject: [PATCH] Return missing files from snapshot verification continuing to marginalize BackupVerificationResult. Return all results valid or otherwise. --- .../priam/backup/BackupVerification.java | 29 +++++---- .../backupv2/BackupVerificationTask.java | 25 +++++--- .../priam/backup/TestBackupVerification.java | 59 ++++++++++++------- .../backupv2/TestBackupVerificationTask.java | 3 +- 4 files changed, 73 insertions(+), 43 deletions(-) diff --git a/priam/src/main/java/com/netflix/priam/backup/BackupVerification.java b/priam/src/main/java/com/netflix/priam/backup/BackupVerification.java index 67b01b086..85f240854 100644 --- a/priam/src/main/java/com/netflix/priam/backup/BackupVerification.java +++ b/priam/src/main/java/com/netflix/priam/backup/BackupVerification.java @@ -13,6 +13,8 @@ */ package com.netflix.priam.backup; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.netflix.priam.backupv2.IMetaProxy; import com.netflix.priam.utils.DateUtil; import com.netflix.priam.utils.DateUtil.DateRange; @@ -75,9 +77,9 @@ public Optional verifyLatestBackup( for (BackupMetadata backupMetadata : backupStatusMgr.getLatestBackupMetadata(backupVersion, dateRange)) { if (backupMetadata.getLastValidated() == null || force) { - Optional result = verifyBackup(metaProxy, backupMetadata); - if (result.isPresent()) { - return result; + BackupVerificationResult result = verifyBackup(metaProxy, backupMetadata); + if (result.valid) { + return Optional.of(result); } } else { updateLatestResult(backupMetadata); @@ -88,18 +90,20 @@ public Optional verifyLatestBackup( return Optional.empty(); } - public List verifyBackupsInRange( + public ImmutableMap> findMissingBackupFilesInRange( BackupVersion backupVersion, DateRange dateRange) throws IllegalArgumentException { IMetaProxy metaProxy = getMetaProxy(backupVersion); - List results = new ArrayList<>(); + ImmutableMap.Builder> mapBuilder = + ImmutableMap.builder(); for (BackupMetadata backupMetadata : backupStatusMgr.getLatestBackupMetadata(backupVersion, dateRange)) { - if (backupMetadata.getLastValidated() != null - || verifyBackup(metaProxy, backupMetadata).isPresent()) { - results.add(backupMetadata); - } + List missingFiles = + backupMetadata.getLastValidated() == null + ? verifyBackup(metaProxy, backupMetadata).filesInMetaOnly + : new ArrayList<>(); + mapBuilder.put(backupMetadata, ImmutableSet.copyOf(missingFiles)); } - return results; + return mapBuilder.build(); } /** returns the latest valid backup verification result if we have found one within the SLO * */ @@ -107,7 +111,7 @@ public Optional getLatestVerfifiedBackupTime() { return latestResult == null ? Optional.empty() : Optional.of(latestResult.snapshotInstant); } - private Optional verifyBackup( + private BackupVerificationResult verifyBackup( IMetaProxy metaProxy, BackupMetadata latestBackupMetaData) { Path metadataLocation = Paths.get(latestBackupMetaData.getSnapshotLocation()); metadataLocation = metadataLocation.subpath(1, metadataLocation.getNameCount()); @@ -119,9 +123,8 @@ private Optional verifyBackup( Date now = new Date(DateUtil.getInstant().toEpochMilli()); latestBackupMetaData.setLastValidated(now); backupStatusMgr.update(latestBackupMetaData); - return Optional.of(result); } - return Optional.empty(); + return result; } private void updateLatestResult(BackupMetadata backupMetadata) { diff --git a/priam/src/main/java/com/netflix/priam/backupv2/BackupVerificationTask.java b/priam/src/main/java/com/netflix/priam/backupv2/BackupVerificationTask.java index 06dc0646e..b5f21d7c4 100644 --- a/priam/src/main/java/com/netflix/priam/backupv2/BackupVerificationTask.java +++ b/priam/src/main/java/com/netflix/priam/backupv2/BackupVerificationTask.java @@ -30,7 +30,9 @@ import com.netflix.priam.utils.DateUtil.DateRange; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Singleton; import org.slf4j.Logger; @@ -84,21 +86,28 @@ public void execute() throws Exception { Instant slo = now.minus(backupRestoreConfig.getBackupVerificationSLOInHours(), ChronoUnit.HOURS); DateRange dateRange = new DateRange(slo, now); - List verifiedBackups = - backupVerification.verifyBackupsInRange( - BackupVersion.SNAPSHOT_META_SERVICE, dateRange); + Set verifiedBackups = + backupVerification + .findMissingBackupFilesInRange( + BackupVersion.SNAPSHOT_META_SERVICE, dateRange) + .entrySet() + .stream() + .filter(entry -> entry.getValue().isEmpty()) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); verifiedBackups .stream() - .filter(result -> result.getLastValidated().toInstant().isAfter(now)) + .filter(metadata -> metadata.getLastValidated().toInstant().isAfter(now)) .forEach( - result -> { + metadata -> { logger.info( "Sending {} message for backup: {}", AbstractBackupPath.BackupFileType.SNAPSHOT_VERIFIED, - result.getSnapshotLocation()); + metadata.getSnapshotLocation()); backupNotificationMgr.notify( - result.getSnapshotLocation(), result.getStart().toInstant()); + metadata.getSnapshotLocation(), + metadata.getStart().toInstant()); }); if (verifiedBackups.isEmpty()) { diff --git a/priam/src/test/java/com/netflix/priam/backup/TestBackupVerification.java b/priam/src/test/java/com/netflix/priam/backup/TestBackupVerification.java index dd233cc12..b87db5246 100644 --- a/priam/src/test/java/com/netflix/priam/backup/TestBackupVerification.java +++ b/priam/src/test/java/com/netflix/priam/backup/TestBackupVerification.java @@ -17,6 +17,7 @@ package com.netflix.priam.backup; +import com.google.common.truth.Truth; import com.google.inject.Guice; import com.google.inject.Injector; import com.netflix.priam.backup.AbstractBackupPath.BackupFileType; @@ -31,6 +32,7 @@ import java.nio.file.Paths; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.AbstractCollection; import java.util.Date; import java.util.List; import java.util.Optional; @@ -109,16 +111,23 @@ public void noBackup() throws Exception { @Test public void noBackupDateRange() throws Exception { - List backupVerificationResults = - backupVerification.verifyBackupsInRange( - BackupVersion.SNAPSHOT_BACKUP, new DateRange(Instant.now(), Instant.now())); - Assert.assertFalse(backupVerificationResults.size() > 0); + long foundBackups = + backupVerification + .findMissingBackupFilesInRange( + BackupVersion.SNAPSHOT_BACKUP, + new DateRange(Instant.now(), Instant.now())) + .entrySet() + .size(); + Truth.assertThat(foundBackups).isEqualTo(0L); - backupVerificationResults = - backupVerification.verifyBackupsInRange( - BackupVersion.SNAPSHOT_META_SERVICE, - new DateRange(Instant.now(), Instant.now())); - Assert.assertFalse(backupVerificationResults.size() > 0); + foundBackups = + backupVerification + .findMissingBackupFilesInRange( + BackupVersion.SNAPSHOT_META_SERVICE, + new DateRange(Instant.now(), Instant.now())) + .entrySet() + .size(); + Truth.assertThat(foundBackups).isEqualTo(0L); } private void setUp() throws Exception { @@ -184,12 +193,16 @@ public void verifyBackupVersion1() throws Exception { public void verifyBackupVersion1DateRange() throws Exception { setUp(); // Verify for backup version 1.0 - List backupVerificationResults = - backupVerification.verifyBackupsInRange( - BackupVersion.SNAPSHOT_BACKUP, - new DateRange(backupDate + "," + backupDateEnd)); - Assert.assertTrue(!backupVerificationResults.isEmpty()); - Assert.assertTrue(backupVerificationResults.size() == numFakeBackups); + long missingFilesCount = + backupVerification + .findMissingBackupFilesInRange( + BackupVersion.SNAPSHOT_BACKUP, + new DateRange(backupDate + "," + backupDateEnd)) + .values() + .stream() + .filter(AbstractCollection::isEmpty) + .count(); + Truth.assertThat(missingFilesCount).isEqualTo(numFakeBackups); List backupMetadata = backupStatusMgr.getLatestBackupMetadata( BackupVersion.SNAPSHOT_BACKUP, @@ -260,12 +273,16 @@ public void verifyBackupVersion2() throws Exception { public void verifyBackupVersion2DateRange() throws Exception { setUp(); // Verify for backup version 2.0 - List backupVerificationResults = - backupVerification.verifyBackupsInRange( - BackupVersion.SNAPSHOT_META_SERVICE, - new DateRange(backupDate + "," + backupDateEnd)); - Assert.assertTrue(!backupVerificationResults.isEmpty()); - Assert.assertTrue(backupVerificationResults.size() == numFakeBackups); + long missingFilesCount = + backupVerification + .findMissingBackupFilesInRange( + BackupVersion.SNAPSHOT_META_SERVICE, + new DateRange(backupDate + "," + backupDateEnd)) + .values() + .stream() + .filter(AbstractCollection::isEmpty) + .count(); + Truth.assertThat(missingFilesCount).isEqualTo(numFakeBackups); List backupMetadata = backupStatusMgr.getLatestBackupMetadata( BackupVersion.SNAPSHOT_META_SERVICE, diff --git a/priam/src/test/java/com/netflix/priam/backupv2/TestBackupVerificationTask.java b/priam/src/test/java/com/netflix/priam/backupv2/TestBackupVerificationTask.java index 50874d71e..31c0d92df 100644 --- a/priam/src/test/java/com/netflix/priam/backupv2/TestBackupVerificationTask.java +++ b/priam/src/test/java/com/netflix/priam/backupv2/TestBackupVerificationTask.java @@ -163,7 +163,8 @@ public void testRestoreMode(@Mocked InstanceState state) throws Exception { Truth.assertThat(badVerifications.count()).isEqualTo(0); new Verifications() { { - backupVerification.verifyBackupsInRange((BackupVersion) any, (DateRange) any); + backupVerification.findMissingBackupFilesInRange( + (BackupVersion) any, (DateRange) any); maxTimes = 0; }