SaketaChalamchala commented on code in PR #10750:
URL: https://github.com/apache/ozone/pull/10750#discussion_r3754152661
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1354,6 +1356,104 @@ private String resolveBucketRelativePath(boolean
isFSOBucket,
.substring(1);
}
+ /**
+ * Returns whether any ancestor of the given parent is a deleted directory.
+ * The walk stops at renamed directories because their descendant deletes are
+ * not subsumed by a deleted ancestor above them.
+ *
+ * @param objectIdToParentId from-snapshot directory parent graph keyed by
stable
+ * objectId; must not be the to-snapshot graph
+ * @param renamedDirectoryIds objectIds of directories renamed in the diff
(from
+ * snapshot objectIds, not destination paths)
+ */
+ @VisibleForTesting
+ boolean hasDeletedAncestor(long parentObjectId, Set<Long>
deletedDirectoryIds,
+ Set<Long> renamedDirectoryIds, Map<Long, Long> objectIdToParentId,
+ long bucketObjectId, Map<Long, Boolean> ancestorMemo) {
+ Objects.requireNonNull(objectIdToParentId, "objectIdToParentId must not be
null");
+ Objects.requireNonNull(renamedDirectoryIds, "renamedDirectoryIds must not
be null");
+
+ if (parentObjectId == bucketObjectId) {
+ return false;
+ }
+ Boolean cached = ancestorMemo.get(parentObjectId);
+ if (cached != null) {
+ return cached;
+ }
+
+ List<Long> path = new ArrayList<>();
+ long current = parentObjectId;
+ boolean result;
+ while (true) {
+ if (current == bucketObjectId) {
+ result = false;
+ break;
+ }
+ cached = ancestorMemo.get(current);
+ if (cached != null) {
+ result = cached;
+ break;
+ }
+ if (renamedDirectoryIds.contains(current)) {
+ result = false;
+ ancestorMemo.put(current, false);
+ break;
+ }
+ if (deletedDirectoryIds.contains(current)) {
+ result = true;
+ ancestorMemo.put(current, true);
+ break;
+ }
+ Long nextParent = objectIdToParentId.get(current);
+ if (nextParent == null) {
+ result = false;
+ ancestorMemo.put(current, false);
+ break;
+ }
Review Comment:
If `nextParent` is null then, it is likely that the parent has already been
deleted in `fromSnapshot` and the child delete appears in the diff due to deep
cleaning. In any case, the intention here was to skip entries which cannot be
reached from the live directory graph(this is same behavior in baseline
snapdiff).
The return value here should be `true`. Will make that change and add a test
case for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
