Replaces DCF isInitialized with a countdown latch and waitForInit

Signed-off-by: Darshit Chanpura <[email protected]>
DarshitChanpura · Sep 13, 2023 · 8fc02dc · 8fc02dc
1 parent c9e109f
commit 8fc02dc
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 54 deletions.
diff --git a/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java b/src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java
@@ -32,6 +32,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
+import org.opensearch.OpenSearchException;
 import org.opensearch.action.FailedNodeException;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.nodes.TransportNodesAction;
@@ -125,7 +126,13 @@ protected ConfigUpdateResponse newResponse(
 
     @Override
     protected ConfigUpdateNodeResponse nodeOperation(final NodeConfigUpdateRequest request) {
-        configurationRepository.reloadConfiguration(CType.fromStringValues((request.request.getConfigTypes())));
+        try {
+            dynamicConfigFactory.waitForInit();
+        } catch (InterruptedException e) {
+            // thread interrupted while waiting for DCF to be initialized
+            throw new OpenSearchException("Error while waiting for DynamicConfigFactory to be initialized: ", e);
+        }
+        configurationRepository.reloadConfiguration(CType.fromStringValues(request.request.getConfigTypes()));
         backendRegistry.get().invalidateCache();
         return new ConfigUpdateNodeResponse(clusterService.localNode(), request.request.getConfigTypes(), null);
     }

diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java
@@ -215,20 +215,19 @@ private ConfigurationRepository(
                     }
                 }
 
-                while (!dynamicConfigFactory.isInitialized()) {
+                // wait for DCF to be initialized
+                dynamicConfigFactory.waitForInit();
+
+                try {
+                    LOGGER.debug("Try to load config ...");
+                    reloadConfiguration(Arrays.asList(CType.values()));
+                } catch (Exception e) {
+                    LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e)));
                     try {
-                        LOGGER.debug("Try to load config ...");
-                        reloadConfiguration(Arrays.asList(CType.values()));
-                        break;
-                    } catch (Exception e) {
-                        LOGGER.debug("Unable to load configuration due to {}", String.valueOf(ExceptionUtils.getRootCause(e)));
-                        try {
-                            Thread.sleep(3000);
-                        } catch (InterruptedException e1) {
-                            Thread.currentThread().interrupt();
-                            LOGGER.debug("Thread was interrupted so we cancel initialization");
-                            break;
-                        }
+                        Thread.sleep(3000);
+                    } catch (InterruptedException e1) {
+                        Thread.currentThread().interrupt();
+                        LOGGER.debug("Thread was interrupted so we cancel initialization");
                     }
                 }
 
@@ -381,7 +380,7 @@ public void reloadConfiguration(Collection<CType> configTypes) throws ConfigUpda
                     LOCK.unlock();
                 }
             } else {
-                throw new ConfigUpdateAlreadyInProgressException("A config update is already imn progress");
+                throw new ConfigUpdateAlreadyInProgressException("A config update is already in progress");
             }
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();

diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java
@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import com.fasterxml.jackson.databind.JsonNode;
@@ -73,8 +74,9 @@
 import org.opensearch.security.support.WildcardMatcher;
 import org.opensearch.threadpool.ThreadPool;
 
-public class DynamicConfigFactory implements Initializable, ConfigurationChangeListener {
+public class DynamicConfigFactory implements ConfigurationChangeListener {
 
+    private final CountDownLatch initializationLatch = new CountDownLatch(1);
     public static final EventBusBuilder EVENT_BUS_BUILDER = EventBus.builder();
     private static SecurityDynamicConfiguration<RoleV7> staticRoles = SecurityDynamicConfiguration.empty();
     private static SecurityDynamicConfiguration<ActionGroupsV7> staticActionGroups = SecurityDynamicConfiguration.empty();
@@ -313,7 +315,7 @@ public void onChange(Map<CType, SecurityDynamicConfiguration<?>> typeToConfig) {
         }
 
         initialized.set(true);
-
+        initializationLatch.countDown();
     }
 
     private static ConfigV6 getConfigV6(SecurityDynamicConfiguration<?> sdc) {
@@ -328,9 +330,10 @@ private static ConfigV7 getConfigV7(SecurityDynamicConfiguration<?> sdc) {
         return c.getCEntry("config");
     }
 
-    @Override
-    public final boolean isInitialized() {
-        return initialized.get();
+    public void waitForInit() throws InterruptedException {
+        // wait for DCF to completely initialized
+        initializationLatch.await();
+
     }
 
     public void registerDCFListener(Object listener) {

diff --git a/src/main/java/org/opensearch/security/securityconf/Initializable.java b/src/main/java/org/opensearch/security/securityconf/Initializable.java