HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (#4065) #4861

smengcl · 2023-06-08T19:34:33Z

(cherry picked from commit 9f84c77)

What changes were proposed in this pull request?

The original PR (#4065) targeted the snapshot feature branch. Unfortunately, I merged that PR after feature branch's merge to master (didn't check the PR target branch at the time of merge). So it is missing from master branch. Thanks @hemantk-12 for catching this.

And because this backport requires more than just simple conflict resolution. I have posted this as a new PR for formality.

Cherry-picked 9f84c77 to master branch
Resolved two minor conflicts
Fixed usage in listStatusBucketSnapshot and a test because of the API change: d6d23cb
getNextListOfSnapshots() now throws IOException directly, rather than wrapping it in another RuntimeException: 6afa196

Q for @adoroszlai : do you think I should keep the original PR number in the title as well? Or just the current one.

What is the link to the Apache JIRA

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

How was this patch tested?

Same as HDDS-7507. [Snapshot] Implement List Snapshot API Pagination #4065 .

…4065) (cherry picked from commit 9f84c77) Conflicts: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

prashantpogde · 2023-06-08T20:11:19Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java

      String snapshotKey = entry.getKey().getCacheKey();
      SnapshotInfo snapshotInfo = entry.getValue().getCacheValue();
-      if (snapshotInfo != null && snapshotKey.startsWith(prefix)) {
+      if (snapshotInfo != null && snapshotKey.startsWith(prefix) &&


should we check that snapshot is active and not being deleted ?

prashantpogde · 2023-06-08T20:11:57Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java

+            snapshotinfo.getKey().compareTo(previous) == 0) {
+          continue;
+        }
+        if (snapshotinfo != null && snapshotinfo.getKey().startsWith(prefix))  {


should we check snapshot being active ?

prashantpogde

Actually on a second thought its OK if we don't skip the snapshots that are being deleted. We are returning the snapshot status as well. The caller can take the decision based on snapshot status while going through the list.

swamirishi

Minor nit picking comments otherwise LGTM

swamirishi · 2023-06-08T20:23:20Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java

+    String seek;
+    if (StringUtil.isNotBlank(prevSnapshot)) {
+      // Seek to the specified snapshot.
+      seek = getOzoneKey(volumeName, bucketName, prevSnapshot);


We can probably append the '\0' (char with ascii value 0 to ignore the previous value. So we wouldn't really have to check the previous it would be ignored automatically

Something like if previous snapshot is snap1 then we can append '\0' to snap1 to make it 'snap1\0' which would be the next string in lexicographical ordering.

swamirishi · 2023-06-08T20:24:43Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java

-      TreeMap snapshotInfoMap, String prefix) {
+  private int appendSnapshotFromCacheToMap(
+      TreeMap snapshotInfoMap, String prefix,
+      String previous, int maxListResult) {


previous wouldn't be required in the case we seek to the value after previous.

swamirishi · 2023-06-08T20:26:24Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java

        snapshotinfo = snapshotIter.next();
-        if (snapshotinfo != null && snapshotinfo.getKey().startsWith(prefix)) {
+        if (snapshotinfo != null &&
+            snapshotinfo.getKey().compareTo(previous) == 0) {


this check can be removed

…apping it inside another RuntimeException.

smengcl · 2023-06-09T04:47:08Z

@prashantpogde @swamirishi Since this is a backport of a previous PR with only absolutely necessary changes to make it function. I will not incorporate further non-critical changes into this. Pls feel free to open a new PR to do any refactoring.

CI has passed. Will merge this in a bit.

* master: (73 commits) HDDS-8587. Test that CertificateClient can store multiple rootCA certificates (apache#4852) HDDS-8801. ReplicationManager: Add metric to count how often replication is throttled (apache#4864) HDDS-8477. Unit test for Snapdiff using tombstone entries (apache#4678) HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (apache#4065) (apache#4861) HDDS-8373. Document that setquota doesn't accept decimals (apache#4856) HDDS-8779. Recon - Expose flag for enable/disable of heatmap. (apache#4845) HDDS-8677. Ozone admin OM CLI command for block tokens (apache#4760) HDDS-8164. Authorize secret key APIs (apache#4597) HDDS-7945. Integrate secret keys to SCM snapshot (apache#4549) HDDS-8003. E2E integration test cases for block tokens (apache#4547) HDDS-7831. Use symmetric secret key to sign and verify token (apache#4417) HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345) HDDS-7734. Implement symmetric SecretKeys lifescycle management in SCM (apache#4194) HDDS-8679. Add dedicated, configurable thread pool for OM gRPC server (apache#4771) HDDS-8790. Split EC acceptance tests (apache#4855) HDDS-8714. TestScmHAFinalization: mark testFinalizationWithRestart as flaky, enable other test cases HDDS-8787. Reduce ozone sh calls in robot tests (apache#4854) HDDS-8774. Log allocation stack trace for leaked CodecBuffer (apache#4840) HDDS-8729. Add metric for count of blocks pending deletion on datanode (apache#4800) HDDS-8780. Leak of ManagedChannel in HASecurityUtils (apache#4850) ...

* tmp-dir-refactor: (73 commits) HDDS-8587. Test that CertificateClient can store multiple rootCA certificates (apache#4852) HDDS-8801. ReplicationManager: Add metric to count how often replication is throttled (apache#4864) HDDS-8477. Unit test for Snapdiff using tombstone entries (apache#4678) HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (apache#4065) (apache#4861) HDDS-8373. Document that setquota doesn't accept decimals (apache#4856) HDDS-8779. Recon - Expose flag for enable/disable of heatmap. (apache#4845) HDDS-8677. Ozone admin OM CLI command for block tokens (apache#4760) HDDS-8164. Authorize secret key APIs (apache#4597) HDDS-7945. Integrate secret keys to SCM snapshot (apache#4549) HDDS-8003. E2E integration test cases for block tokens (apache#4547) HDDS-7831. Use symmetric secret key to sign and verify token (apache#4417) HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345) HDDS-7734. Implement symmetric SecretKeys lifescycle management in SCM (apache#4194) HDDS-8679. Add dedicated, configurable thread pool for OM gRPC server (apache#4771) HDDS-8790. Split EC acceptance tests (apache#4855) HDDS-8714. TestScmHAFinalization: mark testFinalizationWithRestart as flaky, enable other test cases HDDS-8787. Reduce ozone sh calls in robot tests (apache#4854) HDDS-8774. Log allocation stack trace for leaked CodecBuffer (apache#4840) HDDS-8729. Add metric for count of blocks pending deletion on datanode (apache#4800) HDDS-8780. Leak of ManagedChannel in HASecurityUtils (apache#4850) ...

chungen0126 and others added 2 commits June 8, 2023 12:07

Fix breakages on master as a result of the API change.

d6d23cb

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

smengcl requested a review from prashantpogde June 8, 2023 19:37

prashantpogde reviewed Jun 8, 2023

View reviewed changes

prashantpogde approved these changes Jun 8, 2023

View reviewed changes

swamirishi reviewed Jun 8, 2023

View reviewed changes

getNextListOfSnapshots() should throw IOException directly without wr…

6afa196

…apping it inside another RuntimeException.

smengcl merged commit a94d2fc into apache:master Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (#4065) #4861

HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (#4065) #4861

Uh oh!

smengcl commented Jun 8, 2023 •

edited

Loading

Uh oh!

prashantpogde Jun 8, 2023

Uh oh!

prashantpogde Jun 8, 2023

Uh oh!

prashantpogde left a comment

Uh oh!

swamirishi left a comment

Uh oh!

swamirishi Jun 8, 2023

Uh oh!

swamirishi Jun 8, 2023

Uh oh!

swamirishi Jun 8, 2023

Uh oh!

swamirishi Jun 8, 2023

Uh oh!

smengcl commented Jun 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (#4065) #4861

HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (#4065) #4861

Uh oh!

Conversation

smengcl commented Jun 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

prashantpogde Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

prashantpogde Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

prashantpogde left a comment

Choose a reason for hiding this comment

Uh oh!

swamirishi left a comment

Choose a reason for hiding this comment

Uh oh!

swamirishi Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

swamirishi Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

swamirishi Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

swamirishi Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

smengcl commented Jun 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smengcl commented Jun 8, 2023 •

edited

Loading