HDDS-8739. Snapdiff should return complete absolute path in Diff Entry #4823

swamirishi · 2023-06-02T15:55:20Z

What changes were proposed in this pull request?

https://docs.google.com/document/d/15eLimSG0Tre4qfe1D5PeSWMeZ9Si0uENUt1YcF21LeQ/edit?usp=sharing

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8739

How was this patch tested?

Unit tests & integration tests

hemantk-12

Thanks for working on the patch @swamirishi.

Left few comments. Couple of them are major.

Not sure if you are working on unit tests for FSODirectoryPathResolver. It would be great if you could add them in next revision of PR. Also look into CI workflow failure.

...zone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/FSODirectoryPathResolver.java

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

smengcl · 2023-06-17T02:16:04Z

@swamirishi

missing license: https://github.com/apache/ozone/actions/runs/5295547422/jobs/9586397706
checkstyle: https://github.com/apache/ozone/actions/runs/5295547422/jobs/9586397810

hemantk-12 · 2023-06-20T17:28:36Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java

+      return str;
+    }
+  }
+


nit: please remove this extra line.

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java

hemantk-12 · 2023-06-20T17:34:20Z

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java

-    Assert.assertTrue(diff2.getDiffList().contains(
-        SnapshotDiffReportOzone.getDiffReportEntry(
-            SnapshotDiffReportOzone.DiffType.DELETE, key1)));
+    Assert.assertEquals(diff2.getDiffList(),


It should be the other way Assert.assertEquals(expected, actual);

Suggested change

Assert.assertEquals(diff2.getDiffList(),

Assert.assertEquals(

Arrays.asList(SnapshotDiffReportOzone.getDiffReportEntry(

SnapshotDiffReport.DiffType.DELETE, "/" + key1),

SnapshotDiffReportOzone.getDiffReportEntry(

SnapshotDiffReport.DiffType.CREATE, "/" + key2)),

diff2.getDiffList());

Use OM_KEY_PREFIX instead of /.

Curious if it should be ./key.

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java

...zone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/FSODirectoryPathResolver.java

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java

# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java

hemantk-12

LGTM.

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java

# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java

* master: (96 commits) HDDS-8586 Recon. - API for Count of deletePending keys and amount of data mapped to such keys. (apache#4923) HDDS-8908. Intermittent failure in TestBlockDeletion#testBlockDeletion (apache#4958) HDDS-8910. Replace LockManager with striped lock in ContainerStateManager (apache#4962) HDDS-8917. Move protobuf conversion out of the lock in PipelineStateManagerImpl (apache#4965) HDDS-8825. Use apache/hadoop 3.3.5 docker image (apache#4963) HDDS-8906. Avoid stream when getting in-service healthy nodes (apache#4960) HDDS-8907. Store volume count when storage report is updated (apache#4957) HDDS-8905. PipelineManager metrics should not be synchronized (apache#4959) HDDS-8553. Improve scanner integration tests. (apache#4936) HDDS-8854. Avoid unnecessary DatanodeDetails creation for NodeStateManager lookup (apache#4925) HDDS-8315. [Snapshot] Added unit tests for SnapshotDiffManager (apache#4716) HDDS-7968. [Snapshot] Improve KeyDeletingService to reclaim eligible key blocks in snapshot's deletedTable (apache#4935) HDDS-8838. Update default datanode check empty containter on disk to false (apache#4937) HDDS-8763. Support RocksDB iterator with ByteBuffer. (apache#4942) HDDS-8543. FSO directory should reflect bucket/cluster default replication (apache#4947) HDDS-8898. Replication limit should not be less than reconstruction weight (apache#4954) HDDS-8739. Snapdiff should return complete absolute path in Diff Entry (apache#4823) HDDS-8908. Mark TestBlockDeletion#testBlockDeletion as flaky HDDS-8534. Support asynchronous service logging (apache#4663) HDDS-8879. Cleanup SecurityConfig and related class initialization (apache#4921) ...

* tmp-dir-refactor: (99 commits) HDDS-8586 Recon. - API for Count of deletePending keys and amount of data mapped to such keys. (apache#4923) Fix SCM HA finalization compat test HDDS-8908. Intermittent failure in TestBlockDeletion#testBlockDeletion (apache#4958) HDDS-8910. Replace LockManager with striped lock in ContainerStateManager (apache#4962) HDDS-8917. Move protobuf conversion out of the lock in PipelineStateManagerImpl (apache#4965) HDDS-8825. Use apache/hadoop 3.3.5 docker image (apache#4963) HDDS-8906. Avoid stream when getting in-service healthy nodes (apache#4960) HDDS-8907. Store volume count when storage report is updated (apache#4957) HDDS-8905. PipelineManager metrics should not be synchronized (apache#4959) HDDS-8553. Improve scanner integration tests. (apache#4936) HDDS-8854. Avoid unnecessary DatanodeDetails creation for NodeStateManager lookup (apache#4925) HDDS-8315. [Snapshot] Added unit tests for SnapshotDiffManager (apache#4716) HDDS-7968. [Snapshot] Improve KeyDeletingService to reclaim eligible key blocks in snapshot's deletedTable (apache#4935) HDDS-8838. Update default datanode check empty containter on disk to false (apache#4937) HDDS-8763. Support RocksDB iterator with ByteBuffer. (apache#4942) HDDS-8543. FSO directory should reflect bucket/cluster default replication (apache#4947) HDDS-8898. Replication limit should not be less than reconstruction weight (apache#4954) HDDS-8739. Snapdiff should return complete absolute path in Diff Entry (apache#4823) HDDS-8908. Mark TestBlockDeletion#testBlockDeletion as flaky HDDS-8534. Support asynchronous service logging (apache#4663) ...

HDDS-8739. Snapdiff should return complete absolute path in Diff Entry

7796d21

swamirishi marked this pull request as draft June 2, 2023 15:55

adoroszlai added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Jun 2, 2023

hemantk-12 reviewed Jun 13, 2023

View reviewed changes

swamirishi added 8 commits June 13, 2023 17:29

Merge remote-tracking branch 'apache/master' into HEAD

b5f2f77

# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

HDDS-8739. Fix Diff entry representation

4c53136

HDDS-8739. Fix compilation issues

eddb5a1

HDDS-8739. Fix bug

87792cc

HDDS-8739. fix bug to resolve abs path

971321f

HDDS-8739. Fix testcases

0feae64

Merge remote-tracking branch 'apache/master' into HEAD

007d6f6

HDDS-8739. Fix testcases

7756571

swamirishi marked this pull request as ready for review June 17, 2023 01:20

swamirishi added 5 commits June 17, 2023 11:49

HDDS-8739. Fix checkstyle

ccea92b

HDDS-8739. Address review comments

6297ed5

HDDS-8739: Fix failures

c3ec1a7

HDDS-8739: Fix failures

3f6510f

HDDS-8739: Fix failures

ebfb66b

hemantk-12 reviewed Jun 20, 2023

View reviewed changes

HDDS-8739: Fix failures

227f6c1

hemantk-12 reviewed Jun 20, 2023

View reviewed changes

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java Outdated Show resolved Hide resolved

swamirishi added 6 commits June 20, 2023 16:26

HDDS-8739: Address review comments

981cb79

HDDS-8739: Fix failures

3bc5467

HDDS-8739: Address review comments

9d35447

Merge remote-tracking branch 'apache/master' into HEAD

258a36c

# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java

HDDS-8739: Address review comments

d318ec2

HDDS-8739: Address review comments

f48d2a1

hemantk-12 reviewed Jun 21, 2023

View reviewed changes

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java Show resolved Hide resolved

HDDS-8739: Fix checkstyle

c69987e

swamirishi added 3 commits June 21, 2023 20:10

Merge remote-tracking branch 'apache/master' into HEAD

f5b299a

# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java

HDDS-8739: Resolve merge conflicts

d224462

HDDS-8739: Resolve test failure

d03890d

prashantpogde merged commit f4b6bb1 into apache:master Jun 22, 2023

-    Assert.assertEquals(diff2.getDiffList(),
+    Assert.assertEquals(
+        Arrays.asList(SnapshotDiffReportOzone.getDiffReportEntry(
+            SnapshotDiffReport.DiffType.DELETE, "/" + key1),
+            SnapshotDiffReportOzone.getDiffReportEntry(
+                SnapshotDiffReport.DiffType.CREATE, "/" + key2)),
+        diff2.getDiffList());

HDDS-8739. Snapdiff should return complete absolute path in Diff Entry #4823

HDDS-8739. Snapdiff should return complete absolute path in Diff Entry #4823

Uh oh!

Conversation

swamirishi commented Jun 2, 2023

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

hemantk-12 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smengcl commented Jun 17, 2023

Uh oh!

hemantk-12 Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hemantk-12 Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hemantk-12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hemantk-12 left a comment •

edited

Loading