From 197377e5d3007131a29cfe73463455bdc5f1b85f Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Tue, 18 Apr 2023 18:03:18 +0200 Subject: [PATCH 01/10] fix: increase max number of dimensions from 16 to 21 --- .../support/TimeSeriesDimensionsLimitIT.java | 178 ++++++++++++++++++ .../index/mapper/MapperService.java | 2 +- .../index/mapper/TimeSeriesIdFieldMapper.java | 2 +- 3 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java new file mode 100644 index 0000000000000..3d24027da01a7 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -0,0 +1,178 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.timeseries.support; + +import org.elasticsearch.action.index.IndexResponse; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.CheckedConsumer; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.mapper.DocumentParsingException; +import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.json.JsonXContent; + +import java.io.IOException; +import java.time.Instant; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; + +import static org.hamcrest.Matchers.equalTo; + +public class TimeSeriesDimensionsLimitIT extends ESIntegTestCase { + + public void testDimensionFieldNameLimit() throws IOException { + int dimensionFieldLimit = 16; + final String dimensionFieldName = randomAlphaOfLength(randomIntBetween(513, 1024)); + createTimeSeriesIndex(mapping -> { + mapping.startObject("routing_field").field("type", "keyword").field("time_series_dimension", true).endObject(); + mapping.startObject(dimensionFieldName).field("type", "keyword").field("time_series_dimension", true).endObject(); + }, + mapping -> { mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(); }, + () -> List.of("routing_field"), + dimensionFieldLimit + ); + final Exception ex = expectThrows( + DocumentParsingException.class, + () -> client().prepareIndex("test") + .setSource( + "routing_field", randomAlphaOfLength(10), + dimensionFieldName, randomAlphaOfLength(1536), + "gauge", randomIntBetween(10, 20), + "@timestamp", Instant.now().toEpochMilli() + ) + .get() + ); + assertThat( + ex.getCause().getMessage(), + equalTo( + "Dimension name must be less than [512] bytes but [" + dimensionFieldName + "] was [" + dimensionFieldName.length() + "]." + ) + ); + } + + public void testDimensionFieldValueLimit() throws IOException { + int dimensionFieldLimit = 16; + createTimeSeriesIndex( + mapping -> { mapping.startObject("field").field("type", "keyword").field("time_series_dimension", true).endObject(); }, + mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), + () -> List.of("field"), + dimensionFieldLimit + ); + long startTime = Instant.now().toEpochMilli(); + client().prepareIndex("test") + .setSource("field", randomAlphaOfLength(1536), "gauge", randomIntBetween(10, 20), "@timestamp", startTime) + .get(); + final Exception ex = expectThrows( + DocumentParsingException.class, + () -> client().prepareIndex("test") + .setSource("field", randomAlphaOfLength(1537), "gauge", randomIntBetween(10, 20), "@timestamp", startTime + 1) + .get() + ); + assertThat(ex.getCause().getMessage(), equalTo("Dimension fields must be less than [1536] bytes but was [1537].")); + } + + public void testTotalNumberOfDimensionFieldsLimit() { + int dimensionFieldLimit = 16; + final Exception ex = expectThrows(IllegalArgumentException.class, () -> createTimeSeriesIndex(mapping -> { + mapping.startObject("routing_field").field("type", "keyword").field("time_series_dimension", true).endObject(); + for (int i = 0; i < dimensionFieldLimit; i++) { + mapping.startObject(randomAlphaOfLength(10)).field("type", "keyword").field("time_series_dimension", true).endObject(); + } + }, + mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), + () -> List.of("routing_field"), + dimensionFieldLimit + )); + + assertThat(ex.getMessage(), equalTo("Limit of total dimension fields [16] has been exceeded")); + } + + public void testTotalDimensionFieldsSizeLuceneLimit() throws IOException { + int dimensionFieldLimit = 21; + final List dimensionFieldNames = new ArrayList<>(); + createTimeSeriesIndex(mapping -> { + for (int i = 0; i < dimensionFieldLimit; i++) { + String dimensionFieldName = randomAlphaOfLength(10); + dimensionFieldNames.add(dimensionFieldName); + mapping.startObject(dimensionFieldName).field("type", "keyword").field("time_series_dimension", true).endObject(); + } + }, + mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), + () -> List.of(dimensionFieldNames.get(0)), + dimensionFieldLimit + ); + + final Map source = new HashMap<>(); + source.put("gauge", randomIntBetween(10, 20)); + source.put("@timestamp", Instant.now().toEpochMilli()); + for (int i = 0; i < dimensionFieldLimit; i++) { + source.put(dimensionFieldNames.get(i), randomAlphaOfLength(1536)); + } + final IndexResponse indexResponse = client().prepareIndex("test").setSource(source).get(); + assertEquals(RestStatus.CREATED.getStatus(), indexResponse.status().getStatus()); + } + + public void testTotalDimensionFieldsSizeLuceneLimitPlusOne() throws IOException { + int dimensionFieldLimit = 22; + final List dimensionFieldNames = new ArrayList<>(); + createTimeSeriesIndex(mapping -> { + for (int i = 0; i < dimensionFieldLimit; i++) { + String dimensionFieldName = randomAlphaOfLength(10); + dimensionFieldNames.add(dimensionFieldName); + mapping.startObject(dimensionFieldName).field("type", "keyword").field("time_series_dimension", true).endObject(); + } + }, + mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), + () -> List.of(dimensionFieldNames.get(0)), + dimensionFieldLimit + ); + + final Map source = new HashMap<>(); + source.put("routing_field", randomAlphaOfLength(1536)); + source.put("gauge", randomIntBetween(10, 20)); + source.put("@timestamp", Instant.now().toEpochMilli()); + for (int i = 0; i < dimensionFieldLimit; i++) { + source.put(dimensionFieldNames.get(i), randomAlphaOfLength(1536)); + } + final Exception ex = expectThrows(DocumentParsingException.class, () -> client().prepareIndex("test").setSource(source).get()); + // NOTE: the number of bytes of the tsid might change slightly, which is why we do not match strings exactly. + assertEquals("_tsid longer than [32766] bytes [34104].".substring(0, 30), ex.getCause().getMessage().substring(0, 30)); + } + + private void createTimeSeriesIndex( + final CheckedConsumer dimensions, + final CheckedConsumer metrics, + final Supplier> routingPaths, + int dimensionsFieldLimit + ) throws IOException { + XContentBuilder mapping = JsonXContent.contentBuilder(); + mapping.startObject().startObject("properties"); + mapping.startObject("@timestamp").field("type", "date").endObject(); + metrics.accept(mapping); + dimensions.accept(mapping); + mapping.endObject().endObject(); + + Settings settings = Settings.builder() + .put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES) + .putList(IndexMetadata.INDEX_ROUTING_PATH.getKey(), routingPaths.get()) + .put(IndexSettings.TIME_SERIES_START_TIME.getKey(), "2000-01-08T23:40:53.384Z") + .put(IndexSettings.TIME_SERIES_END_TIME.getKey(), "2106-01-08T23:40:53.384Z") + .put(MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING.getKey(), dimensionsFieldLimit) + .build(); + client().admin().indices().prepareCreate("test").setSettings(settings).setMapping(mapping).get(); + } + +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index f58b5cdd08d28..1a8c2a9252c26 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -110,7 +110,7 @@ public enum MergeReason { ); public static final Setting INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING = Setting.longSetting( "index.mapping.dimension_fields.limit", - 16, + 21, 0, Property.Dynamic, Property.IndexScope diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java index a85ba6d0e9a45..139c86216be02 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java @@ -67,7 +67,7 @@ public class TimeSeriesIdFieldMapper extends MetadataFieldMapper { * comfortable given that dimensions are typically going to be less than a * hundred bytes each, but we're being paranoid here. */ - private static final int DIMENSION_VALUE_LIMIT = 1024; + private static final int DIMENSION_VALUE_LIMIT = 1536; @Override public FieldMapper.Builder getMergeBuilder() { From 97a131f5dce9100505c5ff7295719eec9d5fb9d4 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Tue, 18 Apr 2023 18:33:23 +0200 Subject: [PATCH 02/10] fix: checkstyle format violation --- .../support/TimeSeriesDimensionsLimitIT.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index 3d24027da01a7..64fba6706b78d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -48,10 +48,14 @@ public void testDimensionFieldNameLimit() throws IOException { DocumentParsingException.class, () -> client().prepareIndex("test") .setSource( - "routing_field", randomAlphaOfLength(10), - dimensionFieldName, randomAlphaOfLength(1536), - "gauge", randomIntBetween(10, 20), - "@timestamp", Instant.now().toEpochMilli() + "routing_field", + randomAlphaOfLength(10), + dimensionFieldName, + randomAlphaOfLength(1536), + "gauge", + randomIntBetween(10, 20), + "@timestamp", + Instant.now().toEpochMilli() ) .get() ); From ee7abd33bf35f881365f9d90745f750e64142d63 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Tue, 18 Apr 2023 19:08:32 +0200 Subject: [PATCH 03/10] fix: tests using previous dimension fields limits --- .../elasticsearch/index/mapper/DocumentMapperTests.java | 2 +- .../index/mapper/KeywordFieldMapperTests.java | 4 ++-- .../index/mapper/TimeSeriesIdFieldMapperTests.java | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java index af1a0f94522ed..448b55eeea0e2 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java @@ -325,7 +325,7 @@ public void testTooManyDimensionFields() { int max; Settings settings; if (randomBoolean()) { - max = 16; // By default no more than 16 dimensions per document are supported + max = 21; // By default no more than 21 dimensions per document are supported settings = getIndexSettings(); } else { max = between(1, 10000); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 8cdd4d84a030e..84cb5d55fc58c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -396,9 +396,9 @@ public void testDimensionExtraLongKeyword() throws IOException { Exception e = expectThrows( DocumentParsingException.class, - () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1025, 2048)))) + () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1537, 2048)))) ); - assertThat(e.getCause().getMessage(), containsString("Dimension fields must be less than [1024] bytes but was")); + assertThat(e.getCause().getMessage(), containsString("Dimension fields must be less than [1536] bytes but was")); } public void testConfigureSimilarity() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java index f7bac4f303e5a..fb49bca2f0d89 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java @@ -160,9 +160,9 @@ public void testKeywordTooLong() throws IOException { Exception e = expectThrows( DocumentParsingException.class, - () -> parseDocument(docMapper, b -> b.field("a", "more_than_1024_bytes".repeat(52)).field("@timestamp", "2021-10-01")) + () -> parseDocument(docMapper, b -> b.field("a", "more_than_1536_bytes".repeat(80)).field("@timestamp", "2021-10-01")) ); - assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1024] bytes but was [1040].")); + assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1536] bytes but was [1600].")); } public void testKeywordTooLongUtf8() throws IOException { @@ -173,9 +173,9 @@ public void testKeywordTooLongUtf8() throws IOException { String theWordLong = "長い"; Exception e = expectThrows( DocumentParsingException.class, - () -> parseDocument(docMapper, b -> b.field("a", theWordLong.repeat(200)).field("@timestamp", "2021-10-01")) + () -> parseDocument(docMapper, b -> b.field("a", theWordLong.repeat(300)).field("@timestamp", "2021-10-01")) ); - assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1024] bytes but was [1200].")); + assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1536] bytes but was [1800].")); } public void testKeywordNull() throws IOException { From 221adaf562437e3fbfddd611cb1d95ca6dd58c03 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Tue, 18 Apr 2023 19:11:10 +0200 Subject: [PATCH 04/10] fix: always use 21 as dimension limit --- .../timeseries/support/TimeSeriesDimensionsLimitIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index 64fba6706b78d..befed0c275310 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -34,7 +34,7 @@ public class TimeSeriesDimensionsLimitIT extends ESIntegTestCase { public void testDimensionFieldNameLimit() throws IOException { - int dimensionFieldLimit = 16; + int dimensionFieldLimit = 21; final String dimensionFieldName = randomAlphaOfLength(randomIntBetween(513, 1024)); createTimeSeriesIndex(mapping -> { mapping.startObject("routing_field").field("type", "keyword").field("time_series_dimension", true).endObject(); @@ -68,7 +68,7 @@ public void testDimensionFieldNameLimit() throws IOException { } public void testDimensionFieldValueLimit() throws IOException { - int dimensionFieldLimit = 16; + int dimensionFieldLimit = 21; createTimeSeriesIndex( mapping -> { mapping.startObject("field").field("type", "keyword").field("time_series_dimension", true).endObject(); }, mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), @@ -89,7 +89,7 @@ public void testDimensionFieldValueLimit() throws IOException { } public void testTotalNumberOfDimensionFieldsLimit() { - int dimensionFieldLimit = 16; + int dimensionFieldLimit = 21; final Exception ex = expectThrows(IllegalArgumentException.class, () -> createTimeSeriesIndex(mapping -> { mapping.startObject("routing_field").field("type", "keyword").field("time_series_dimension", true).endObject(); for (int i = 0; i < dimensionFieldLimit; i++) { @@ -101,7 +101,7 @@ public void testTotalNumberOfDimensionFieldsLimit() { dimensionFieldLimit )); - assertThat(ex.getMessage(), equalTo("Limit of total dimension fields [16] has been exceeded")); + assertThat(ex.getMessage(), equalTo("Limit of total dimension fields [" + dimensionFieldLimit + "] has been exceeded")); } public void testTotalDimensionFieldsSizeLuceneLimit() throws IOException { From 9b3e0230271cf5b6a5186aa2d06cd32a294b20a2 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Tue, 18 Apr 2023 19:13:27 +0200 Subject: [PATCH 05/10] chore: remove unnecessary braces --- .../timeseries/support/TimeSeriesDimensionsLimitIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index befed0c275310..6185f086400e8 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -40,7 +40,7 @@ public void testDimensionFieldNameLimit() throws IOException { mapping.startObject("routing_field").field("type", "keyword").field("time_series_dimension", true).endObject(); mapping.startObject(dimensionFieldName).field("type", "keyword").field("time_series_dimension", true).endObject(); }, - mapping -> { mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(); }, + mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), () -> List.of("routing_field"), dimensionFieldLimit ); @@ -70,7 +70,7 @@ public void testDimensionFieldNameLimit() throws IOException { public void testDimensionFieldValueLimit() throws IOException { int dimensionFieldLimit = 21; createTimeSeriesIndex( - mapping -> { mapping.startObject("field").field("type", "keyword").field("time_series_dimension", true).endObject(); }, + mapping -> mapping.startObject("field").field("type", "keyword").field("time_series_dimension", true).endObject(), mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), () -> List.of("field"), dimensionFieldLimit From 7549da323684c2e8d1dfc3b6ea912e64e29c02a7 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Wed, 19 Apr 2023 09:49:52 +0200 Subject: [PATCH 06/10] fix: restore value limit to 1024 The tsid encodes both the field name and the field value. As a result, 1536 bytes is the sum of the field name max size and the field value max size, respectively 512 and 1024. --- .../support/TimeSeriesDimensionsLimitIT.java | 21 +++++++++---------- .../index/mapper/TimeSeriesIdFieldMapper.java | 2 +- .../index/mapper/KeywordFieldMapperTests.java | 4 ++-- .../mapper/TimeSeriesIdFieldMapperTests.java | 8 +++---- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index 6185f086400e8..33bddd3f1c4c3 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -51,7 +51,7 @@ public void testDimensionFieldNameLimit() throws IOException { "routing_field", randomAlphaOfLength(10), dimensionFieldName, - randomAlphaOfLength(1536), + randomAlphaOfLength(1024), "gauge", randomIntBetween(10, 20), "@timestamp", @@ -77,15 +77,15 @@ public void testDimensionFieldValueLimit() throws IOException { ); long startTime = Instant.now().toEpochMilli(); client().prepareIndex("test") - .setSource("field", randomAlphaOfLength(1536), "gauge", randomIntBetween(10, 20), "@timestamp", startTime) + .setSource("field", randomAlphaOfLength(1024), "gauge", randomIntBetween(10, 20), "@timestamp", startTime) .get(); final Exception ex = expectThrows( DocumentParsingException.class, () -> client().prepareIndex("test") - .setSource("field", randomAlphaOfLength(1537), "gauge", randomIntBetween(10, 20), "@timestamp", startTime + 1) + .setSource("field", randomAlphaOfLength(1025), "gauge", randomIntBetween(10, 20), "@timestamp", startTime + 1) .get() ); - assertThat(ex.getCause().getMessage(), equalTo("Dimension fields must be less than [1536] bytes but was [1537].")); + assertThat(ex.getCause().getMessage(), equalTo("Dimension fields must be less than [1024] bytes but was [1025].")); } public void testTotalNumberOfDimensionFieldsLimit() { @@ -109,7 +109,7 @@ public void testTotalDimensionFieldsSizeLuceneLimit() throws IOException { final List dimensionFieldNames = new ArrayList<>(); createTimeSeriesIndex(mapping -> { for (int i = 0; i < dimensionFieldLimit; i++) { - String dimensionFieldName = randomAlphaOfLength(10); + String dimensionFieldName = randomAlphaOfLength(512); dimensionFieldNames.add(dimensionFieldName); mapping.startObject(dimensionFieldName).field("type", "keyword").field("time_series_dimension", true).endObject(); } @@ -123,7 +123,7 @@ public void testTotalDimensionFieldsSizeLuceneLimit() throws IOException { source.put("gauge", randomIntBetween(10, 20)); source.put("@timestamp", Instant.now().toEpochMilli()); for (int i = 0; i < dimensionFieldLimit; i++) { - source.put(dimensionFieldNames.get(i), randomAlphaOfLength(1536)); + source.put(dimensionFieldNames.get(i), randomAlphaOfLength(1024)); } final IndexResponse indexResponse = client().prepareIndex("test").setSource(source).get(); assertEquals(RestStatus.CREATED.getStatus(), indexResponse.status().getStatus()); @@ -134,7 +134,7 @@ public void testTotalDimensionFieldsSizeLuceneLimitPlusOne() throws IOException final List dimensionFieldNames = new ArrayList<>(); createTimeSeriesIndex(mapping -> { for (int i = 0; i < dimensionFieldLimit; i++) { - String dimensionFieldName = randomAlphaOfLength(10); + String dimensionFieldName = randomAlphaOfLength(512); dimensionFieldNames.add(dimensionFieldName); mapping.startObject(dimensionFieldName).field("type", "keyword").field("time_series_dimension", true).endObject(); } @@ -145,15 +145,14 @@ public void testTotalDimensionFieldsSizeLuceneLimitPlusOne() throws IOException ); final Map source = new HashMap<>(); - source.put("routing_field", randomAlphaOfLength(1536)); + source.put("routing_field", randomAlphaOfLength(1024)); source.put("gauge", randomIntBetween(10, 20)); source.put("@timestamp", Instant.now().toEpochMilli()); for (int i = 0; i < dimensionFieldLimit; i++) { - source.put(dimensionFieldNames.get(i), randomAlphaOfLength(1536)); + source.put(dimensionFieldNames.get(i), randomAlphaOfLength(1024)); } final Exception ex = expectThrows(DocumentParsingException.class, () -> client().prepareIndex("test").setSource(source).get()); - // NOTE: the number of bytes of the tsid might change slightly, which is why we do not match strings exactly. - assertEquals("_tsid longer than [32766] bytes [34104].".substring(0, 30), ex.getCause().getMessage().substring(0, 30)); + assertEquals("_tsid longer than [32766] bytes [33903].", ex.getCause().getMessage()); } private void createTimeSeriesIndex( diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java index 139c86216be02..a85ba6d0e9a45 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java @@ -67,7 +67,7 @@ public class TimeSeriesIdFieldMapper extends MetadataFieldMapper { * comfortable given that dimensions are typically going to be less than a * hundred bytes each, but we're being paranoid here. */ - private static final int DIMENSION_VALUE_LIMIT = 1536; + private static final int DIMENSION_VALUE_LIMIT = 1024; @Override public FieldMapper.Builder getMergeBuilder() { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 84cb5d55fc58c..8cdd4d84a030e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -396,9 +396,9 @@ public void testDimensionExtraLongKeyword() throws IOException { Exception e = expectThrows( DocumentParsingException.class, - () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1537, 2048)))) + () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1025, 2048)))) ); - assertThat(e.getCause().getMessage(), containsString("Dimension fields must be less than [1536] bytes but was")); + assertThat(e.getCause().getMessage(), containsString("Dimension fields must be less than [1024] bytes but was")); } public void testConfigureSimilarity() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java index fb49bca2f0d89..99e40c602614d 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java @@ -160,9 +160,9 @@ public void testKeywordTooLong() throws IOException { Exception e = expectThrows( DocumentParsingException.class, - () -> parseDocument(docMapper, b -> b.field("a", "more_than_1536_bytes".repeat(80)).field("@timestamp", "2021-10-01")) + () -> parseDocument(docMapper, b -> b.field("a", "more_than_1536_bytes".repeat(52)).field("@timestamp", "2021-10-01")) ); - assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1536] bytes but was [1600].")); + assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1024] bytes but was [1040].")); } public void testKeywordTooLongUtf8() throws IOException { @@ -173,9 +173,9 @@ public void testKeywordTooLongUtf8() throws IOException { String theWordLong = "長い"; Exception e = expectThrows( DocumentParsingException.class, - () -> parseDocument(docMapper, b -> b.field("a", theWordLong.repeat(300)).field("@timestamp", "2021-10-01")) + () -> parseDocument(docMapper, b -> b.field("a", theWordLong.repeat(200)).field("@timestamp", "2021-10-01")) ); - assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1536] bytes but was [1800].")); + assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1024] bytes but was [1200].")); } public void testKeywordNull() throws IOException { From 81e8705a195864fa55cd0a46b93099295cc414a6 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Wed, 19 Apr 2023 10:07:42 +0200 Subject: [PATCH 07/10] fix: restore value limit to 1024 --- .../index/mapper/TimeSeriesIdFieldMapperTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java index 99e40c602614d..f7bac4f303e5a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapperTests.java @@ -160,7 +160,7 @@ public void testKeywordTooLong() throws IOException { Exception e = expectThrows( DocumentParsingException.class, - () -> parseDocument(docMapper, b -> b.field("a", "more_than_1536_bytes".repeat(52)).field("@timestamp", "2021-10-01")) + () -> parseDocument(docMapper, b -> b.field("a", "more_than_1024_bytes".repeat(52)).field("@timestamp", "2021-10-01")) ); assertThat(e.getCause().getMessage(), equalTo("Dimension fields must be less than [1024] bytes but was [1040].")); } From 258a06fedac9f0493b75083e02fcda705c6d7d8e Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Wed, 19 Apr 2023 11:59:10 +0200 Subject: [PATCH 08/10] fix: test default number of dimensions --- .../support/TimeSeriesDimensionsLimitIT.java | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index 33bddd3f1c4c3..358ef271a5450 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -104,6 +104,22 @@ public void testTotalNumberOfDimensionFieldsLimit() { assertThat(ex.getMessage(), equalTo("Limit of total dimension fields [" + dimensionFieldLimit + "] has been exceeded")); } + public void testTotalNumberOfDimensionFieldsDefaultLimit() { + int dimensionFieldLimit = 21; + final Exception ex = expectThrows(IllegalArgumentException.class, () -> createTimeSeriesIndex(mapping -> { + mapping.startObject("routing_field").field("type", "keyword").field("time_series_dimension", true).endObject(); + for (int i = 0; i < dimensionFieldLimit; i++) { + mapping.startObject(randomAlphaOfLength(10)).field("type", "keyword").field("time_series_dimension", true).endObject(); + } + }, + mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), + () -> List.of("routing_field"), + 0 // NOTE: using default field limit + )); + + assertThat(ex.getMessage(), equalTo("Limit of total dimension fields [" + dimensionFieldLimit + "] has been exceeded")); + } + public void testTotalDimensionFieldsSizeLuceneLimit() throws IOException { int dimensionFieldLimit = 21; final List dimensionFieldNames = new ArrayList<>(); @@ -168,14 +184,17 @@ private void createTimeSeriesIndex( dimensions.accept(mapping); mapping.endObject().endObject(); - Settings settings = Settings.builder() + Settings.Builder settings = Settings.builder() .put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES) .putList(IndexMetadata.INDEX_ROUTING_PATH.getKey(), routingPaths.get()) .put(IndexSettings.TIME_SERIES_START_TIME.getKey(), "2000-01-08T23:40:53.384Z") - .put(IndexSettings.TIME_SERIES_END_TIME.getKey(), "2106-01-08T23:40:53.384Z") - .put(MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING.getKey(), dimensionsFieldLimit) - .build(); - client().admin().indices().prepareCreate("test").setSettings(settings).setMapping(mapping).get(); + .put(IndexSettings.TIME_SERIES_END_TIME.getKey(), "2106-01-08T23:40:53.384Z"); + + if (dimensionsFieldLimit > 0) { + settings.put(MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING.getKey(), dimensionsFieldLimit); + } + + client().admin().indices().prepareCreate("test").setSettings(settings.build()).setMapping(mapping).get(); } } From 7947a7117466dd60d9da404904fa20558e320802 Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Thu, 20 Apr 2023 09:01:45 +0200 Subject: [PATCH 09/10] fix: use Integer instead of int --- .../timeseries/support/TimeSeriesDimensionsLimitIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index 358ef271a5450..c7f6da1d4508d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -175,7 +175,7 @@ private void createTimeSeriesIndex( final CheckedConsumer dimensions, final CheckedConsumer metrics, final Supplier> routingPaths, - int dimensionsFieldLimit + final Integer dimensionsFieldLimit ) throws IOException { XContentBuilder mapping = JsonXContent.contentBuilder(); mapping.startObject().startObject("properties"); @@ -190,7 +190,7 @@ private void createTimeSeriesIndex( .put(IndexSettings.TIME_SERIES_START_TIME.getKey(), "2000-01-08T23:40:53.384Z") .put(IndexSettings.TIME_SERIES_END_TIME.getKey(), "2106-01-08T23:40:53.384Z"); - if (dimensionsFieldLimit > 0) { + if (dimensionsFieldLimit != null) { settings.put(MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING.getKey(), dimensionsFieldLimit); } From 164bfa3dc5804a9f6907be5402c00b447a2a303c Mon Sep 17 00:00:00 2001 From: Salvatore Campagna Date: Thu, 20 Apr 2023 11:02:57 +0200 Subject: [PATCH 10/10] fix: use null instead of 0 --- .../timeseries/support/TimeSeriesDimensionsLimitIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java index c7f6da1d4508d..6812a6448c946 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/timeseries/support/TimeSeriesDimensionsLimitIT.java @@ -114,7 +114,7 @@ public void testTotalNumberOfDimensionFieldsDefaultLimit() { }, mapping -> mapping.startObject("gauge").field("type", "integer").field("time_series_metric", "gauge").endObject(), () -> List.of("routing_field"), - 0 // NOTE: using default field limit + null // NOTE: using default field limit )); assertThat(ex.getMessage(), equalTo("Limit of total dimension fields [" + dimensionFieldLimit + "] has been exceeded"));