Revert opensearch-project#15258 to figure out a better approach to fi…

…x the issue. (opensearch-project#16377) Signed-off-by: zane-neo <zaniu@amazon.com>
dk2k · Oct 21, 2024 · f4f37d5 · f4f37d5
1 parent e98974d
commit f4f37d5
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 62 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -84,7 +84,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix multi-search with template doesn't return status code ([#16265](https://github.com/opensearch-project/OpenSearch/pull/16265))
 - [Streaming Indexing] Fix intermittent 'The bulk request must be terminated by a newline [\n]' failures [#16337](https://github.com/opensearch-project/OpenSearch/pull/16337))
 - Fix wrong default value when setting `index.number_of_routing_shards` to null on index creation ([#16331](https://github.com/opensearch-project/OpenSearch/pull/16331))
-- Fix disk usage exceeds threshold cluster can't spin up issue ([#15258](https://github.com/opensearch-project/OpenSearch/pull/15258)))
 - [Workload Management] Make query groups persistent across process restarts [#16370](https://github.com/opensearch-project/OpenSearch/pull/16370)
 
 - Fix inefficient Stream API call chains ending with count() ([#15386](https://github.com/opensearch-project/OpenSearch/pull/15386))

diff --git a/distribution/tools/keystore-cli/src/test/java/org/opensearch/bootstrap/BootstrapTests.java b/distribution/tools/keystore-cli/src/test/java/org/opensearch/bootstrap/BootstrapTests.java
@@ -31,15 +31,13 @@
 
 package org.opensearch.bootstrap;
 
-import org.opensearch.common.logging.LogConfigurator;
 import org.opensearch.common.settings.KeyStoreCommandTestCase;
 import org.opensearch.common.settings.KeyStoreWrapper;
 import org.opensearch.common.settings.SecureSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.io.IOUtils;
 import org.opensearch.core.common.settings.SecureString;
 import org.opensearch.env.Environment;
-import org.opensearch.node.Node;
 import org.opensearch.test.OpenSearchTestCase;
 import org.junit.After;
 import org.junit.Before;
@@ -53,14 +51,8 @@
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.hamcrest.Matchers.equalTo;
-import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 
 public class BootstrapTests extends OpenSearchTestCase {
     Environment env;
@@ -139,38 +131,4 @@ private void assertPassphraseRead(String source, String expected) {
         }
     }
 
-    public void testInitExecutionOrder() throws Exception {
-        AtomicInteger order = new AtomicInteger(0);
-        CountDownLatch countDownLatch = new CountDownLatch(1);
-        Thread mockThread = new Thread(() -> {
-            assertEquals(0, order.getAndIncrement());
-            countDownLatch.countDown();
-        });
-
-        Node mockNode = mock(Node.class);
-        doAnswer(invocation -> {
-            try {
-                boolean threadStarted = countDownLatch.await(1000, TimeUnit.MILLISECONDS);
-                assertTrue(
-                    "Waited for one second but the keepAliveThread isn't started, please check the execution order of"
-                        + "keepAliveThread.start and node.start",
-                    threadStarted
-                );
-            } catch (InterruptedException e) {
-                fail("Thread interrupted");
-            }
-            assertEquals(1, order.getAndIncrement());
-            return null;
-        }).when(mockNode).start();
-
-        LogConfigurator.registerErrorListener();
-        Bootstrap testBootstrap = new Bootstrap(mockThread, mockNode);
-        Bootstrap.setInstance(testBootstrap);
-
-        Bootstrap.startInstance(testBootstrap);
-
-        verify(mockNode).start();
-        assertEquals(2, order.get());
-    }
-
 }
diff --git a/server/src/main/java/org/opensearch/bootstrap/Bootstrap.java b/server/src/main/java/org/opensearch/bootstrap/Bootstrap.java
@@ -93,17 +93,6 @@ final class Bootstrap {
     private final Thread keepAliveThread;
     private final Spawner spawner = new Spawner();
 
-    // For testing purpose
-    static void setInstance(Bootstrap bootstrap) {
-        INSTANCE = bootstrap;
-    }
-
-    // For testing purpose
-    Bootstrap(Thread keepAliveThread, Node node) {
-        this.keepAliveThread = keepAliveThread;
-        this.node = node;
-    }
-
     /** creates a new instance */
     Bootstrap() {
         keepAliveThread = new Thread(new Runnable() {
@@ -347,10 +336,8 @@ private static Environment createEnvironment(
     }
 
     private void start() throws NodeValidationException {
-        // keepAliveThread should start first than node to ensure the cluster can spin up successfully in edge cases:
-        // https://github.com/opensearch-project/OpenSearch/issues/14791
-        keepAliveThread.start();
         node.start();
+        keepAliveThread.start();
     }
 
     static void stop() throws IOException {
@@ -423,7 +410,7 @@ static void init(final boolean foreground, final Path pidFile, final boolean qui
                 throw new BootstrapException(e);
             }
 
-            startInstance(INSTANCE);
+            INSTANCE.start();
 
             // We don't close stderr if `--quiet` is passed, because that
             // hides fatal startup errors. For example, if OpenSearch is
@@ -475,10 +462,6 @@ static void init(final boolean foreground, final Path pidFile, final boolean qui
         }
     }
 
-    static void startInstance(Bootstrap instance) throws NodeValidationException {
-        instance.start();
-    }
-
     @SuppressForbidden(reason = "System#out")
     private static void closeSystOut() {
         System.out.close();