Fix for upgrading mapping (#309)

* Fix for upgrading mapping This PR fixes two bugs of upgrading mapping: First, previously we only tried once when upgrading mapping. It is possible we need to try a few more times. The drawback is that if the code has a bug, we retry endlessly. I should fix it in the future. Second, we enclose the upgrade mapping function call under a user context in a recent commit. Upgrade mapping will fail when security plugin is enabled since a regular user cannot upgrade mappings of system indices. The PR also adds upper bounds to our dynamic settings if reasonable. Testing done: 1. Tested that upgrade mapping succeeds with new changes. 2. Tested the upper bounds of the changed settings are in effect.
opendistro-for-elasticsearch · Nov 10, 2020 · 75db58b · 75db58b
1 parent 10dff2a
commit 75db58b
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 7 deletions.
diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/ad/AnomalyDetectorJobRunner.java b/src/main/java/com/amazon/opendistroforelasticsearch/ad/AnomalyDetectorJobRunner.java
@@ -210,6 +210,7 @@ protected void runAdJob(
             );
             return;
         }
+        indexUtil.updateMappingIfNecessary();
         /*
          * We need to handle 3 cases:
          * 1. Detectors created by older versions and never updated. These detectors wont have User details in the
@@ -233,7 +234,7 @@ protected void runAdJob(
         try (InjectSecurity injectSecurity = new InjectSecurity(detectorId, settings, client.threadPool().getThreadContext())) {
             // Injecting user role to verify if the user has permissions for our API.
             injectSecurity.inject(user, roles);
-            indexUtil.updateMappingIfNecessary();
+
             AnomalyResultRequest request = new AnomalyResultRequest(
                 detectorId,
                 detectionStartTime.toEpochMilli(),

diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/ad/indices/AnomalyDetectionIndices.java b/src/main/java/com/amazon/opendistroforelasticsearch/ad/indices/AnomalyDetectionIndices.java
@@ -578,7 +578,11 @@ public void updateMappingIfNecessary() {
         }
 
         final GroupedActionListener<Void> conglomerateListeneer = new GroupedActionListener<>(
-            ActionListener.wrap(r -> updateRunning.set(false), exception -> logger.error("Fail to updatea all mappings")),
+            ActionListener.wrap(r -> updateRunning.set(false), exception -> {
+                // TODO: don't retry endlessly. Can be annoying if there are too many exception logs.
+                updateRunning.set(false);
+                logger.error("Fail to update AD indices' mappings");
+            }),
             updates.size()
         );
 
@@ -602,7 +606,14 @@ public void updateMappingIfNecessary() {
                                 }
                                 conglomerateListeneer.onResponse(null);
                             }, exception -> {
-                                logger.error(new ParameterizedMessage("Fail to update [{}]'s mapping", adIndex.getIndexName()), exception);
+                                logger
+                                    .error(
+                                        new ParameterizedMessage(
+                                            "Fail to update [{}]'s mapping due to [{}]",
+                                            adIndex.getIndexName(),
+                                            exception.getMessage()
+                                        )
+                                    );
                                 conglomerateListeneer.onFailure(exception);
                             })
                         );

diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/ad/settings/AnomalyDetectorSettings.java b/src/main/java/com/amazon/opendistroforelasticsearch/ad/settings/AnomalyDetectorSettings.java
@@ -28,18 +28,27 @@ public final class AnomalyDetectorSettings {
     private AnomalyDetectorSettings() {}
 
     public static final Setting<Integer> MAX_SINGLE_ENTITY_ANOMALY_DETECTORS = Setting
-        .intSetting("opendistro.anomaly_detection.max_anomaly_detectors", 1000, Setting.Property.NodeScope, Setting.Property.Dynamic);
+        .intSetting(
+            "opendistro.anomaly_detection.max_anomaly_detectors",
+            1000,
+            0,
+            10_000,
+            Setting.Property.NodeScope,
+            Setting.Property.Dynamic
+        );
 
     public static final Setting<Integer> MAX_MULTI_ENTITY_ANOMALY_DETECTORS = Setting
         .intSetting(
             "opendistro.anomaly_detection.max_multi_entity_anomaly_detectors",
             10,
+            0,
+            10_000,
             Setting.Property.NodeScope,
             Setting.Property.Dynamic
         );
 
     public static final Setting<Integer> MAX_ANOMALY_FEATURES = Setting
-        .intSetting("opendistro.anomaly_detection.max_anomaly_features", 5, Setting.Property.NodeScope, Setting.Property.Dynamic);
+        .intSetting("opendistro.anomaly_detection.max_anomaly_features", 5, 0, 100, Setting.Property.NodeScope, Setting.Property.Dynamic);
 
     public static final Setting<TimeValue> REQUEST_TIMEOUT = Setting
         .positiveTimeSetting(
@@ -251,7 +260,14 @@ private AnomalyDetectorSettings() {}
 
     // Increase the value will adding pressure to indexing anomaly results and our feature query
     public static final Setting<Integer> MAX_ENTITIES_PER_QUERY = Setting
-        .intSetting("opendistro.anomaly_detection.max_entities_per_query", 1000, 1, Setting.Property.NodeScope, Setting.Property.Dynamic);
+        .intSetting(
+            "opendistro.anomaly_detection.max_entities_per_query",
+            1000,
+            1,
+            100_000_000,
+            Setting.Property.NodeScope,
+            Setting.Property.Dynamic
+        );
 
     // Default number of entities retrieved for Preview API
     public static final int DEFAULT_ENTITIES_FOR_PREVIEW = 30;
@@ -262,6 +278,7 @@ private AnomalyDetectorSettings() {}
             "opendistro.anomaly_detection.max_entities_for_preview",
             DEFAULT_ENTITIES_FOR_PREVIEW,
             1,
+            1000,
             Setting.Property.NodeScope,
             Setting.Property.Dynamic
         );
@@ -278,7 +295,7 @@ private AnomalyDetectorSettings() {}
 
     // max number of primary shards of an AD index
     public static final Setting<Integer> MAX_PRIMARY_SHARDS = Setting
-        .intSetting("opendistro.anomaly_detection.max_primary_shards", 10, 0, Setting.Property.NodeScope, Setting.Property.Dynamic);
+        .intSetting("opendistro.anomaly_detection.max_primary_shards", 10, 0, 200, Setting.Property.NodeScope, Setting.Property.Dynamic);
 
     // max entity value's length
     public static int MAX_ENTITY_LENGTH = 256;