hemantk-12 commented on code in PR #8157:
URL: https://github.com/apache/ozone/pull/8157#discussion_r2021559024


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java:
##########
@@ -228,6 +231,52 @@ public void testCloseOnEviction() throws IOException,
     GenericTestUtils.waitFor(() -> {
       return logCapture.getOutput().contains(msg);
     }, 100, 30_000);
+
+    // clean up all snapshot
+    snapshotChainManager.deleteSnapshot(second);
+    snapshotChainManager.deleteSnapshot(first);
+    assertEquals(snapshotChainManager.getGlobalSnapshotChain().size(), 0);
+  }
+
+  @Test
+  public void testValidateSnapshotLimit() throws IOException {
+    Table<String, OmVolumeArgs> volumeTable = mock(Table.class);
+    Table<String, OmBucketInfo> bucketTable = mock(Table.class);
+    Table<String, SnapshotInfo> snapshotInfoTable = mock(Table.class);
+    HddsWhiteboxTestUtils.setInternalState(
+        om.getMetadataManager(), VOLUME_TABLE, volumeTable);
+    HddsWhiteboxTestUtils.setInternalState(
+        om.getMetadataManager(), BUCKET_TABLE, bucketTable);
+    HddsWhiteboxTestUtils.setInternalState(
+        om.getMetadataManager(), SNAPSHOT_INFO_TABLE, snapshotInfoTable);
+
+    om.getOmSnapshotManager().setFsSnapshotMaxLimit(2);

Review Comment:
   Is it possible to set it in `OzoneConfig` rather than this way?



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java:
##########
@@ -272,11 +273,16 @@ public void testEntryExist() throws Exception {
 
   private OMSnapshotDeleteRequest doPreExecute(
       OMRequest originalRequest) throws Exception {
+    return doPreExecute(originalRequest, getOzoneManager());
+  }
+
+  public static OMSnapshotDeleteRequest doPreExecute(

Review Comment:
   Why do we need this change?



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java:
##########
@@ -228,6 +231,52 @@ public void testCloseOnEviction() throws IOException,
     GenericTestUtils.waitFor(() -> {
       return logCapture.getOutput().contains(msg);
     }, 100, 30_000);
+
+    // clean up all snapshot
+    snapshotChainManager.deleteSnapshot(second);

Review Comment:
   Why do we need to remove these snapshots from the chain? Shouldn't 
`setupData` take care of it? If not, I'll suggest to move this code to 
`@BeforeEach` and `@AfterEach`. 
   



##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java:
##########
@@ -352,6 +402,9 @@ public void testGetSnapshotInfo() throws IOException {
     ome = assertThrows(OMException.class,
         () -> om.getOmSnapshotManager().getSnapshot(s2.getSnapshotId()));
     assertEquals(OMException.ResultCodes.FILE_NOT_FOUND, ome.getResult());
+
+    // clean up
+    snapshotChainManager.deleteSnapshot(s1);

Review Comment:
   Same as previous.



-- 
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]

Reply via email to