You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "hemantk-12 (via GitHub)" <gi...@apache.org> on 2023/11/15 07:11:47 UTC

[PR] Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

hemantk-12 opened a new pull request, #5605:
URL: https://github.com/apache/ozone/pull/5605

   ## What changes were proposed in this pull request?
   There are two tasks asked in [HDDS-8201](https://issues.apache.org/jira/browse/HDDS-8201). In #5518, TestOmSnapshotFileSystem tests were migrated to JUnit-5. This change is to address the second task and make some remaining tasks Snapshot delete aware. 
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-8201
   
   ## How was this patch tested?
   Workflow run: https://github.com/hemantk-12/ozone/actions/runs/6872953402/job/18692239021
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-8201. Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5605:
URL: https://github.com/apache/ozone/pull/5605#issuecomment-1819549891

   Thank for the review @aswinshakil 


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-8201. Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on code in PR #5605:
URL: https://github.com/apache/ozone/pull/5605#discussion_r1394819564


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java:
##########
@@ -655,7 +698,6 @@ public void testListStatusOnRoot() throws Exception {
   // based on TestOzoneFileSystem:testListStatusOnLargeDirectory
   public void testListStatusOnLargeDirectory() throws Exception {
     Path root = new Path("/");
-    deleteRootDir(); // cleanup

Review Comment:
   Do we not need the cleanup anymore? I believe this is to clean up from the previous test run right?



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java:
##########
@@ -291,27 +269,46 @@ public void testListKeysAtDifferentLevels() throws Exception {
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Intermediate level keyPrefix - 3rd level
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b2/d1", null);
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b2/d1", null);
     expectedKeys = new LinkedList<>();
     expectedKeys.add("a/b2/d1/");
     expectedKeys.add("a/b2/d1/d11.tx");
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Boundary of a level
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b2/d2",
-            keyPrefix + "a/b2/d2/d21.tx");
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b2/d2",
+        keyPrefix + "a/b2/d2/d21.tx");
     expectedKeys = new LinkedList<>();
     expectedKeys.add("a/b2/d2/d22.tx");
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Boundary case - last node in the depth-first-traversal
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b3/e3",
-            keyPrefix + "a/b3/e3/e31.tx");
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b3/e3",
+        keyPrefix + "a/b3/e3/e31.tx");
     expectedKeys = new LinkedList<>();
     checkKeyList(ozoneKeyIterator, expectedKeys);
+
+    deleteSnapshot(snapshotName);
+    String expectedMessage = String.format("Unable to load snapshot. " +
+            "Snapshot with table key '/%s/%s/%s' is no longer active",
+        volumeName, bucketName, snapshotName);
+    OMException exception = assertThrows(OMException.class,
+        () -> ozoneBucket.listKeys(keyPrefix + "a/", null));
+    assertEquals(expectedMessage, exception.getMessage());
+    exception = assertThrows(OMException.class,
+        () -> ozoneBucket.listKeys(keyPrefix + "a///b2///", null));

Review Comment:
   Should this be just "a/b2/"?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-8201. Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5605:
URL: https://github.com/apache/ozone/pull/5605#discussion_r1394841153


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java:
##########
@@ -655,7 +698,6 @@ public void testListStatusOnRoot() throws Exception {
   // based on TestOzoneFileSystem:testListStatusOnLargeDirectory
   public void testListStatusOnLargeDirectory() throws Exception {
     Path root = new Path("/");
-    deleteRootDir(); // cleanup

Review Comment:
   "this is to clean up from the previous test run right" is my understanding as well.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-8201. Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5605:
URL: https://github.com/apache/ozone/pull/5605#discussion_r1394844516


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java:
##########
@@ -291,27 +269,46 @@ public void testListKeysAtDifferentLevels() throws Exception {
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Intermediate level keyPrefix - 3rd level
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b2/d1", null);
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b2/d1", null);
     expectedKeys = new LinkedList<>();
     expectedKeys.add("a/b2/d1/");
     expectedKeys.add("a/b2/d1/d11.tx");
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Boundary of a level
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b2/d2",
-            keyPrefix + "a/b2/d2/d21.tx");
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b2/d2",
+        keyPrefix + "a/b2/d2/d21.tx");
     expectedKeys = new LinkedList<>();
     expectedKeys.add("a/b2/d2/d22.tx");
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Boundary case - last node in the depth-first-traversal
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b3/e3",
-            keyPrefix + "a/b3/e3/e31.tx");
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b3/e3",
+        keyPrefix + "a/b3/e3/e31.tx");
     expectedKeys = new LinkedList<>();
     checkKeyList(ozoneKeyIterator, expectedKeys);
+
+    deleteSnapshot(snapshotName);
+    String expectedMessage = String.format("Unable to load snapshot. " +
+            "Snapshot with table key '/%s/%s/%s' is no longer active",
+        volumeName, bucketName, snapshotName);
+    OMException exception = assertThrows(OMException.class,
+        () -> ozoneBucket.listKeys(keyPrefix + "a/", null));
+    assertEquals(expectedMessage, exception.getMessage());
+    exception = assertThrows(OMException.class,
+        () -> ozoneBucket.listKeys(keyPrefix + "a///b2///", null));

Review Comment:
   I kept it as it is here: https://github.com/apache/ozone/blob/master/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java#L280



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-8201. Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 merged PR #5605:
URL: https://github.com/apache/ozone/pull/5605


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-8201. Added snapshot delete in all tests of TestOmSnapshotFileSystem [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5605:
URL: https://github.com/apache/ozone/pull/5605#discussion_r1394972204


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java:
##########
@@ -291,27 +269,46 @@ public void testListKeysAtDifferentLevels() throws Exception {
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Intermediate level keyPrefix - 3rd level
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b2/d1", null);
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b2/d1", null);
     expectedKeys = new LinkedList<>();
     expectedKeys.add("a/b2/d1/");
     expectedKeys.add("a/b2/d1/d11.tx");
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Boundary of a level
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b2/d2",
-            keyPrefix + "a/b2/d2/d21.tx");
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b2/d2",
+        keyPrefix + "a/b2/d2/d21.tx");
     expectedKeys = new LinkedList<>();
     expectedKeys.add("a/b2/d2/d22.tx");
     checkKeyList(ozoneKeyIterator, expectedKeys);
 
     // Boundary case - last node in the depth-first-traversal
-    ozoneKeyIterator =
-        ozoneBucket.listKeys(keyPrefix + "a/b3/e3",
-            keyPrefix + "a/b3/e3/e31.tx");
+    ozoneKeyIterator = ozoneBucket.listKeys(keyPrefix + "a/b3/e3",
+        keyPrefix + "a/b3/e3/e31.tx");
     expectedKeys = new LinkedList<>();
     checkKeyList(ozoneKeyIterator, expectedKeys);
+
+    deleteSnapshot(snapshotName);
+    String expectedMessage = String.format("Unable to load snapshot. " +
+            "Snapshot with table key '/%s/%s/%s' is no longer active",
+        volumeName, bucketName, snapshotName);
+    OMException exception = assertThrows(OMException.class,
+        () -> ozoneBucket.listKeys(keyPrefix + "a/", null));
+    assertEquals(expectedMessage, exception.getMessage());
+    exception = assertThrows(OMException.class,
+        () -> ozoneBucket.listKeys(keyPrefix + "a///b2///", null));

Review Comment:
   Changed it to "a/b2/".



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org