Skip to content

Commit

Permalink
Fix enrich cache size setting name (elastic#117575)
Browse files Browse the repository at this point in the history
The enrich cache size setting accidentally got renamed from
`enrich.cache_size` to `enrich.cache.size` in elastic#111412. This commit
updates the enrich plugin to accept both names and deprecates the
wrong name.
  • Loading branch information
nielsbauman committed Dec 9, 2024
1 parent eebc196 commit 4d6116a
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 3 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/117575.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 117575
summary: Fix enrich cache size setting name
area: Ingest Node
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,6 +25,7 @@
import org.elasticsearch.common.unit.MemorySizeValue;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.UpdateForV10;
import org.elasticsearch.features.NodeFeature;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.ingest.Processor;
Expand Down Expand Up @@ -74,6 +77,8 @@

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,
Expand Down Expand Up @@ -126,9 +131,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,
Expand All @@ -138,16 +143,59 @@ 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.
*/
@UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT)
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();
}
}

Expand Down Expand Up @@ -286,6 +334,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.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}

0 comments on commit 4d6116a

Please sign in to comment.