You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2023/10/27 09:10:51 UTC

[PR] HDDS-9520. Merge some TestOzoneManagerHA subclasses [ozone]

adoroszlai opened a new pull request, #5500:
URL: https://github.com/apache/ozone/pull/5500

   ## What changes were proposed in this pull request?
   
   Multiple `TestOzoneManagerHA` subclasses test OM HA setup.  Each subclass needs to start/stop its own cluster, which takes about 30 seconds.  Some of the classes could be merged to reduce the number of clusters.
   
   https://issues.apache.org/jira/browse/HDDS-9520
   
   ## How was this patch tested?
   
   Before:
   
   ```
   [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.235 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAWithACL
   [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 62.146 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAKeyDeletion
   [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.204 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAWithFailover
   [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 39.471 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAMetrics
   [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 154.38 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAWithData
   [INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 155.955 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAMetadataOnly
   ```
   
   After:
   
   ```
   [INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 210.821 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAWithData
   [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 164.196 s - in org.apache.hadoop.ozone.om.TestOzoneManagerHAMetadataOnly
   ```


-- 
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-9520. Merge some TestOzoneManagerHA subclasses [ozone]

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

   Thanks @hemantk-12 for the reviews.


-- 
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-9520. Merge some TestOzoneManagerHA subclasses [ozone]

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

   Thanks @hemantk-12 for the review.
   
   I've updated the patch: instead of grouping tests by "metadata" and "data", now they are split based on whether they stop/restart OMs or not.  This way we can avoid `@BeforeEach`/`@AfterEach` steps for one of the test classes and split the ACL test method without performance impact.


-- 
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-9520. Merge some TestOzoneManagerHA subclasses [ozone]

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


-- 
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-9520. Merge some TestOzoneManagerHA subclasses [ozone]

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


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithData.java:
##########
@@ -551,4 +571,577 @@ private String createMultipartUploadPartKey(OzoneBucket ozoneBucket,
 
     return ozoneOutputStream.getCommitUploadPartInfo().getPartName();
   }
+
+  @Test
+  public void testConf() {
+    final RaftProperties p = getCluster()
+        .getOzoneManager()
+        .getOmRatisServer()
+        .getServer()
+        .getProperties();
+    final TimeDuration t = RaftServerConfigKeys.Log.Appender.waitTimeMin(p);
+    Assertions.assertEquals(TimeDuration.ZERO, t,
+        RaftServerConfigKeys.Log.Appender.WAIT_TIME_MIN_KEY);
+  }
+
+  @Test
+  public void testKeyDeletion() throws Exception {
+    OzoneBucket ozoneBucket = setupBucket();
+    String data = "random data";
+    String keyName1 = "dir/file1";
+    String keyName2 = "dir/file2";
+    String keyName3 = "dir/file3";
+    String keyName4 = "dir/file4";
+    List<String> keyList1 = new ArrayList<>();
+    keyList1.add(keyName2);
+    keyList1.add(keyName3);
+
+    testCreateFile(ozoneBucket, keyName1, data, true, false);
+    testCreateFile(ozoneBucket, keyName2, data, true, false);
+    testCreateFile(ozoneBucket, keyName3, data, true, false);
+    testCreateFile(ozoneBucket, keyName4, data, true, false);
+
+    ozoneBucket.deleteKey(keyName1);
+    ozoneBucket.deleteKey(keyName2);
+    ozoneBucket.deleteKey(keyName3);
+    ozoneBucket.deleteKey(keyName4);
+
+    // Now check delete table has entries been removed.
+
+    OzoneManager ozoneManager = getCluster().getOMLeader();
+
+    KeyDeletingService keyDeletingService =
+        (KeyDeletingService) ozoneManager.getKeyManager().getDeletingService();
+
+    // Check on leader OM Count.
+    GenericTestUtils.waitFor(() ->
+        keyDeletingService.getRunCount().get() >= 2, 10000, 120000);
+    GenericTestUtils.waitFor(() ->
+        keyDeletingService.getDeletedKeyCount().get() == 4, 10000, 120000);
+
+    // Check delete table is empty or not on all OMs.
+    getCluster().getOzoneManagersList().forEach((om) -> {
+      try {
+        GenericTestUtils.waitFor(() -> {
+          Table<String, RepeatedOmKeyInfo> deletedTable =
+              om.getMetadataManager().getDeletedTable();
+          try (TableIterator<?, ?> iterator = deletedTable.iterator()) {
+            return !iterator.hasNext();
+          } catch (Exception ex) {
+            return false;
+          }
+        },
+            10000, 120000);
+      } catch (Exception ex) {
+        Assertions.fail("TestOzoneManagerHAKeyDeletion failed");
+      }
+    });
+  }
+  /**
+   * 1. Stop one of the OM
+   * 2. make a call to OM, this will make failover attempts to find new node.
+   * a) if LE finishes but leader not ready, it retries to same node
+   * b) if LE not done, it will failover to new node and check
+   * 3. Try failover to same OM explicitly.
+   * Now #3 should wait additional waitBetweenRetries time.
+   * LE: Leader Election.
+   */
+  @Test
+  @Order(Integer.MAX_VALUE)
+  void testIncrementalWaitTimeWithSameNodeFailover() throws Exception {
+    long waitBetweenRetries = getConf().getLong(
+        OzoneConfigKeys.OZONE_CLIENT_WAIT_BETWEEN_RETRIES_MILLIS_KEY,
+        OzoneConfigKeys.OZONE_CLIENT_WAIT_BETWEEN_RETRIES_MILLIS_DEFAULT);
+    HadoopRpcOMFailoverProxyProvider omFailoverProxyProvider =
+        OmFailoverProxyUtil
+            .getFailoverProxyProvider(getObjectStore().getClientProxy());
+
+    // The omFailoverProxyProvider will point to the current leader OM node.
+    String leaderOMNodeId = omFailoverProxyProvider.getCurrentProxyOMNodeId();
+
+    getCluster().stopOzoneManager(leaderOMNodeId);
+    Thread.sleep(NODE_FAILURE_TIMEOUT * 4);
+    createKeyTest(true); // failover should happen to new node
+
+    long numTimesTriedToSameNode = omFailoverProxyProvider.getWaitTime()
+        / waitBetweenRetries;
+    omFailoverProxyProvider.setNextOmProxy(omFailoverProxyProvider.
+        getCurrentProxyOMNodeId());
+    Assertions.assertEquals((numTimesTriedToSameNode + 1) * waitBetweenRetries,
+        omFailoverProxyProvider.getWaitTime());
+  }
+
+  @Test
+  void testRunAllTests() throws Exception {

Review Comment:
   nit: I think individual tests can run independently.



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