Skip to content

Commit e3b48b1

Browse files
Add a new setting which triggers the byte-based capacity
This version of the code does *not* affect the default behaviour. I'm not sure whether that would be considered a breaking change. (We could override the default in serverless, which I am sure we're free to do.) It would obviously be simple to flip the logic around so that we do change the default to, say, 1% of heap (like we do with enrich).
1 parent 3a92e98 commit e3b48b1

File tree

2 files changed

+39
-5
lines changed

2 files changed

+39
-5
lines changed

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpCache.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ static <V> GeoIpCache createGeoIpCacheWithMaxCount(long maxSize) {
4545
return new GeoIpCache(System::nanoTime, CacheBuilder.<CacheKey, CacheableValue>builder().setMaximumWeight(maxSize).build());
4646
}
4747

48-
// TODO PETE: Make plugin use this instead of the other factory method when the settings require it
4948
static GeoIpCache createGeoIpCacheWithMaxBytes(ByteSizeValue maxByteSize) {
5049
if (maxByteSize.getBytes() < 0) {
5150
throw new IllegalArgumentException("geoip max cache size in bytes must be 0 or greater");

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.elasticsearch.common.settings.Settings;
2525
import org.elasticsearch.common.settings.SettingsFilter;
2626
import org.elasticsearch.common.settings.SettingsModule;
27+
import org.elasticsearch.common.unit.ByteSizeValue;
28+
import org.elasticsearch.core.Strings;
2729
import org.elasticsearch.features.NodeFeature;
2830
import org.elasticsearch.indices.SystemIndexDescriptor;
2931
import org.elasticsearch.ingest.EnterpriseGeoIpTask.EnterpriseGeoIpTaskParams;
@@ -86,7 +88,17 @@ public class IngestGeoIpPlugin extends Plugin
8688
PersistentTaskPlugin,
8789
ActionPlugin,
8890
ReloadablePlugin {
89-
public static final Setting<Long> CACHE_SIZE = Setting.longSetting("ingest.geoip.cache_size", 1000, 0, Setting.Property.NodeScope);
91+
private static final Setting<Long> CACHE_SIZE_COUNT = Setting.longSetting(
92+
"ingest.geoip.cache_size",
93+
1000,
94+
0,
95+
Setting.Property.NodeScope
96+
);
97+
private static final Setting<ByteSizeValue> CACHE_SIZE_BYTES = Setting.byteSizeSetting(
98+
"ingest.geoip.cache_memory_size",
99+
ByteSizeValue.MINUS_ONE,
100+
Setting.Property.NodeScope
101+
);
90102
private static final int GEOIP_INDEX_MAPPINGS_VERSION = 1;
91103
/**
92104
* No longer used for determining the age of mappings, but system index descriptor
@@ -105,7 +117,8 @@ public class IngestGeoIpPlugin extends Plugin
105117
@Override
106118
public List<Setting<?>> getSettings() {
107119
return List.of(
108-
CACHE_SIZE,
120+
CACHE_SIZE_COUNT,
121+
CACHE_SIZE_BYTES,
109122
GeoIpDownloaderTaskExecutor.EAGER_DOWNLOAD_SETTING,
110123
GeoIpDownloaderTaskExecutor.ENABLED_SETTING,
111124
GeoIpDownloader.ENDPOINT_SETTING,
@@ -119,8 +132,7 @@ public List<Setting<?>> getSettings() {
119132
public Map<String, Processor.Factory> getProcessors(Processor.Parameters parameters) {
120133
ingestService.set(parameters.ingestService);
121134

122-
long cacheSize = CACHE_SIZE.get(parameters.env.settings());
123-
GeoIpCache geoIpCache = GeoIpCache.createGeoIpCacheWithMaxCount(cacheSize);
135+
GeoIpCache geoIpCache = createGeoIpCache(parameters.env.settings());
124136
DatabaseNodeService registry = new DatabaseNodeService(
125137
parameters.env,
126138
parameters.client,
@@ -135,6 +147,29 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet
135147
);
136148
}
137149

150+
private static GeoIpCache createGeoIpCache(Settings settings) {
151+
if (settings.hasValue(CACHE_SIZE_BYTES.getKey())) {
152+
if (settings.hasValue(CACHE_SIZE_COUNT.getKey())) {
153+
// Both CACHE_SIZE_COUNT and CACHE_SIZE_BYTES are set, which is an error:
154+
throw new IllegalArgumentException(
155+
Strings.format(
156+
"Both %s and %s are set: please use either %s to set a size based on count or %s to set a size based on bytes of memory",
157+
CACHE_SIZE_COUNT.getKey(),
158+
CACHE_SIZE_BYTES.getKey(),
159+
CACHE_SIZE_COUNT.getKey(),
160+
CACHE_SIZE_BYTES.getKey()
161+
)
162+
);
163+
} else {
164+
// Only CACHE_SIZE_BYTES is set, so use that:
165+
return GeoIpCache.createGeoIpCacheWithMaxBytes(CACHE_SIZE_BYTES.get(settings));
166+
}
167+
} else {
168+
// CACHE_SIZE_BYTES is not set, so use either the explicit or default value of CACHE_SIZE_COUNT:
169+
return GeoIpCache.createGeoIpCacheWithMaxCount(CACHE_SIZE_COUNT.get(settings));
170+
}
171+
}
172+
138173
@Override
139174
public Collection<?> createComponents(PluginServices services) {
140175
try {

0 commit comments

Comments
 (0)