From 631519894f264b31e2902cb9258c3154656926c1 Mon Sep 17 00:00:00 2001 From: Niels Bauman <33722607+nielsbauman@users.noreply.github.com> Date: Mon, 9 Dec 2024 19:15:49 +0100 Subject: [PATCH] [8.16] Fix enrich cache size setting name (#117575) (#118286) * 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 --- docs/changelog/117575.yaml | 5 ++ .../xpack/enrich/EnrichPlugin.java | 57 ++++++++++++++++- .../xpack/enrich/EnrichPluginTests.java | 62 +++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/117575.yaml create mode 100644 x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichPluginTests.java diff --git a/docs/changelog/117575.yaml b/docs/changelog/117575.yaml new file mode 100644 index 0000000000000..781444ae97be5 --- /dev/null +++ b/docs/changelog/117575.yaml @@ -0,0 +1,5 @@ +pr: 117575 +summary: Fix enrich cache size setting name +area: Ingest Node +type: bug +issues: [] diff --git a/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java b/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java index 1a68ada60b6f1..2a2d5211daadf 100644 --- a/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java +++ b/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java @@ -14,6 +14,8 @@ 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 @@ public class EnrichPlugin extends Plugin implements SystemIndexPlugin, IngestPlugin { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(EnrichPlugin.class); + static final Setting 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 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 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 String getFeatureDescription() { 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. */ diff --git a/x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichPluginTests.java b/x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichPluginTests.java new file mode 100644 index 0000000000000..07de0e0967448 --- /dev/null +++ b/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 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; + } +}