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 2726ecc5064..ff5a13a4b61 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 @@ -58,6 +58,7 @@ import org.jetbrains.annotations.NotNull; import org.junit.Assert; import org.junit.AfterClass; +import org.junit.Assume; import org.junit.Rule; import org.junit.Test; import org.junit.Ignore; @@ -570,6 +571,29 @@ public void testBucketDeleteIfSnapshotExists() throws Exception { volume.deleteBucket(bucket2); } + @Test + public void testSnapDiffWithDirRename() throws Exception { + Assume.assumeTrue(bucketLayout.isFileSystemOptimized()); + String volume = "vol-" + counter.incrementAndGet(); + String bucket = "buck-" + counter.incrementAndGet(); + store.createVolume(volume); + OzoneVolume volume1 = store.getVolume(volume); + volume1.createBucket(bucket); + OzoneBucket bucket1 = volume1.getBucket(bucket); + bucket1.createDirectory("dir1"); + String snap1 = "snap1"; + createSnapshot(volume, bucket, snap1); + bucket1.renameKey("dir1", "dir1_rename"); + String snap2 = "snap2"; + createSnapshot(volume, bucket, snap2); + SnapshotDiffReportOzone diff = getSnapDiffReport(volume, bucket, + snap1, snap2); + Assertions.assertEquals(Arrays.asList( + SnapshotDiffReportOzone.getDiffReportEntry( + SnapshotDiffReport.DiffType.RENAME, "/dir1", "/dir1_rename")), + diff.getDiffList()); + } + @Test public void testSnapDiff() throws Exception { String volume = "vol-" + counter.incrementAndGet(); 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 ac70c44d567..d3bf0e3ec1e 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 @@ -890,15 +890,21 @@ void generateSnapshotDiffReport(final String jobKey, .getKeyTable(bucketLayout); Table tsKeyTable = toSnapshot.getMetadataManager() .getKeyTable(bucketLayout); + Table fsDirTable; + Table tsDirTable; final Optional> oldParentIds; final Optional> newParentIds; if (bucketLayout.isFileSystemOptimized()) { oldParentIds = Optional.of(new HashSet<>()); newParentIds = Optional.of(new HashSet<>()); + fsDirTable = fromSnapshot.getMetadataManager().getDirectoryTable(); + tsDirTable = toSnapshot.getMetadataManager().getDirectoryTable(); } else { oldParentIds = Optional.empty(); newParentIds = Optional.empty(); + fsDirTable = null; + tsDirTable = null; } final Optional> oldParentIdPathMap; @@ -924,11 +930,6 @@ void generateSnapshotDiffReport(final String jobKey, }, () -> { if (bucketLayout.isFileSystemOptimized()) { - Table fsDirTable = - fromSnapshot.getMetadataManager().getDirectoryTable(); - Table tsDirTable = - toSnapshot.getMetadataManager().getDirectoryTable(); - getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsDirTable, tsDirTable, fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff, tablePrefixes, objectIdToKeyNameMapForFromSnapshot, @@ -959,6 +960,8 @@ void generateSnapshotDiffReport(final String jobKey, long totalDiffEntries = generateDiffReport(jobId, fsKeyTable, tsKeyTable, + fsDirTable, + tsDirTable, objectIdToDiffObject, objectIdToKeyNameMapForFromSnapshot, objectIdToKeyNameMapForToSnapshot, @@ -1116,7 +1119,7 @@ void addToObjectIdMap(Table fsTable, oldObjIdToKeyMap.put(rawObjId, rawValue); SnapshotDiffObject diffObject = createDiffObjectWithOldName(fromObjectId.getObjectID(), key, - objectIdToDiffObject.get(rawObjId)); + objectIdToDiffObject.get(rawObjId), isDirectoryTable); objectIdToDiffObject.put(rawObjId, diffObject); oldParentIds.ifPresent(set -> set.add( fromObjectId.getParentObjectID())); @@ -1128,7 +1131,7 @@ void addToObjectIdMap(Table fsTable, newObjIdToKeyMap.put(rawObjId, rawValue); SnapshotDiffObject diffObject = createDiffObjectWithNewName(toObjectId.getObjectID(), key, - objectIdToDiffObject.get(rawObjId)); + objectIdToDiffObject.get(rawObjId), isDirectoryTable); objectIdToDiffObject.put(rawObjId, diffObject); newParentIds.ifPresent(set -> set.add(toObjectId .getParentObjectID())); @@ -1145,9 +1148,10 @@ void addToObjectIdMap(Table fsTable, } private SnapshotDiffObject createDiffObjectWithOldName( - long objectId, String oldName, SnapshotDiffObject diffObject) { + long objectId, String oldName, SnapshotDiffObject diffObject, + boolean isDirectory) { SnapshotDiffObjectBuilder builder = new SnapshotDiffObjectBuilder(objectId) - .withOldKeyName(oldName); + .withOldKeyName(oldName).setIsDirectory(isDirectory); if (diffObject != null) { builder.withNewKeyName(diffObject.getNewKeyName()); } @@ -1155,10 +1159,11 @@ private SnapshotDiffObject createDiffObjectWithOldName( } private SnapshotDiffObject createDiffObjectWithNewName( - long objectId, String newName, SnapshotDiffObject diffObject) { + long objectId, String newName, SnapshotDiffObject diffObject, + boolean isDirectory) { SnapshotDiffObjectBuilder builder = new SnapshotDiffObjectBuilder(objectId) - .withNewKeyName(newName); + .withNewKeyName(newName).setIsDirectory(isDirectory); if (diffObject != null) { builder.withOldKeyName(diffObject.getOldKeyName()); } @@ -1269,8 +1274,10 @@ private String resolveAbsolutePath(boolean isFSOBucket, @SuppressWarnings({"checkstyle:ParameterNumber", "checkstyle:MethodLength"}) long generateDiffReport( final String jobId, - final Table fsTable, - final Table tsTable, + final Table fsTable, + final Table tsTable, + final Table fsDirTable, + final Table tsDirTable, final PersistentMap objectIdToDiffObject, final PersistentMap oldObjIdToKeyMap, final PersistentMap newObjIdToKeyMap, @@ -1374,8 +1381,10 @@ long generateDiffReport( // Check if block location is same or not. If it is not same, // key must have been overridden as well. - if (!isBlockLocationSame(snapshotDiffObject.getOldKeyName(), - snapshotDiffObject.getNewKeyName(), fsTable, tsTable)) { + if (isObjectModified(snapshotDiffObject.getOldKeyName(), + snapshotDiffObject.getNewKeyName(), + snapshotDiffObject.isDirectory() ? fsDirTable : fsTable, + snapshotDiffObject.isDirectory() ? tsDirTable : tsTable)) { // Here, oldKey name is returned as modified. Modified key name is // based on base snapshot (from snapshot). renameDiffs.add(codecRegistry.asRawData( @@ -1439,6 +1448,34 @@ long generateDiffReport( } } + private boolean isObjectModified(String fromObjectName, String toObjectName, + final Table fromSnapshotTable, + final Table toSnapshotTable) + throws IOException { + Objects.requireNonNull(fromObjectName, "fromObjectName is null."); + Objects.requireNonNull(toObjectName, "toObjectName is null."); + + final WithObjectID fromObject = fromSnapshotTable.get(fromObjectName); + final WithObjectID toObject = toSnapshotTable.get(toObjectName); + if ((fromObject instanceof OmKeyInfo) && (toObject instanceof OmKeyInfo)) { + return !SnapshotDeletingService.isBlockLocationInfoSame( + (OmKeyInfo) fromObject, (OmKeyInfo) toObject); + } else if ((fromObject instanceof OmDirectoryInfo) + && (toObject instanceof OmDirectoryInfo)) { + return !areAclsSame((OmDirectoryInfo) fromObject, + (OmDirectoryInfo) toObject); + } else { + throw new IllegalStateException("fromObject or toObject is not of " + + "the expected type. fromObject type : " + + fromObject.getClass().getName() + "toObject type: " + + toObject.getClass().getName()); + } + } + private boolean areAclsSame(OmDirectoryInfo fromObject, + OmDirectoryInfo toObject) { + return fromObject.getAcls().equals(toObject.getAcls()); + } + private boolean isBlockLocationSame( String fromObjectName, String toObjectName, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffObject.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffObject.java index ea04b9d2c2c..c99757f3643 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffObject.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffObject.java @@ -41,6 +41,8 @@ public static Codec getCodec() { private String oldKeyName; private String newKeyName; + private boolean isDirectory; + // Default constructor for Jackson Serializer. public SnapshotDiffObject() { @@ -48,10 +50,12 @@ public SnapshotDiffObject() { public SnapshotDiffObject(long objectId, String oldKeyName, - String newKeyName) { + String newKeyName, + boolean isDirectory) { this.objectId = objectId; this.oldKeyName = oldKeyName; this.newKeyName = newKeyName; + this.isDirectory = isDirectory; } public long getObjectId() { @@ -81,6 +85,14 @@ public void setNewKeyName(String newKeyName) { this.newKeyName = newKeyName; } + public void setDirectory(boolean directory) { + isDirectory = directory; + } + + public boolean isDirectory() { + return isDirectory; + } + /** * Builder for SnapshotDiffObject. */ @@ -89,6 +101,8 @@ public static class SnapshotDiffObjectBuilder { private String oldKeyName; private String newKeyName; + private boolean isDirectory; + public SnapshotDiffObjectBuilder(long objectID) { this.objectId = objectID; } @@ -107,8 +121,14 @@ public SnapshotDiffObjectBuilder withNewKeyName(String keyName) { return this; } + public SnapshotDiffObjectBuilder setIsDirectory(boolean directory) { + isDirectory = directory; + return this; + } + public SnapshotDiffObject build() { - return new SnapshotDiffObject(objectId, oldKeyName, newKeyName); + return new SnapshotDiffObject(objectId, oldKeyName, newKeyName, + isDirectory); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java index 46f1773f7b6..410c9baa91b 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java @@ -721,9 +721,10 @@ public void testGenerateDiffReport() throws IOException { toSnapName); long totalDiffEntries = spy.generateDiffReport("jobId", - fromSnapTable, toSnapTable, objectIdToDiffObject, oldObjectIdKeyMap, - newObjectIdKeyMap, volumeName, bucketName, fromSnapName, toSnapName, - false, Optional.empty(), Optional.empty()); + fromSnapTable, toSnapTable, null, null, + objectIdToDiffObject, oldObjectIdKeyMap, newObjectIdKeyMap, volumeName, + bucketName, fromSnapName, toSnapName, false, Optional.empty(), + Optional.empty()); assertEquals(100, totalDiffEntries); SnapshotDiffJob snapshotDiffJob = new SnapshotDiffJob(0, "jobId", @@ -1101,6 +1102,8 @@ public void testGenerateDiffReportWhenThereInEntry() { long totalDiffEntries = snapshotDiffManager.generateDiffReport("jobId", keyInfoTable, keyInfoTable, + null, + null, objectIdToDiffObject, oldObjIdToKeyMap, newObjIdToKeyMap, @@ -1140,6 +1143,8 @@ public void testGenerateDiffReportFailure() throws IOException { () -> spy.generateDiffReport("jobId", keyInfoTable, keyInfoTable, + null, + null, objectIdToDiffObject, oldObjIdToKeyMap, newObjIdToKeyMap, @@ -1411,8 +1416,9 @@ public void testGetSnapshotDiffReportHappyCase() throws Exception { doNothing().when(spy).checkReportsIntegrity(any(), anyInt(), anyInt()); doReturn(10L).when(spy).generateDiffReport(anyString(), - any(), any(), any(), any(), any(), anyString(), anyString(), - anyString(), anyString(), anyBoolean(), any(), any()); + any(), any(), any(), any(), any(), any(), any(), + anyString(), anyString(), anyString(), anyString(), anyBoolean(), + any(), any()); doReturn(LEGACY).when(spy).getBucketLayout(VOLUME_NAME, BUCKET_NAME, omMetadataManager);