This is an automated email from the ASF dual-hosted git repository.
siyao pushed a commit to branch HDDS-6517-Snapshot
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/HDDS-6517-Snapshot by this
push:
new 125eccfef2 HDDS-7607. [Snapshot] SnapshotDiff command lists unmodified
file as modified (#4108)
125eccfef2 is described below
commit 125eccfef261f598ae65d353c3d0f2e32bbae43b
Author: Sadanand Shenoy <[email protected]>
AuthorDate: Wed Jan 4 01:02:32 2023 +0530
HDDS-7607. [Snapshot] SnapshotDiff command lists unmodified file as
modified (#4108)
---
.../org/apache/ozone/rocksdb/util/RdbUtil.java | 25 +++--
.../ozone/om/helpers/OmKeyLocationInfoGroup.java | 19 ++++
.../hadoop/ozone/snapshot/SnapshotDiffReport.java | 6 +-
.../hadoop/ozone/om/helpers/TestOmKeyInfo.java | 6 +-
.../org/apache/hadoop/ozone/om/TestOmSnapshot.java | 99 ++++++++++++++++---
.../ozone/om/snapshot/SnapshotDiffManager.java | 106 +++++++++++++++------
6 files changed, 201 insertions(+), 60 deletions(-)
diff --git
a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java
b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java
index 15757df7b5..fbf6e731de 100644
---
a/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java
+++
b/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java
@@ -40,21 +40,20 @@ public final class RdbUtil {
private RdbUtil() { }
- public static Set<String> getKeyTableSSTFiles(final String dbLocation)
- throws RocksDBException {
- final List<ColumnFamilyHandle> columnFamilyHandles = new ArrayList<>();
+ public static Set<String> getSSTFilesForComparison(final String dbLocation,
+ List<String> cfs) throws RocksDBException {
+ final List<ColumnFamilyHandle> columnFamilyHandles = new ArrayList<>();
final List<ColumnFamilyDescriptor> cfd = new ArrayList<>();
+ for (String columnFamily : cfs) {
+ cfd.add(new ColumnFamilyDescriptor(
+ columnFamily.getBytes(StandardCharsets.UTF_8)));
+ }
cfd.add(
- new ColumnFamilyDescriptor(
- "keyTable".getBytes(StandardCharsets.UTF_8)));
- cfd.add(
- new ColumnFamilyDescriptor(
- "default".getBytes(StandardCharsets.UTF_8)));
- try (DBOptions options = new DBOptions();
- RocksDB rocksDB = RocksDB.openReadOnly(options, dbLocation,
- cfd, columnFamilyHandles)) {
- return rocksDB.getLiveFilesMetaData().stream().map(lfm ->
- new File(lfm.path(), lfm.fileName()).getPath())
+ new
ColumnFamilyDescriptor("default".getBytes(StandardCharsets.UTF_8)));
+ try (DBOptions options = new DBOptions(); RocksDB rocksDB = RocksDB
+ .openReadOnly(options, dbLocation, cfd, columnFamilyHandles)) {
+ return rocksDB.getLiveFilesMetaData().stream()
+ .map(lfm -> new File(lfm.path(), lfm.fileName()).getPath())
.collect(Collectors.toCollection(HashSet::new));
}
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyLocationInfoGroup.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyLocationInfoGroup.java
index 2f02db087e..991f366664 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyLocationInfoGroup.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyLocationInfoGroup.java
@@ -16,6 +16,7 @@
*/
package org.apache.hadoop.ozone.om.helpers;
+import com.google.common.base.Objects;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyLocationList;
@@ -186,4 +187,22 @@ public class OmKeyLocationInfoGroup {
}
return sb.toString();
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ OmKeyLocationInfoGroup that = (OmKeyLocationInfoGroup) o;
+ return version == that.version && isMultipartKey == that.isMultipartKey
+ && Objects.equal(locationVersionMap, that.locationVersionMap);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(version, locationVersionMap, isMultipartKey);
+ }
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReport.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReport.java
index 24b8af76c5..e8e387099c 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReport.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReport.java
@@ -126,9 +126,9 @@ public class SnapshotDiffReport {
}
if (other instanceof DiffReportEntry) {
DiffReportEntry entry = (DiffReportEntry) other;
- return type.equals(entry.getType())
- && sourcePath.equals(entry.sourcePath)
- && targetPath.equals(entry.targetPath);
+ return this.type.equals(entry.getType()) && this.sourcePath
+ .equals(entry.sourcePath) && (this.targetPath != null ?
+ this.targetPath.equals(entry.targetPath) : true);
}
return false;
}
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java
index 50218eaf9a..f4e5fc9932 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java
@@ -143,9 +143,9 @@ public class TestOmKeyInfo {
OmKeyInfo cloneKey = key.copyObject();
- // Because for OmKeyLocationInfoGroup we have not implemented equals()
- // method, so it checks only references.
- Assert.assertNotEquals(key, cloneKey);
+ // OmKeyLocationInfoGroup has now implemented equals() method.
+ // assertEquals should work now.
+ Assert.assertEquals(key, cloneKey);
// Check each version content here.
Assert.assertEquals(key.getKeyLocationVersions().size(),
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
index 2d62d22879..719b07c222 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
@@ -17,6 +17,7 @@
package org.apache.hadoop.ozone.om;
import java.util.List;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
@@ -38,6 +39,7 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffReport;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ozone.test.LambdaTestUtils;
import org.junit.Assert;
@@ -74,6 +76,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
* Test OmSnapshot bucket interface.
*/
@RunWith(Parameterized.class)
+@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT")
public class TestOmSnapshot {
private static MiniOzoneCluster cluster = null;
private static String volumeName;
@@ -396,23 +399,90 @@ public class TestOmSnapshot {
() -> createSnapshot(volume, bucket));
}
+ @Test
+ public void testSnapDiff() throws Exception {
+ String volume = "vol-" + RandomStringUtils.randomNumeric(5);
+ String bucket = "buck-" + RandomStringUtils.randomNumeric(5);
+ store.createVolume(volume);
+ OzoneVolume volume1 = store.getVolume(volume);
+ volume1.createBucket(bucket);
+ OzoneBucket bucket1 = volume1.getBucket(bucket);
+ // Create Key1 and take snapshot
+ String key1 = "key-1-";
+ key1 = createFileKey(bucket1, key1);
+ String snap1 = "snap" + RandomStringUtils.randomNumeric(5);
+ createSnapshot(volume, bucket, snap1);
+ // Do nothing, take another snapshot
+ String snap2 = "snap" + RandomStringUtils.randomNumeric(5);
+ createSnapshot(volume, bucket, snap2);
+ SnapshotDiffReport diff1 = store.snapshotDiff(volume, bucket, snap1,
snap2);
+ Assert.assertTrue(diff1.getDiffList().isEmpty());
+ // Create Key2 and delete Key1, take snapshot
+ String key2 = "key-2-";
+ key2 = createFileKey(bucket1, key2);
+ bucket1.deleteKey(key1);
+ String snap3 = "snap" + RandomStringUtils.randomNumeric(5);
+ createSnapshot(volume, bucket, snap3);
+ // Diff should have 2 entries
+ SnapshotDiffReport diff2 = store.snapshotDiff(volume, bucket, snap2,
snap3);
+ Assert.assertEquals(2, diff2.getDiffList().size());
+ Assert.assertTrue(diff2.getDiffList().contains(
+ SnapshotDiffReport.DiffReportEntry
+ .of(SnapshotDiffReport.DiffType.CREATE, key2)));
+ Assert.assertTrue(diff2.getDiffList().contains(
+ SnapshotDiffReport.DiffReportEntry
+ .of(SnapshotDiffReport.DiffType.DELETE, key1)));
+
+ // Rename Key2
+ String key2Renamed = key2 + "_renamed";
+ bucket1.renameKey(key2, key2Renamed);
+ String snap4 = "snap" + RandomStringUtils.randomNumeric(5);
+ createSnapshot(volume, bucket, snap4);
+ SnapshotDiffReport diff3 = store.snapshotDiff(volume, bucket, snap3,
snap4);
+ Assert.assertEquals(1, diff3.getDiffList().size());
+ Assert.assertTrue(diff3.getDiffList().contains(
+ SnapshotDiffReport.DiffReportEntry
+ .of(SnapshotDiffReport.DiffType.RENAME, key2, key2Renamed)));
+
+
+ // Create a directory
+ String dir1 = "dir-1" + RandomStringUtils.randomNumeric(5);
+ bucket1.createDirectory(dir1);
+ String snap5 = "snap" + RandomStringUtils.randomNumeric(5);
+ createSnapshot(volume, bucket, snap5);
+ SnapshotDiffReport diff4 = store.snapshotDiff(volume, bucket, snap4,
snap5);
+ Assert.assertEquals(1, diff4.getDiffList().size());
+ // for non-fso, directories are a special type of key with "/" appended
+ // at the end.
+ if (!bucket1.getBucketLayout().isFileSystemOptimized()) {
+ dir1 = dir1 + OM_KEY_PREFIX;
+ }
+ Assert.assertTrue(diff4.getDiffList().contains(
+ SnapshotDiffReport.DiffReportEntry
+ .of(SnapshotDiffReport.DiffType.CREATE, dir1)));
+
+ }
+
private String createSnapshot(String volName, String buckName)
throws IOException, InterruptedException, TimeoutException {
- String snapshotName = UUID.randomUUID().toString();
+ return createSnapshot(volName, buckName, UUID.randomUUID().toString());
+ }
+
+ private String createSnapshot(String volName, String buckName,
+ String snapshotName)
+ throws IOException, InterruptedException, TimeoutException {
store.createSnapshot(volName, buckName, snapshotName);
- String snapshotKeyPrefix = OmSnapshotManager
- .getSnapshotPrefix(snapshotName);
- SnapshotInfo snapshotInfo = leaderOzoneManager
- .getMetadataManager()
- .getSnapshotInfoTable()
- .get(SnapshotInfo.getTableKey(volName, buckName, snapshotName));
- String snapshotDirName = metaDir + OM_KEY_PREFIX +
- OM_SNAPSHOT_DIR + OM_KEY_PREFIX + OM_DB_NAME +
- snapshotInfo.getCheckpointDirName() + OM_KEY_PREFIX + "CURRENT";
- GenericTestUtils.waitFor(() -> new File(snapshotDirName).exists(),
- 1000, 120000);
+ String snapshotKeyPrefix =
+ OmSnapshotManager.getSnapshotPrefix(snapshotName);
+ SnapshotInfo snapshotInfo =
+ leaderOzoneManager.getMetadataManager().getSnapshotInfoTable()
+ .get(SnapshotInfo.getTableKey(volName, buckName, snapshotName));
+ String snapshotDirName =
+ metaDir + OM_KEY_PREFIX + OM_SNAPSHOT_DIR + OM_KEY_PREFIX + OM_DB_NAME
+ + snapshotInfo.getCheckpointDirName() + OM_KEY_PREFIX + "CURRENT";
+ GenericTestUtils
+ .waitFor(() -> new File(snapshotDirName).exists(), 1000, 120000);
return snapshotKeyPrefix;
-
}
private void deleteKeys(OzoneBucket bucket) throws IOException {
@@ -423,13 +493,14 @@ public class TestOmSnapshot {
}
}
- private void createFileKey(OzoneBucket bucket, String keyPrefix)
+ private String createFileKey(OzoneBucket bucket, String keyPrefix)
throws IOException {
byte[] value = RandomStringUtils.randomAscii(10240).getBytes(UTF_8);
String key = keyPrefix + RandomStringUtils.randomNumeric(5);
OzoneOutputStream fileKey = bucket.createKey(key, value.length);
fileKey.write(value);
fileKey.close();
+ return key;
}
@Test
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java
index c1b811ffa4..ec70478312 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java
@@ -22,17 +22,21 @@ import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OmSnapshot;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.WithObjectID;
import org.apache.hadoop.ozone.snapshot.SnapshotDiffReport;
import org.apache.hadoop.ozone.snapshot.SnapshotDiffReport.DiffType;
import org.apache.hadoop.ozone.snapshot.SnapshotDiffReport.DiffReportEntry;
import org.apache.ozone.rocksdb.util.ManagedSstFileReader;
import org.apache.ozone.rocksdb.util.RdbUtil;
+import org.jetbrains.annotations.NotNull;
import org.rocksdb.RocksDBException;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -54,28 +58,10 @@ public class SnapshotDiffManager {
// TODO: Once RocksDBCheckpointDiffer exposes method to get list
// of delta SST files, plug it in here.
- Set<String> fromSnapshotFiles = RdbUtil.getKeyTableSSTFiles(fromSnapshot
- .getMetadataManager().getStore().getDbLocation().getPath());
- Set<String> toSnapshotFiles = RdbUtil.getKeyTableSSTFiles(toSnapshot
- .getMetadataManager().getStore().getDbLocation().getPath());
-
- final Set<String> deltaFiles = new HashSet<>();
- deltaFiles.addAll(fromSnapshotFiles);
- deltaFiles.addAll(toSnapshotFiles);
-
- // TODO: Filter out the files.
-
- final Stream<String> keysToCheck = new ManagedSstFileReader(deltaFiles)
- .getKeyStream();
-
final BucketLayout bucketLayout = getBucketLayout(volume, bucket,
fromSnapshot.getMetadataManager());
- final Table<String, OmKeyInfo> fsKeyTable = fromSnapshot
- .getMetadataManager().getKeyTable(bucketLayout);
- final Table<String, OmKeyInfo> tsKeyTable = toSnapshot
- .getMetadataManager().getKeyTable(bucketLayout);
-
+ // TODO: Filter out the files.
/*
* The reason for having ObjectID to KeyName mapping instead of OmKeyInfo
* is to reduce the memory footprint.
@@ -87,22 +73,65 @@ public class SnapshotDiffManager {
final Set<Long> objectIDsToCheck = new HashSet<>();
+ // add to object ID map for key/file.
+
+ final Table<String, OmKeyInfo> fsKeyTable = fromSnapshot
+ .getMetadataManager().getKeyTable(bucketLayout);
+ final Table<String, OmKeyInfo> tsKeyTable = toSnapshot
+ .getMetadataManager().getKeyTable(bucketLayout);
+ final Set<String> deltaFilesForKeyOrFileTable =
+ getDeltaFiles(fromSnapshot, toSnapshot,
+ Collections.singletonList(fsKeyTable.getName()));
+
+ addToObjectIdMap(fsKeyTable, tsKeyTable, deltaFilesForKeyOrFileTable,
+ oldObjIdToKeyMap, newObjIdToKeyMap, objectIDsToCheck, false);
+
+ if (bucketLayout.isFileSystemOptimized()) {
+ // add to object ID map for directory.
+ final Table<String, OmDirectoryInfo> fsDirTable =
+ fromSnapshot.getMetadataManager().getDirectoryTable();
+ final Table<String, OmDirectoryInfo> tsDirTable =
+ toSnapshot.getMetadataManager().getDirectoryTable();
+ final Set<String> deltaFilesForDirTable =
+ getDeltaFiles(fromSnapshot, toSnapshot,
+ Collections.singletonList(fsDirTable.getName()));
+ addToObjectIdMap(fsDirTable, tsDirTable, deltaFilesForDirTable,
+ oldObjIdToKeyMap, newObjIdToKeyMap, objectIDsToCheck, true);
+ }
+
+ return new SnapshotDiffReport(volume, bucket, fromSnapshot.getName(),
+ toSnapshot.getName(), generateDiffReport(objectIDsToCheck,
+ oldObjIdToKeyMap, newObjIdToKeyMap));
+ }
+
+ private void addToObjectIdMap(Table<String, ? extends WithObjectID> fsTable,
+ Table<String, ? extends WithObjectID> tsTable, Set<String> deltaFiles,
+ Map<Long, String> oldObjIdToKeyMap, Map<Long, String> newObjIdToKeyMap,
+ Set<Long> objectIDsToCheck, boolean isDirectoryTable)
+ throws RocksDBException {
+ if (deltaFiles.isEmpty()) {
+ return;
+ }
+ final Stream<String> keysToCheck =
+ new ManagedSstFileReader(deltaFiles).getKeyStream();
keysToCheck.forEach(key -> {
try {
- final OmKeyInfo oldKey = fsKeyTable.get(key);
- final OmKeyInfo newKey = tsKeyTable.get(key);
+ final WithObjectID oldKey = fsTable.get(key);
+ final WithObjectID newKey = tsTable.get(key);
if (areKeysEqual(oldKey, newKey)) {
// We don't have to do anything.
return;
}
if (oldKey != null) {
final long oldObjId = oldKey.getObjectID();
- oldObjIdToKeyMap.put(oldObjId, oldKey.getKeyName());
+ oldObjIdToKeyMap
+ .put(oldObjId, getKeyOrDirectoryName(isDirectoryTable, oldKey));
objectIDsToCheck.add(oldObjId);
}
if (newKey != null) {
final long newObjId = newKey.getObjectID();
- newObjIdToKeyMap.put(newObjId, newKey.getKeyName());
+ newObjIdToKeyMap
+ .put(newObjId, getKeyOrDirectoryName(isDirectoryTable, newKey));
objectIDsToCheck.add(newObjId);
}
} catch (IOException e) {
@@ -110,10 +139,33 @@ public class SnapshotDiffManager {
}
});
keysToCheck.close();
+ }
- return new SnapshotDiffReport(volume, bucket, fromSnapshot.getName(),
- toSnapshot.getName(), generateDiffReport(objectIDsToCheck,
- oldObjIdToKeyMap, newObjIdToKeyMap));
+ private String getKeyOrDirectoryName(boolean isDirectory,
+ WithObjectID object) {
+ if (isDirectory) {
+ OmDirectoryInfo directoryInfo = (OmDirectoryInfo) object;
+ return directoryInfo.getName();
+ }
+ OmKeyInfo keyInfo = (OmKeyInfo) object;
+ return keyInfo.getKeyName();
+ }
+
+ @NotNull
+ private Set<String> getDeltaFiles(OmSnapshot fromSnapshot,
+ OmSnapshot toSnapshot, List<String> tablesToLookUp)
+ throws RocksDBException {
+ Set<String> fromSnapshotFiles = RdbUtil.getSSTFilesForComparison(
+ fromSnapshot.getMetadataManager().getStore().getDbLocation().getPath(),
+ tablesToLookUp);
+ Set<String> toSnapshotFiles = RdbUtil.getSSTFilesForComparison(
+ toSnapshot.getMetadataManager().getStore().getDbLocation().getPath(),
+ tablesToLookUp);
+
+ final Set<String> deltaFiles = new HashSet<>();
+ deltaFiles.addAll(fromSnapshotFiles);
+ deltaFiles.addAll(toSnapshotFiles);
+ return deltaFiles;
}
private List<DiffReportEntry> generateDiffReport(
@@ -224,7 +276,7 @@ public class SnapshotDiffManager {
return mManager.getBucketTable().get(bucketTableKey).getBucketLayout();
}
- private boolean areKeysEqual(OmKeyInfo oldKey, OmKeyInfo newKey) {
+ private boolean areKeysEqual(WithObjectID oldKey, WithObjectID newKey) {
if (oldKey == null && newKey == null) {
return true;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
