Browse Source

[8.x] Fix enrich cache size setting name (#117575) (#118288)

* Fix enrich cache size setting name (#117575)

The enrich cache size setting accidentally got renamed from
`enrich.cache_size` to `enrich.cache.size` in #111412. This commit
updates the enrich plugin to accept both names and deprecates the
wrong name.

* Remove `UpdateForV10` annotation
Niels Bauman 10 months ago
parent
commit
afd8d8411b

+ 5 - 0
docs/changelog/117575.yaml

@@ -0,0 +1,5 @@
+pr: 117575
+summary: Fix enrich cache size setting name
+area: Ingest Node
+type: bug
+issues: []

+ 54 - 3
x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java

@@ -14,6 +14,8 @@ import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.logging.DeprecationCategory;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Setting;
@@ -74,6 +76,8 @@ import static org.elasticsearch.xpack.core.enrich.EnrichPolicy.ENRICH_INDEX_PATT
 
 public class EnrichPlugin extends Plugin implements SystemIndexPlugin, IngestPlugin {
 
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(EnrichPlugin.class);
+
     static final Setting<Integer> ENRICH_FETCH_SIZE_SETTING = Setting.intSetting(
         "enrich.fetch_size",
         10000,
@@ -126,9 +130,9 @@ public class EnrichPlugin extends Plugin implements SystemIndexPlugin, IngestPlu
         return String.valueOf(maxConcurrentRequests * maxLookupsPerRequest);
     }, val -> Setting.parseInt(val, 1, Integer.MAX_VALUE, QUEUE_CAPACITY_SETTING_NAME), Setting.Property.NodeScope);
 
-    public static final String CACHE_SIZE_SETTING_NAME = "enrich.cache.size";
+    public static final String CACHE_SIZE_SETTING_NAME = "enrich.cache_size";
     public static final Setting<FlatNumberOrByteSizeValue> CACHE_SIZE = new Setting<>(
-        "enrich.cache.size",
+        CACHE_SIZE_SETTING_NAME,
         (String) null,
         (String s) -> FlatNumberOrByteSizeValue.parse(
             s,
@@ -138,16 +142,58 @@ public class EnrichPlugin extends Plugin implements SystemIndexPlugin, IngestPlu
         Setting.Property.NodeScope
     );
 
+    /**
+     * This setting solely exists because the original setting was accidentally renamed in
+     * https://github.com/elastic/elasticsearch/pull/111412.
+     */
+    public static final String CACHE_SIZE_SETTING_BWC_NAME = "enrich.cache.size";
+    public static final Setting<FlatNumberOrByteSizeValue> CACHE_SIZE_BWC = new Setting<>(
+        CACHE_SIZE_SETTING_BWC_NAME,
+        (String) null,
+        (String s) -> FlatNumberOrByteSizeValue.parse(
+            s,
+            CACHE_SIZE_SETTING_BWC_NAME,
+            new FlatNumberOrByteSizeValue(ByteSizeValue.ofBytes((long) (0.01 * JvmInfo.jvmInfo().getConfiguredMaxHeapSize())))
+        ),
+        Setting.Property.NodeScope,
+        Setting.Property.Deprecated
+    );
+
     private final Settings settings;
     private final EnrichCache enrichCache;
+    private final long maxCacheSize;
 
     public EnrichPlugin(final Settings settings) {
         this.settings = settings;
-        FlatNumberOrByteSizeValue maxSize = CACHE_SIZE.get(settings);
+        FlatNumberOrByteSizeValue maxSize;
+        if (settings.hasValue(CACHE_SIZE_SETTING_BWC_NAME)) {
+            if (settings.hasValue(CACHE_SIZE_SETTING_NAME)) {
+                throw new IllegalArgumentException(
+                    Strings.format(
+                        "Both [{}] and [{}] are set, please use [{}]",
+                        CACHE_SIZE_SETTING_NAME,
+                        CACHE_SIZE_SETTING_BWC_NAME,
+                        CACHE_SIZE_SETTING_NAME
+                    )
+                );
+            }
+            deprecationLogger.warn(
+                DeprecationCategory.SETTINGS,
+                "enrich_cache_size_name",
+                "The [{}] setting is deprecated and will be removed in a future version. Please use [{}] instead.",
+                CACHE_SIZE_SETTING_BWC_NAME,
+                CACHE_SIZE_SETTING_NAME
+            );
+            maxSize = CACHE_SIZE_BWC.get(settings);
+        } else {
+            maxSize = CACHE_SIZE.get(settings);
+        }
         if (maxSize.byteSizeValue() != null) {
             this.enrichCache = new EnrichCache(maxSize.byteSizeValue());
+            this.maxCacheSize = maxSize.byteSizeValue().getBytes();
         } else {
             this.enrichCache = new EnrichCache(maxSize.flatNumber());
+            this.maxCacheSize = maxSize.flatNumber();
         }
     }
 
@@ -286,6 +332,11 @@ public class EnrichPlugin extends Plugin implements SystemIndexPlugin, IngestPlu
         return "Manages data related to Enrich policies";
     }
 
+    // Visible for testing
+    long getMaxCacheSize() {
+        return maxCacheSize;
+    }
+
     /**
      * A class that specifies either a flat (unit-less) number or a byte size value.
      */

+ 62 - 0
x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichPluginTests.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.enrich;
+
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.List;
+
+public class EnrichPluginTests extends ESTestCase {
+
+    public void testConstructWithByteSize() {
+        final var size = randomNonNegativeInt();
+        Settings settings = Settings.builder().put(EnrichPlugin.CACHE_SIZE_SETTING_NAME, size + "b").build();
+        EnrichPlugin plugin = new EnrichPlugin(settings);
+        assertEquals(size, plugin.getMaxCacheSize());
+    }
+
+    public void testConstructWithFlatNumber() {
+        final var size = randomNonNegativeInt();
+        Settings settings = Settings.builder().put(EnrichPlugin.CACHE_SIZE_SETTING_NAME, size).build();
+        EnrichPlugin plugin = new EnrichPlugin(settings);
+        assertEquals(size, plugin.getMaxCacheSize());
+    }
+
+    public void testConstructWithByteSizeBwc() {
+        final var size = randomNonNegativeInt();
+        Settings settings = Settings.builder().put(EnrichPlugin.CACHE_SIZE_SETTING_BWC_NAME, size + "b").build();
+        EnrichPlugin plugin = new EnrichPlugin(settings);
+        assertEquals(size, plugin.getMaxCacheSize());
+    }
+
+    public void testConstructWithFlatNumberBwc() {
+        final var size = randomNonNegativeInt();
+        Settings settings = Settings.builder().put(EnrichPlugin.CACHE_SIZE_SETTING_BWC_NAME, size).build();
+        EnrichPlugin plugin = new EnrichPlugin(settings);
+        assertEquals(size, plugin.getMaxCacheSize());
+    }
+
+    public void testConstructWithBothSettings() {
+        Settings settings = Settings.builder()
+            .put(EnrichPlugin.CACHE_SIZE_SETTING_NAME, randomNonNegativeInt())
+            .put(EnrichPlugin.CACHE_SIZE_SETTING_BWC_NAME, randomNonNegativeInt())
+            .build();
+        assertThrows(IllegalArgumentException.class, () -> new EnrichPlugin(settings));
+    }
+
+    @Override
+    protected List<String> filteredWarnings() {
+        final var warnings = super.filteredWarnings();
+        warnings.add("[enrich.cache.size] setting was deprecated in Elasticsearch and will be removed in a future release.");
+        warnings.add(
+            "The [enrich.cache.size] setting is deprecated and will be removed in a future version. Please use [enrich.cache_size] instead."
+        );
+        return warnings;
+    }
+}