[GitHub] [ozone] errose28 commented on a change in pull request #2554: HDDS-5644. Speed up decommission tests using a background Mini Cluster provider

[GitBox <gi...@apache.org>]

errose28 commented on a change in pull request #2554:
URL: https://github.com/apache/ozone/pull/2554#discussion_r695052521



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDecommissionAndMaintenance.java
##########
@@ -98,8 +104,123 @@
 
   private ContainerOperationClient scmClient;
 
-  @Before
-  public void setUp() throws Exception {
+  private static MiniClusterProvider clusterProvider;
+
+  /**
+   * Class to create mini-clusters in the background.
+   */
+  public static class MiniClusterProvider {
+
+    private int preCreatedLimit = 1;
+
+    private final OzoneConfiguration conf;
+    private final MiniOzoneCluster.Builder builder;
+    private boolean shouldRun = true;
+    private boolean shouldReap = true;

Review comment:
       I think shouldRun and shouldReap should be volatile to make sure the create and reap threads see the latest values on each iteration. Do we actually need these variables or can we just interrupt the threads to stop the loops?

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDecommissionAndMaintenance.java
##########
@@ -98,8 +104,123 @@
 
   private ContainerOperationClient scmClient;
 
-  @Before
-  public void setUp() throws Exception {
+  private static MiniClusterProvider clusterProvider;
+
+  /**
+   * Class to create mini-clusters in the background.
+   */
+  public static class MiniClusterProvider {
+
+    private int preCreatedLimit = 1;
+
+    private final OzoneConfiguration conf;
+    private final MiniOzoneCluster.Builder builder;
+    private boolean shouldRun = true;
+    private boolean shouldReap = true;
+    private Thread createThread;
+    private Thread reapThread;
+
+    private final BlockingQueue<MiniOzoneCluster> clusters
+        = new ArrayBlockingQueue<>(preCreatedLimit);
+    private final BlockingQueue<MiniOzoneCluster> expiredClusters
+        = new ArrayBlockingQueue<>(1024);
+
+
+    public MiniClusterProvider(OzoneConfiguration conf,
+        MiniOzoneCluster.Builder builder) {
+      this.conf = conf;
+      this.builder = builder;
+      createThread = createClusters();
+      reapThread = reapClusters();
+    }
+
+    public MiniOzoneCluster provide() throws InterruptedException {
+      return clusters.poll(100, SECONDS);
+    }
+
+    public void destroy(MiniOzoneCluster c) throws InterruptedException {
+      expiredClusters.put(c);
+    }
+
+    public void shutdown() throws InterruptedException {
+      shouldRun = false;
+      createThread.interrupt();
+      createThread.join();
+      destroyRemainingClusters();
+      shouldReap = false;
+      reapThread.join();
+    }
+
+    private Thread reapClusters() {
+      Thread t = new Thread(() -> {
+        while(shouldReap || !expiredClusters.isEmpty()) {
+          try {
+            MiniOzoneCluster c = expiredClusters.take();
+            c.shutdown();
+          } catch (InterruptedException e) {
+            break;
+          }
+        }
+      });
+      t.start();
+      return t;
+    }
+
+    private Thread createClusters() {
+      Thread t = new Thread(() -> {
+        while (shouldRun && !Thread.interrupted()) {
+          MiniOzoneCluster cluster = null;
+          try {
+            builder.setClusterId(UUID.randomUUID().toString());
+
+            OzoneConfiguration newConf = new OzoneConfiguration(conf);
+            List<Integer> portList = getFreePortList(4);
+            newConf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY,
+                "127.0.0.1:" + portList.get(0));
+            newConf.set(OMConfigKeys.OZONE_OM_HTTP_ADDRESS_KEY,
+                "127.0.0.1:" + portList.get(1));
+            newConf.set(OMConfigKeys.OZONE_OM_HTTPS_ADDRESS_KEY,
+                "127.0.0.1:" + portList.get(2));
+            newConf.setInt(OMConfigKeys.OZONE_OM_RATIS_PORT_KEY,
+                portList.get(3));
+            builder.setConf(newConf);
+
+            cluster = builder.build();
+            cluster.waitForClusterToBeReady();
+            clusters.put(cluster);

Review comment:
       It looks like for every test suite this will build an extra cluster that never gets used. Since all tests know how many clusters they need, perhaps the number of clusters to make can be configured and then either
   1. The create thread keeps creating clusters and queueing them until that number is reached.
       - Faster, but could result in lots of clusters being up at once, which might be bad for performance.
    or
    2. We only leave one cluster in the queue at a time, but the create thread checks if it has made the required number of clusters before making another one.




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