From 8f6d6f8466b94913b392373e6aef2c178164bbcb Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Tue, 24 Sep 2024 10:58:02 +0200 Subject: [PATCH 1/5] feature: ignore_above default to 1024 for logsdb --- .../logsdb/LogsIndexModeCustomSettingsIT.java | 49 +++++++++++++++++++ .../common/settings/Setting.java | 10 ++++ .../elasticsearch/index/IndexSettings.java | 15 +++--- .../elasticsearch/index/IndexVersions.java | 1 + 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java index c0e3142b9a8db..de5f1c04f808d 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java @@ -334,6 +334,55 @@ public void testIgnoreMalformedSetting() throws IOException { } } + public void testIgnoreAboveSetting() throws IOException { + // with default template + { + assertOK(createDataStream(client, "logs-test-1")); + String logsIndex1 = getDataStreamBackingIndex(client, "logs-test-1", 0); + assertThat(getSetting(client, logsIndex1, "index.mapping.ignore_above"), equalTo("1024")); + for (String newValue : List.of("512", "2048")) { + closeIndex(logsIndex1); + updateIndexSettings(logsIndex1, Settings.builder().put("index.mapping.ignore_above", newValue)); + assertThat(getSetting(client, logsIndex1, "index.mapping.ignore_above"), equalTo(newValue)); + } + } + // with override template + { + var template = """ + { + "template": { + "settings": { + "index": { + "mapping": { + "ignore_above": "128" + } + } + } + } + }"""; + assertOK(putComponentTemplate(client, "logs@custom", template)); + assertOK(createDataStream(client, "logs-custom-dev")); + String index = getDataStreamBackingIndex(client, "logs-custom-dev", 0); + assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo("128")); + for (String newValue : List.of("64", "256")) { + closeIndex(index); + updateIndexSettings(index, Settings.builder().put("index.mapping.ignore_above", newValue)); + assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo(newValue)); + } + } + // standard index + { + String index = "test-index"; + createIndex(index); + assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo(Integer.toString(Integer.MAX_VALUE))); + for (String newValue : List.of("256", "512")) { + closeIndex(index); + updateIndexSettings(index, Settings.builder().put("index.mapping.ignore_above", newValue)); + assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo(newValue)); + } + } + } + private static Map getMapping(final RestClient client, final String indexName) throws IOException { final Request request = new Request("GET", "/" + indexName + "/_mapping"); diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 9a235bc4e162c..9481116187ba1 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -1366,6 +1366,16 @@ public static Setting intSetting(String key, int defaultValue, int minV return new Setting<>(key, Integer.toString(defaultValue), intParser(key, minValue, properties), properties); } + public static Setting intSetting( + String key, + Function defaultValueFn, + int minValue, + int maxValue, + Property... properties + ) { + return new Setting<>(key, defaultValueFn, intParser(key, minValue, maxValue, properties), properties); + } + private static Function intParser(String key, int minValue, Property[] properties) { final boolean isFiltered = isFiltered(properties); return s -> parseInt(s, minValue, key, isFiltered); diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index f6ad5e22b9ed7..d24ffb017c68c 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -714,13 +714,14 @@ public Iterator> settings() { * "index.mapping.ignore_above": 256 * */ - public static final Setting IGNORE_ABOVE_SETTING = Setting.intSetting( - "index.mapping.ignore_above", - Integer.MAX_VALUE, - 0, - Property.IndexScope, - Property.ServerlessPublic - ); + public static final Setting IGNORE_ABOVE_SETTING = Setting.intSetting("index.mapping.ignore_above", settings -> { + if (IndexSettings.MODE.get(settings) == IndexMode.LOGSDB + && IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(IndexVersions.ENABLE_IGNORE_ABOVE_LOGSDB)) { + return "1024"; + } else { + return String.valueOf(Integer.MAX_VALUE); + } + }, 0, Integer.MAX_VALUE, Property.IndexScope, Property.ServerlessPublic); public static final NodeFeature IGNORE_ABOVE_INDEX_LEVEL_SETTING = new NodeFeature("mapper.ignore_above_index_level_setting"); private final Index index; diff --git a/server/src/main/java/org/elasticsearch/index/IndexVersions.java b/server/src/main/java/org/elasticsearch/index/IndexVersions.java index 18d14aad9a912..0013421cec0c6 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexVersions.java +++ b/server/src/main/java/org/elasticsearch/index/IndexVersions.java @@ -115,6 +115,7 @@ private static IndexVersion def(int id, Version luceneVersion) { public static final IndexVersion INDEX_SORTING_ON_NESTED = def(8_512_00_0, Version.LUCENE_9_11_1); public static final IndexVersion LENIENT_UPDATEABLE_SYNONYMS = def(8_513_00_0, Version.LUCENE_9_11_1); public static final IndexVersion ENABLE_IGNORE_MALFORMED_LOGSDB = def(8_514_00_0, Version.LUCENE_9_11_1); + public static final IndexVersion ENABLE_IGNORE_ABOVE_LOGSDB = def(8_515_00_0, Version.LUCENE_9_11_1); /* * STOP! READ THIS FIRST! No, really, From eed95bd3f417ad12ee96f37a6c0664c0c3d383ec Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Mon, 30 Sep 2024 13:19:42 +0200 Subject: [PATCH 2/5] fix: default to 32766 / 4 = 8191 --- .../main/java/org/elasticsearch/index/IndexSettings.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index d24ffb017c68c..9fe660038eedc 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -701,7 +701,7 @@ public Iterator> settings() { /** * The `index.mapping.ignore_above` setting defines the maximum length for the content of a field that will be indexed * or stored. If the length of the field’s content exceeds this limit, the field value will be ignored during indexing. - * This setting is useful for `keyword`, `flattened`, and `wildcard` fields where very large values are undesirable. + * This setting is useful for `keyword`, `flattened`, and `wildcard` fields where very large values are undesirable. * It allows users to manage the size of indexed data by skipping fields with excessively long content. As an index-level * setting, it applies to all `keyword` and `wildcard` fields, as well as to keyword values within `flattened` fields. * When it comes to arrays, the `ignore_above` setting applies individually to each element of the array. If any element's @@ -713,11 +713,16 @@ public Iterator> settings() { * 
      * "index.mapping.ignore_above": 256
      *
+ *

+ * NOTE: The value for `ignore_above` is the _character count_, but Lucene counts + * bytes. Here we set the limit to `32766 / 4 = 8191` since UTF-8 characters may + * occupy at most 4 bytes. */ + public static final Setting IGNORE_ABOVE_SETTING = Setting.intSetting("index.mapping.ignore_above", settings -> { if (IndexSettings.MODE.get(settings) == IndexMode.LOGSDB && IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(IndexVersions.ENABLE_IGNORE_ABOVE_LOGSDB)) { - return "1024"; + return "8191"; } else { return String.valueOf(Integer.MAX_VALUE); } From a47efe3e158fec3b1a1f79c36f7c9a5d1f4d4b9c Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Mon, 30 Sep 2024 15:27:58 +0200 Subject: [PATCH 3/5] fix: extract method getIgnoreAboveDefaultValue --- .../org/elasticsearch/index/IndexSettings.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index 9fe660038eedc..e82f9eff7d5e0 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -719,14 +719,24 @@ public Iterator> settings() { * occupy at most 4 bytes. */ - public static final Setting IGNORE_ABOVE_SETTING = Setting.intSetting("index.mapping.ignore_above", settings -> { + public static final Setting IGNORE_ABOVE_SETTING = Setting.intSetting( + "index.mapping.ignore_above", + IndexSettings::getIgnoreAboveDefaultValue, + 0, + Integer.MAX_VALUE, + Property.IndexScope, + Property.ServerlessPublic + ); + + private static String getIgnoreAboveDefaultValue(final Settings settings) { if (IndexSettings.MODE.get(settings) == IndexMode.LOGSDB && IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(IndexVersions.ENABLE_IGNORE_ABOVE_LOGSDB)) { return "8191"; } else { return String.valueOf(Integer.MAX_VALUE); } - }, 0, Integer.MAX_VALUE, Property.IndexScope, Property.ServerlessPublic); + } + public static final NodeFeature IGNORE_ABOVE_INDEX_LEVEL_SETTING = new NodeFeature("mapper.ignore_above_index_level_setting"); private final Index index; From e95feb704f30fdee6eda62ccb6e3654b7587ddaa Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Mon, 30 Sep 2024 16:13:49 +0200 Subject: [PATCH 4/5] test: update default value --- .../datastreams/logsdb/LogsIndexModeCustomSettingsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java index fd35f658417c3..6ade772fd63c5 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java @@ -361,7 +361,7 @@ public void testIgnoreAboveSetting() throws IOException { { assertOK(createDataStream(client, "logs-test-1")); String logsIndex1 = getDataStreamBackingIndex(client, "logs-test-1", 0); - assertThat(getSetting(client, logsIndex1, "index.mapping.ignore_above"), equalTo("1024")); + assertThat(getSetting(client, logsIndex1, "index.mapping.ignore_above"), equalTo("8191")); for (String newValue : List.of("512", "2048")) { closeIndex(logsIndex1); updateIndexSettings(logsIndex1, Settings.builder().put("index.mapping.ignore_above", newValue)); From 5dff17401f2a2a0179b432c131d0cee805f11726 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Wed, 2 Oct 2024 10:46:31 +0200 Subject: [PATCH 5/5] test: include some more values --- .../logsdb/LogsIndexModeCustomSettingsIT.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java index 6ade772fd63c5..bcede8014eb9e 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.ClassRule; @@ -362,11 +363,22 @@ public void testIgnoreAboveSetting() throws IOException { assertOK(createDataStream(client, "logs-test-1")); String logsIndex1 = getDataStreamBackingIndex(client, "logs-test-1", 0); assertThat(getSetting(client, logsIndex1, "index.mapping.ignore_above"), equalTo("8191")); - for (String newValue : List.of("512", "2048")) { + for (String newValue : List.of("512", "2048", "12000", String.valueOf(Integer.MAX_VALUE))) { closeIndex(logsIndex1); updateIndexSettings(logsIndex1, Settings.builder().put("index.mapping.ignore_above", newValue)); assertThat(getSetting(client, logsIndex1, "index.mapping.ignore_above"), equalTo(newValue)); } + for (String newValue : List.of(String.valueOf((long) Integer.MAX_VALUE + 1), String.valueOf(Long.MAX_VALUE))) { + closeIndex(logsIndex1); + ResponseException ex = assertThrows( + ResponseException.class, + () -> updateIndexSettings(logsIndex1, Settings.builder().put("index.mapping.ignore_above", newValue)) + ); + assertThat( + ex.getMessage(), + Matchers.containsString("Failed to parse value [" + newValue + "] for setting [index.mapping.ignore_above]") + ); + } } // with override template { @@ -386,7 +398,7 @@ public void testIgnoreAboveSetting() throws IOException { assertOK(createDataStream(client, "logs-custom-dev")); String index = getDataStreamBackingIndex(client, "logs-custom-dev", 0); assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo("128")); - for (String newValue : List.of("64", "256")) { + for (String newValue : List.of("64", "256", "12000", String.valueOf(Integer.MAX_VALUE))) { closeIndex(index); updateIndexSettings(index, Settings.builder().put("index.mapping.ignore_above", newValue)); assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo(newValue)); @@ -397,7 +409,7 @@ public void testIgnoreAboveSetting() throws IOException { String index = "test-index"; createIndex(index); assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo(Integer.toString(Integer.MAX_VALUE))); - for (String newValue : List.of("256", "512")) { + for (String newValue : List.of("256", "512", "12000", String.valueOf(Integer.MAX_VALUE))) { closeIndex(index); updateIndexSettings(index, Settings.builder().put("index.mapping.ignore_above", newValue)); assertThat(getSetting(client, index, "index.mapping.ignore_above"), equalTo(newValue));