This is an automated email from the ASF dual-hosted git repository.
hemant pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new f6f1769059 HDDS-9597. Snapdiff fails in case of key renames to deleted
directories (#5526)
f6f1769059 is described below
commit f6f176905972f164292f655bd61712043e2a07b6
Author: Swaminathan Balachandran <[email protected]>
AuthorDate: Thu Nov 9 11:16:17 2023 -0800
HDDS-9597. Snapdiff fails in case of key renames to deleted directories
(#5526)
---
.../org/apache/hadoop/ozone/om/TestOmSnapshot.java | 39 ++++++++++
.../om/snapshot/FSODirectoryPathResolver.java | 7 +-
.../ozone/om/snapshot/ObjectPathResolver.java | 8 ++-
.../ozone/om/snapshot/SnapshotDiffManager.java | 83 +++++++++++++---------
.../om/snapshot/TestFSODirectoryPathResolver.java | 4 +-
5 files changed, 103 insertions(+), 38 deletions(-)
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 096d21d8e9..8d33ab999a 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
@@ -1068,6 +1068,45 @@ public class TestOmSnapshot {
Assert.assertEquals(diff.getDiffList(), diffEntries);
}
+ /**
+ * Testing scenario:
+ * 1) Key k1 is created.
+ * 2) Snapshot snap1 created.
+ * 3) Dir dir1/dir2 is created.
+ * 4) Key k1 is renamed to key dir1/dir2/k1_renamed
+ * 5) Dir dir1 is deleted.
+ * 6) Snapshot snap2 created.
+ * 5) Snapdiff b/w snapshot of Active FS & snap1 taken to assert difference
+ * of 1 delete key entry.
+ */
+ @Test
+ public void testSnapDiffWithDirectoryDelete() throws Exception {
+ Assume.assumeTrue(bucketLayout.isFileSystemOptimized());
+ String testVolumeName = "vol" + counter.incrementAndGet();
+ String testBucketName = "bucket1";
+ store.createVolume(testVolumeName);
+ OzoneVolume volume = store.getVolume(testVolumeName);
+ volume.createBucket(testBucketName);
+ OzoneBucket bucket = volume.getBucket(testBucketName);
+ String snap1 = "snap1";
+ String key1 = "k1";
+ key1 = createFileKeyWithPrefix(bucket, key1);
+ createSnapshot(testVolumeName, testBucketName, snap1);
+ bucket.createDirectory("dir1/dir2");
+ String key1Renamed = "dir1/dir2/" + key1 + "_renamed";
+ bucket.renameKey(key1, key1Renamed);
+ bucket.deleteDirectory("dir1", true);
+ String snap2 = "snap2";
+ createSnapshot(testVolumeName, testBucketName, snap2);
+ SnapshotDiffReport diff = getSnapDiffReport(testVolumeName, testBucketName,
+ snap1, snap2);
+
+ List<SnapshotDiffReport.DiffReportEntry> diffEntries = Lists.newArrayList(
+ SnapshotDiffReportOzone.getDiffReportEntry(
+ SnapshotDiffReport.DiffType.DELETE, key1));
+ Assert.assertEquals(diff.getDiffList(), diffEntries);
+ }
+
private OzoneObj buildKeyObj(OzoneBucket bucket, String key) {
return OzoneObjInfo.Builder.newBuilder()
.setResType(OzoneObj.ResourceType.KEY)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/FSODirectoryPathResolver.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/FSODirectoryPathResolver.java
index 0d0bcb2276..d37a37f20b 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/FSODirectoryPathResolver.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/FSODirectoryPathResolver.java
@@ -66,12 +66,15 @@ public class FSODirectoryPathResolver implements
ObjectPathResolver {
* Assuming all dirObjIds belong to a bucket this function resolves absolute
* path for a given FSO bucket.
* @param dirObjIds Object Ids corresponding to which absolute path is
needed.
+ * @param skipUnresolvedObjs boolean value to skipUnresolved objects when
+ * false exception will be thrown.
* @return Map of Path corresponding to provided directory object IDs
*/
@SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
@Override
public Map<Long, Path> getAbsolutePathForObjectIDs(
- Optional<Set<Long>> dirObjIds) throws IOException {
+ Optional<Set<Long>> dirObjIds, boolean skipUnresolvedObjs)
+ throws IOException {
// Root of a bucket would always have the
// key as /volumeId/bucketId/bucketId/
if (!dirObjIds.isPresent() || dirObjIds.get().isEmpty()) {
@@ -100,7 +103,7 @@ public class FSODirectoryPathResolver implements
ObjectPathResolver {
}
}
// Invalid directory objectId which does not exist in the given bucket.
- if (objIds.size() > 0) {
+ if (objIds.size() > 0 && !skipUnresolvedObjs) {
throw new IllegalArgumentException(
"Dir object Ids required but not found in bucket: " + objIds);
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ObjectPathResolver.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ObjectPathResolver.java
index b9c5bb0c45..dc6afed649 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ObjectPathResolver.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ObjectPathResolver.java
@@ -29,6 +29,12 @@ import java.util.Set;
*/
public interface ObjectPathResolver {
- Map<Long, Path> getAbsolutePathForObjectIDs(Optional<Set<Long>> objIds)
+ Map<Long, Path> getAbsolutePathForObjectIDs(Optional<Set<Long>> objIds,
+ boolean skipUnresolvedObjs)
throws IOException;
+
+ default Map<Long, Path> getAbsolutePathForObjectIDs(
+ Optional<Set<Long>> objIds) throws IOException {
+ return getAbsolutePathForObjectIDs(objIds, false);
+ }
}
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 0b38607f81..e0cba9575f 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
@@ -962,7 +962,7 @@ public class SnapshotDiffManager implements AutoCloseable {
newParentIdPathMap.get().putAll(new FSODirectoryPathResolver(
tablePrefix, bucketId,
toSnapshot.getMetadataManager().getDirectoryTable())
- .getAbsolutePathForObjectIDs(newParentIds));
+ .getAbsolutePathForObjectIDs(newParentIds, true));
}
return null;
},
@@ -1248,16 +1248,22 @@ public class SnapshotDiffManager implements
AutoCloseable {
@SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
private String resolveBucketRelativePath(boolean isFSOBucket,
- final Optional<Map<Long, Path>> parentIdMap, byte[] keyVal)
+ final Optional<Map<Long, Path>> parentIdMap, byte[] keyVal,
+ boolean skipUnresolvedObjIds)
throws IOException {
String key = codecRegistry.asObject(keyVal, String.class);
if (isFSOBucket) {
String[] splitKey = key.split(OM_KEY_PREFIX, 2);
Long parentId = Long.valueOf(splitKey[0]);
if (parentIdMap.map(m -> !m.containsKey(parentId)).orElse(true)) {
- throw new IllegalStateException(String.format(
- "Cannot resolve path for key: %s with parent Id: %d", key,
- parentId));
+ if (skipUnresolvedObjIds) {
+ return null;
+ } else {
+ throw new IllegalStateException(String.format(
+ "Cannot resolve path for key: %s with parent Id: %d", key,
+ parentId));
+ }
+
}
return parentIdMap.map(m -> m.get(parentId).resolve(splitKey[1]))
.get().toString().substring(1);
@@ -1350,47 +1356,56 @@ public class SnapshotDiffManager implements
AutoCloseable {
"Old and new key name both are null");
} else if (oldKeyName == null) { // Key Created.
String key = resolveBucketRelativePath(isFSOBucket,
- newParentIdPathMap, newKeyName);
- DiffReportEntry entry =
- SnapshotDiffReportOzone.getDiffReportEntry(CREATE, key);
- createDiffs.add(codecRegistry.asRawData(entry));
+ newParentIdPathMap, newKeyName, true);
+ if (key != null) {
+ DiffReportEntry entry =
+ SnapshotDiffReportOzone.getDiffReportEntry(CREATE, key);
+ createDiffs.add(codecRegistry.asRawData(entry));
+ }
} else if (newKeyName == null) { // Key Deleted.
String key = resolveBucketRelativePath(isFSOBucket,
- oldParentIdPathMap, oldKeyName);
+ oldParentIdPathMap, oldKeyName, false);
DiffReportEntry entry =
SnapshotDiffReportOzone.getDiffReportEntry(DELETE, key);
deleteDiffs.add(codecRegistry.asRawData(entry));
} else if (isDirectoryObject &&
Arrays.equals(oldKeyName, newKeyName)) {
String key = resolveBucketRelativePath(isFSOBucket,
- newParentIdPathMap, newKeyName);
- DiffReportEntry entry =
- SnapshotDiffReportOzone.getDiffReportEntry(MODIFY, key);
- modifyDiffs.add(codecRegistry.asRawData(entry));
+ newParentIdPathMap, newKeyName, true);
+ if (key != null) {
+ DiffReportEntry entry =
+ SnapshotDiffReportOzone.getDiffReportEntry(MODIFY, key);
+ modifyDiffs.add(codecRegistry.asRawData(entry));
+ }
} else {
String keyPrefix = getTablePrefix(tablePrefix,
(isDirectoryObject ? fsDirTable : fsTable).getName());
String oldKey = resolveBucketRelativePath(isFSOBucket,
- oldParentIdPathMap, oldKeyName);
- // Check if block location is same or not. If it is not same,
- // key must have been overridden as well.
- boolean isObjectModified = isObjectModified(
- keyPrefix + codecRegistry.asObject(oldKeyName, String.class),
- keyPrefix + codecRegistry.asObject(newKeyName, String.class),
- isDirectoryObject ? fsDirTable : fsTable,
- isDirectoryObject ? tsDirTable : tsTable);
- if (isObjectModified) {
- // Here, oldKey name is returned as modified. Modified key name
is
- // based on base snapshot (from snapshot).
- modifyDiffs.add(codecRegistry.asRawData(
- SnapshotDiffReportOzone.getDiffReportEntry(MODIFY, oldKey)));
- }
- if (!isObjectModified || !Arrays.equals(oldKeyName, newKeyName)) {
- String newKey = resolveBucketRelativePath(isFSOBucket,
- newParentIdPathMap, newKeyName);
- renameDiffs.add(codecRegistry.asRawData(
- SnapshotDiffReportOzone.getDiffReportEntry(RENAME, oldKey,
- newKey)));
+ oldParentIdPathMap, oldKeyName, false);
+ String newKey = resolveBucketRelativePath(isFSOBucket,
+ newParentIdPathMap, newKeyName, true);
+ if (newKey == null) {
+ deleteDiffs.add(codecRegistry.asRawData(SnapshotDiffReportOzone
+ .getDiffReportEntry(DELETE, oldKey)));
+ } else {
+ // Check if block location is same or not. If it is not same,
+ // key must have been overridden as well.
+ boolean isObjectModified = isObjectModified(
+ keyPrefix + codecRegistry.asObject(oldKeyName, String.class),
+ keyPrefix + codecRegistry.asObject(newKeyName, String.class),
+ isDirectoryObject ? fsDirTable : fsTable,
+ isDirectoryObject ? tsDirTable : tsTable);
+ if (isObjectModified) {
+ // Here, oldKey name is returned as modified. Modified key name
+ // is based on base snapshot (from snapshot).
+ modifyDiffs.add(codecRegistry.asRawData(SnapshotDiffReportOzone
+ .getDiffReportEntry(MODIFY, oldKey)));
+ }
+ if (!isObjectModified || !Arrays.equals(oldKeyName, newKeyName))
{
+ renameDiffs.add(codecRegistry.asRawData(
+ SnapshotDiffReportOzone.getDiffReportEntry(RENAME, oldKey,
+ newKey)));
+ }
}
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java
index f56b8657e4..22120409f9 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestFSODirectoryPathResolver.java
@@ -127,6 +127,8 @@ public class TestFSODirectoryPathResolver {
new FSODirectoryPathResolver(prefix, 1,
getMockedDirectoryInfoTable(prefix, dirMap));
Set<Long> objIds = Sets.newHashSet(17L, 9L, 10L, 15L, 4L, 3L, 1L);
+ Set<Long> invalidObjIds = Sets.newHashSet(17L, 9L, 10L, 15L, 4L, 3L, 1L,
+ 19L);
Map<Long, Path> absolutePathMap = fsoDirectoryPathResolver
.getAbsolutePathForObjectIDs(Optional.of(objIds));
@@ -143,7 +145,7 @@ public class TestFSODirectoryPathResolver {
// Invalid Obj Id 19 with dirInfo dir19 which is not present in the bucket.
Assertions.assertThrows(IllegalArgumentException.class,
() -> fsoDirectoryPathResolver.getAbsolutePathForObjectIDs(
- Optional.of(Sets.newHashSet(17L, 9L, 10L, 15L, 4L, 3L, 1L, 19L))),
+ Optional.of(invalidObjIds)),
"Dir object Ids required but not found in bucket: [19]");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
