From 6c5c5af816a7fe9cd3f8a901f51070534f4dd812 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 3 Sep 2024 15:22:51 +0530 Subject: [PATCH] [Star tree] Star tree index validations (#15533) --------- Signed-off-by: Bharathwaj G --- .../index/mapper/StarTreeMapperIT.java | 347 +++++++++++++++++- .../metadata/MetadataCreateIndexService.java | 70 ++++ .../MetadataIndexTemplateService.java | 2 + .../MetadataUpdateSettingsService.java | 7 + .../common/settings/ClusterSettings.java | 4 + .../common/settings/IndexScopedSettings.java | 2 + .../org/opensearch/index/IndexSettings.java | 9 +- .../CompositeIndexSettings.java | 15 +- .../startree/StarTreeIndexSettings.java | 21 ++ .../index/mapper/DocumentParser.java | 11 + .../index/mapper/MapperService.java | 14 + .../opensearch/index/mapper/ObjectMapper.java | 9 + .../index/mapper/StarTreeMapper.java | 68 +++- .../MetadataIndexTemplateServiceTests.java | 15 + .../TranslogFlushIntervalSettingsTests.java | 146 ++++++++ .../StarTreeDocValuesFormatTests.java | 21 +- .../index/mapper/ObjectMapperTests.java | 10 +- .../index/mapper/StarTreeMapperTests.java | 118 ++++++ 18 files changed, 863 insertions(+), 26 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/metadata/TranslogFlushIntervalSettingsTests.java diff --git a/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java b/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java index 52c6c6801a3c2..8cfb710679137 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java @@ -8,19 +8,28 @@ package org.opensearch.index.mapper; +import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.opensearch.action.index.IndexResponse; +import org.opensearch.action.search.SearchResponse; import org.opensearch.action.support.master.AcknowledgedResponse; import org.opensearch.common.Rounding; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.index.Index; +import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.IndexService; +import org.opensearch.index.IndexSettings; import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.compositeindex.datacube.DateDimension; import org.opensearch.index.compositeindex.datacube.MetricStat; import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; +import org.opensearch.index.query.QueryBuilders; import org.opensearch.indices.IndicesService; +import org.opensearch.search.SearchHit; import org.opensearch.test.OpenSearchIntegTestCase; import org.junit.After; import org.junit.Before; @@ -39,6 +48,10 @@ */ public class StarTreeMapperIT extends OpenSearchIntegTestCase { private static final String TEST_INDEX = "test"; + Settings settings = Settings.builder() + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + .build(); private static XContentBuilder createMinimalTestMapping(boolean invalidDim, boolean invalidMetric, boolean keywordDim) { try { @@ -116,6 +129,12 @@ private static XContentBuilder createMaxDimTestMapping() { .startObject() .field("name", "dim2") .endObject() + .startObject() + .field("name", "dim3") + .endObject() + .startObject() + .field("name", "dim4") + .endObject() .endArray() .endObject() .endObject() @@ -132,6 +151,10 @@ private static XContentBuilder createMaxDimTestMapping() { .field("type", "integer") .field("doc_values", true) .endObject() + .startObject("dim4") + .field("type", "integer") + .field("doc_values", true) + .endObject() .endObject() .endObject(); } catch (IOException e) { @@ -223,6 +246,56 @@ private static XContentBuilder createUpdateTestMapping(boolean changeDim, boolea } } + private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, boolean isDuplicateMetric) { + XContentBuilder mapping = null; + try { + mapping = jsonBuilder().startObject() + .startObject("composite") + .startObject("startree-1") + .field("type", "star_tree") + .startObject("config") + .startArray("ordered_dimensions") + .startObject() + .field("name", "timestamp") + .endObject() + .startObject() + .field("name", "numeric_dv") + .endObject() + .startObject() + .field("name", isDuplicateDim ? "numeric_dv" : "numeric_dv1") // Duplicate dimension + .endObject() + .endArray() + .startArray("metrics") + .startObject() + .field("name", "numeric_dv") + .endObject() + .startObject() + .field("name", isDuplicateMetric ? "numeric_dv" : "numeric_dv1") // Duplicate metric + .endObject() + .endArray() + .endObject() + .endObject() + .endObject() + .startObject("properties") + .startObject("timestamp") + .field("type", "date") + .endObject() + .startObject("numeric_dv") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .startObject("numeric_dv1") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .endObject() + .endObject(); + } catch (IOException e) { + fail("Failed to create mapping: " + e.getMessage()); + } + return mapping; + } + private static String getDim(boolean hasDocValues, boolean isKeyword) { if (hasDocValues) { return "numeric"; @@ -244,7 +317,7 @@ public final void setupNodeSettings() { } public void testValidCompositeIndex() { - prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); Iterable dataNodeInstances = internalCluster().getDataNodeInstances(IndicesService.class); for (IndicesService service : dataNodeInstances) { final Index index = resolveIndex("test"); @@ -285,8 +358,91 @@ public void testValidCompositeIndex() { } } + public void testCompositeIndexWithIndexNotSpecified() { + Settings settings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + .build(); + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get() + ); + assertEquals( + "Failed to parse mapping [_doc]: Set 'index.composite_index' as true as part of index settings to use star tree index", + ex.getMessage() + ); + } + + public void testCompositeIndexWithHigherTranslogFlushSize() { + Settings settings = Settings.builder() + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(513, ByteSizeUnit.MB)) + .build(); + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get() + ); + assertEquals("You can configure 'index.translog.flush_threshold_size' with upto '512mb' for composite index", ex.getMessage()); + } + + public void testCompositeIndexWithArraysInCompositeField() throws IOException { + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); + // Attempt to index a document with an array field + XContentBuilder doc = jsonBuilder().startObject() + .field("timestamp", "2023-06-01T12:00:00Z") + .startArray("numeric_dv") + .value(10) + .value(20) + .value(30) + .endArray() + .endObject(); + + // Index the document and refresh + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> client().prepareIndex(TEST_INDEX).setSource(doc).get() + ); + assertEquals( + "object mapping for [_doc] with array for [numeric_dv] cannot be accepted as field is also part of composite index mapping which does not accept arrays", + ex.getMessage() + ); + } + + public void testCompositeIndexWithArraysInNonCompositeField() throws IOException { + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); + // Attempt to index a document with an array field + XContentBuilder doc = jsonBuilder().startObject() + .field("timestamp", "2023-06-01T12:00:00Z") + .startArray("numeric") + .value(10) + .value(20) + .value(30) + .endArray() + .endObject(); + + // Index the document and refresh + IndexResponse indexResponse = client().prepareIndex(TEST_INDEX).setSource(doc).get(); + + assertEquals(RestStatus.CREATED, indexResponse.status()); + + client().admin().indices().prepareRefresh(TEST_INDEX).get(); + // Verify the document was indexed + SearchResponse searchResponse = client().prepareSearch(TEST_INDEX).setQuery(QueryBuilders.matchAllQuery()).get(); + + assertEquals(1, searchResponse.getHits().getTotalHits().value); + + // Verify the values in the indexed document + SearchHit hit = searchResponse.getHits().getAt(0); + assertEquals("2023-06-01T12:00:00Z", hit.getSourceAsMap().get("timestamp")); + + List values = (List) hit.getSourceAsMap().get("numeric"); + assertEquals(3, values.size()); + assertTrue(values.contains(10)); + assertTrue(values.contains(20)); + assertTrue(values.contains(30)); + } + public void testUpdateIndexWithAdditionOfStarTree() { - prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -296,7 +452,7 @@ public void testUpdateIndexWithAdditionOfStarTree() { } public void testUpdateIndexWithNewerStarTree() { - prepareCreate(TEST_INDEX).setMapping(createTestMappingWithoutStarTree(false, false, false)).get(); + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createTestMappingWithoutStarTree(false, false, false)).get(); IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, @@ -309,7 +465,7 @@ public void testUpdateIndexWithNewerStarTree() { } public void testUpdateIndexWhenMappingIsDifferent() { - prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); // update some field in the mapping IllegalArgumentException ex = expectThrows( @@ -320,7 +476,7 @@ public void testUpdateIndexWhenMappingIsDifferent() { } public void testUpdateIndexWhenMappingIsSame() { - prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); // update some field in the mapping AcknowledgedResponse putMappingResponse = client().admin() @@ -368,7 +524,7 @@ public void testUpdateIndexWhenMappingIsSame() { public void testInvalidDimCompositeIndex() { IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, - () -> prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(true, false, false)).get() + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(true, false, false)).get() ); assertEquals( "Aggregations not supported for the dimension field [numeric] with field type [integer] as part of star tree field", @@ -379,8 +535,14 @@ public void testInvalidDimCompositeIndex() { public void testMaxDimsCompositeIndex() { MapperParsingException ex = expectThrows( MapperParsingException.class, - () -> prepareCreate(TEST_INDEX).setMapping(createMaxDimTestMapping()) - .setSettings(Settings.builder().put(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 2)) + () -> prepareCreate(TEST_INDEX).setSettings(settings) + .setMapping(createMaxDimTestMapping()) + .setSettings( + Settings.builder() + .put(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 2) + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + ) .get() ); assertEquals( @@ -389,11 +551,35 @@ public void testMaxDimsCompositeIndex() { ); } + public void testMaxMetricsCompositeIndex() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> prepareCreate(TEST_INDEX).setSettings(settings) + .setMapping(createMaxDimTestMapping()) + .setSettings( + Settings.builder() + .put(StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING.getKey(), 4) + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + ) + .get() + ); + assertEquals( + "Failed to parse mapping [_doc]: There cannot be more than [4] base metrics for star tree field [startree-1]", + ex.getMessage() + ); + } + public void testMaxCalendarIntervalsCompositeIndex() { MapperParsingException ex = expectThrows( MapperParsingException.class, () -> prepareCreate(TEST_INDEX).setMapping(createMaxDimTestMapping()) - .setSettings(Settings.builder().put(StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING.getKey(), 1)) + .setSettings( + Settings.builder() + .put(StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING.getKey(), 1) + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + ) .get() ); assertEquals( @@ -405,7 +591,7 @@ public void testMaxCalendarIntervalsCompositeIndex() { public void testUnsupportedDim() { MapperParsingException ex = expectThrows( MapperParsingException.class, - () -> prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, true)).get() + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, true)).get() ); assertEquals( "Failed to parse mapping [_doc]: unsupported field type associated with dimension [keyword] as part of star tree field [startree-1]", @@ -416,7 +602,7 @@ public void testUnsupportedDim() { public void testInvalidMetric() { IllegalArgumentException ex = expectThrows( IllegalArgumentException.class, - () -> prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, true, false)).get() + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, true, false)).get() ); assertEquals( "Aggregations not supported for the metrics field [numeric] with field type [integer] as part of star tree field", @@ -424,6 +610,145 @@ public void testInvalidMetric() { ); } + public void testDuplicateDimensions() { + XContentBuilder finalMapping = getMappingWithDuplicateFields(true, false); + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(finalMapping).setSettings(settings).get() + ); + assertEquals( + "Failed to parse mapping [_doc]: Duplicate dimension [numeric_dv] present as part star tree index field [startree-1]", + ex.getMessage() + ); + } + + public void testDuplicateMetrics() { + XContentBuilder finalMapping = getMappingWithDuplicateFields(false, true); + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(finalMapping).setSettings(settings).get() + ); + assertEquals( + "Failed to parse mapping [_doc]: Duplicate metrics [numeric_dv] present as part star tree index field [startree-1]", + ex.getMessage() + ); + } + + public void testValidTranslogFlushThresholdSize() { + Settings indexSettings = Settings.builder() + .put(settings) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(256, ByteSizeUnit.MB)) + .build(); + + AcknowledgedResponse response = prepareCreate(TEST_INDEX).setSettings(indexSettings) + .setMapping(createMinimalTestMapping(false, false, false)) + .get(); + + assertTrue(response.isAcknowledged()); + } + + public void testInvalidTranslogFlushThresholdSize() { + Settings indexSettings = Settings.builder() + .put(settings) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(1024, ByteSizeUnit.MB)) + .build(); + + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> prepareCreate(TEST_INDEX).setSettings(indexSettings).setMapping(createMinimalTestMapping(false, false, false)).get() + ); + + assertTrue( + ex.getMessage().contains("You can configure 'index.translog.flush_threshold_size' with upto '512mb' for composite index") + ); + } + + public void testTranslogFlushThresholdSizeWithDefaultCompositeSettingLow() { + Settings updatedSettings = Settings.builder() + .put(CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "130m") + .build(); + + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest().transientSettings(updatedSettings); + + client().admin().cluster().updateSettings(updateSettingsRequest).actionGet(); + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get() + ); + + assertEquals("You can configure 'index.translog.flush_threshold_size' with upto '130mb' for composite index", ex.getMessage()); + } + + public void testUpdateTranslogFlushThresholdSize() { + prepareCreate(TEST_INDEX).setSettings(settings).setMapping(createMinimalTestMapping(false, false, false)).get(); + + // Update to a valid value + AcknowledgedResponse validUpdateResponse = client().admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings(Settings.builder().put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "256mb")) + .get(); + assertTrue(validUpdateResponse.isAcknowledged()); + + // Try to update to an invalid value + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> client().admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings(Settings.builder().put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "1024mb")) + .get() + ); + + assertTrue( + ex.getMessage().contains("You can configure 'index.translog.flush_threshold_size' with upto '512mb' for composite index") + ); + + // update cluster settings to higher value + Settings updatedSettings = Settings.builder() + .put(CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "1030m") + .build(); + + ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest().transientSettings(updatedSettings); + + client().admin().cluster().updateSettings(updateSettingsRequest).actionGet(); + + // update index threshold flush to higher value + validUpdateResponse = client().admin() + .indices() + .prepareUpdateSettings(TEST_INDEX) + .setSettings(Settings.builder().put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "1024mb")) + .get(); + assertTrue(validUpdateResponse.isAcknowledged()); + } + + public void testMinimumTranslogFlushThresholdSize() { + Settings indexSettings = Settings.builder() + .put(settings) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(56, ByteSizeUnit.BYTES)) + .build(); + + AcknowledgedResponse response = prepareCreate(TEST_INDEX).setSettings(indexSettings) + .setMapping(createMinimalTestMapping(false, false, false)) + .get(); + + assertTrue(response.isAcknowledged()); + } + + public void testBelowMinimumTranslogFlushThresholdSize() { + Settings indexSettings = Settings.builder() + .put(settings) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(55, ByteSizeUnit.BYTES)) + .build(); + + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> prepareCreate(TEST_INDEX).setSettings(indexSettings).setMapping(createMinimalTestMapping(false, false, false)).get() + ); + + assertEquals("failed to parse value [55b] for setting [index.translog.flush_threshold_size], must be >= [56b]", ex.getMessage()); + } + @After public final void cleanupNodeSettings() { assertAcked( diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index 9764d840ecc64..0931ca8216556 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -82,6 +82,7 @@ import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.Strings; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; @@ -89,7 +90,9 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexService; import org.opensearch.index.IndexSettings; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.compositeindex.CompositeIndexValidator; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.mapper.DocumentMapper; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.mapper.MapperService.MergeReason; @@ -154,6 +157,7 @@ import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING; import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findContextTemplateName; import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; +import static org.opensearch.index.IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; @@ -1079,6 +1083,7 @@ static Settings aggregateIndexSettings( validateTranslogRetentionSettings(indexSettings); validateStoreTypeSettings(indexSettings); validateRefreshIntervalSettings(request.settings(), clusterSettings); + validateTranslogFlushIntervalSettingsForCompositeIndex(request.settings(), clusterSettings); validateTranslogDurabilitySettings(request.settings(), clusterSettings, settings); return indexSettings; } @@ -1766,6 +1771,71 @@ public static void validateTranslogRetentionSettings(Settings indexSettings) { } } + /** + * Validates {@code index.translog.flush_threshold_size} is equal or below the {@code indices.composite_index.translog.max_flush_threshold_size} + * for composite indices based on {{@code index.composite_index}} + * + * @param requestSettings settings passed in during index create/update request + * @param clusterSettings cluster setting + */ + public static void validateTranslogFlushIntervalSettingsForCompositeIndex(Settings requestSettings, ClusterSettings clusterSettings) { + if (StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.exists(requestSettings) == false + || requestSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey()) == null) { + return; + } + ByteSizeValue translogFlushSize = INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.get(requestSettings); + ByteSizeValue compositeIndexMaxFlushSize = clusterSettings.get( + CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING + ); + if (translogFlushSize.compareTo(compositeIndexMaxFlushSize) > 0) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "You can configure '%s' with upto '%s' for composite index", + INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), + compositeIndexMaxFlushSize + ) + ); + } + } + + /** + * Validates {@code index.translog.flush_threshold_size} is equal or below the {@code indices.composite_index.translog.max_flush_threshold_size} + * for composite indices based on {{@code index.composite_index}} + * This is used during update index settings flow + * + * @param requestSettings settings passed in during index update request + * @param clusterSettings cluster setting + * @param indexSettings index settings + */ + public static Optional validateTranslogFlushIntervalSettingsForCompositeIndex( + Settings requestSettings, + ClusterSettings clusterSettings, + Settings indexSettings + ) { + if (INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.exists(requestSettings) == false + || requestSettings.get(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey()) == null + || StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.exists(indexSettings) == false + || indexSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey()) == null) { + return Optional.empty(); + } + ByteSizeValue translogFlushSize = INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.get(requestSettings); + ByteSizeValue compositeIndexMaxFlushSize = clusterSettings.get( + CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING + ); + if (translogFlushSize.compareTo(compositeIndexMaxFlushSize) > 0) { + return Optional.of( + String.format( + Locale.ROOT, + "You can configure '%s' with upto '%s' for composite index", + INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), + compositeIndexMaxFlushSize + ) + ); + } + return Optional.empty(); + } + /** * Validates {@code index.refresh_interval} is equal or below the {@code cluster.minimum.index.refresh_interval}. * diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java index a7b9eba6dbc05..e4afc798cc64d 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -102,6 +102,7 @@ import static org.opensearch.cluster.metadata.MetadataCreateDataStreamService.validateTimestampFieldMapping; import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings; +import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex; import static org.opensearch.common.util.concurrent.ThreadContext.ACTION_ORIGIN_TRANSIENT_NAME; import static org.opensearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED; @@ -1639,6 +1640,7 @@ private void validate(String name, @Nullable Settings settings, List ind // validate index refresh interval and translog durability settings validateRefreshIntervalSettings(settings, clusterService.getClusterSettings()); + validateTranslogFlushIntervalSettingsForCompositeIndex(settings, clusterService.getClusterSettings()); validateTranslogDurabilitySettingsInTemplate(settings, clusterService.getClusterSettings()); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java index 5d4cc6593dba5..7957a808970eb 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java @@ -82,6 +82,7 @@ import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateOverlap; import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings; import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogDurabilitySettings; +import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex; import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findComponentTemplate; import static org.opensearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX; import static org.opensearch.index.IndexSettings.same; @@ -221,6 +222,12 @@ public ClusterState execute(ClusterState currentState) { index.getName() ).ifPresent(validationErrors::add); } + validateTranslogFlushIntervalSettingsForCompositeIndex( + normalizedSettings, + clusterService.getClusterSettings(), + metadata.getSettings() + ).ifPresent(validationErrors::add); + } if (validationErrors.size() > 0) { diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 09ecefcf56efb..22b1d30b586c8 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -768,6 +768,10 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_LOOKBACK_INTERVAL, RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED, + // Composite index settings + CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING, + CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING, + SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED, // WorkloadManagement settings diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 40ec1ec6c7794..70ad0c7aebc0f 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -249,6 +249,8 @@ public final class IndexScopedSettings extends AbstractScopedSettings { StarTreeIndexSettings.DEFAULT_METRICS_LIST, StarTreeIndexSettings.DEFAULT_DATE_INTERVALS, StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING, + StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING, + StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING, IndexSettings.INDEX_CONTEXT_CREATED_VERSION, IndexSettings.INDEX_CONTEXT_CURRENT_VERSION, diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 77e13c9c02ba3..bb98967265455 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -49,6 +49,7 @@ import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.index.Index; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.remote.RemoteStorePathStrategy; import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.index.translog.Translog; @@ -880,6 +881,8 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) { */ private volatile double docIdFuzzySetFalsePositiveProbability; + private final boolean isCompositeIndex; + /** * Returns the default search fields for this index. */ @@ -1042,7 +1045,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING)); setDocIdFuzzySetFalsePositiveProbability(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING)); - + isCompositeIndex = scopedSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING); scopedSettings.addSettingsUpdateConsumer( TieredMergePolicyProvider.INDEX_COMPOUND_FORMAT_SETTING, tieredMergePolicyProvider::setNoCFSRatio @@ -1287,6 +1290,10 @@ public int getNumberOfReplicas() { return settings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, null); } + public boolean isCompositeIndex() { + return isCompositeIndex; + } + /** * Returns true if segment replication is enabled on the index. * diff --git a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java index 014dd22426a10..a29e642d30f05 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java @@ -13,6 +13,8 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.core.common.unit.ByteSizeValue; /** * Cluster level settings for composite indices @@ -37,12 +39,23 @@ public class CompositeIndexSettings { Setting.Property.Dynamic ); + /** + * This sets the max flush threshold size for composite index + */ + public static final Setting COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING = Setting.byteSizeSetting( + "indices.composite_index.translog.max_flush_threshold_size", + new ByteSizeValue(512, ByteSizeUnit.MB), + new ByteSizeValue(128, ByteSizeUnit.MB), + new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES), + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + private volatile boolean starTreeIndexCreationEnabled; public CompositeIndexSettings(Settings settings, ClusterSettings clusterSettings) { this.starTreeIndexCreationEnabled = STAR_TREE_INDEX_ENABLED_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(STAR_TREE_INDEX_ENABLED_SETTING, this::starTreeIndexCreationEnabled); - } private void starTreeIndexCreationEnabled(boolean value) { diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java index ce389a99b3626..e665831b83d93 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java @@ -26,6 +26,7 @@ public class StarTreeIndexSettings { public static int STAR_TREE_MAX_DIMENSIONS_DEFAULT = 10; + public static int STAR_TREE_MAX_BASE_METRICS_DEFAULT = 100; /** * This setting determines the max number of star tree fields that can be part of composite index mapping. For each * star tree field, we will generate associated star tree index. @@ -52,6 +53,19 @@ public class StarTreeIndexSettings { Setting.Property.Final ); + /** + * This setting determines the max number of dimensions that can be part of star tree index field. Number of + * dimensions and associated cardinality has direct effect of star tree index size and query performance. + */ + public static final Setting STAR_TREE_MAX_BASE_METRICS_SETTING = Setting.intSetting( + "index.composite_index.star_tree.field.max_base_metrics", + STAR_TREE_MAX_BASE_METRICS_DEFAULT, + 4, + 100, + Setting.Property.IndexScope, + Setting.Property.Final + ); + /** * This setting determines the max number of date intervals that can be part of star tree date field. */ @@ -108,4 +122,11 @@ public static Rounding.DateTimeUnit getTimeUnit(String expression) { } return DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expression); } + + public static final Setting IS_COMPOSITE_INDEX_SETTING = Setting.boolSetting( + "index.composite_index", + false, + Setting.Property.IndexScope, + Setting.Property.Final + ); } diff --git a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java index b03026d560dbf..50ff816695156 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -661,6 +661,17 @@ private static void parseNonDynamicArray(ParseContext context, ObjectMapper mapp throws IOException { XContentParser parser = context.parser(); XContentParser.Token token; + // block array values for composite index fields + if (context.indexSettings().isCompositeIndex() && context.mapperService().isFieldPartOfCompositeIndex(arrayFieldName)) { + throw new MapperParsingException( + String.format( + Locale.ROOT, + "object mapping for [%s] with array for [%s] cannot be accepted as field is also part of composite index mapping which does not accept arrays", + mapper.name(), + arrayFieldName + ) + ); + } final String[] paths = splitAndValidatePath(lastFieldName); while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { if (token == XContentParser.Token.START_OBJECT) { diff --git a/server/src/main/java/org/opensearch/index/mapper/MapperService.java b/server/src/main/java/org/opensearch/index/mapper/MapperService.java index a20e3884d4bbc..a3f9d75cbb00a 100644 --- a/server/src/main/java/org/opensearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/opensearch/index/mapper/MapperService.java @@ -228,6 +228,7 @@ public enum MergeReason { private final BooleanSupplier idFieldDataEnabled; private volatile Set compositeMappedFieldTypes; + private volatile Set fieldsPartOfCompositeMappings; public MapperService( IndexSettings indexSettings, @@ -543,9 +544,18 @@ private synchronized Map internalMerge(DocumentMapper ma // initialize composite fields post merge this.compositeMappedFieldTypes = getCompositeFieldTypesFromMapper(); + buildCompositeFieldLookup(); return results; } + private void buildCompositeFieldLookup() { + Set fieldsPartOfCompositeMappings = new HashSet<>(); + for (CompositeMappedFieldType fieldType : compositeMappedFieldTypes) { + fieldsPartOfCompositeMappings.addAll(fieldType.fields()); + } + this.fieldsPartOfCompositeMappings = fieldsPartOfCompositeMappings; + } + private boolean assertSerialization(DocumentMapper mapper) { // capture the source now, it may change due to concurrent parsing final CompressedXContent mappingSource = mapper.mappingSource(); @@ -672,6 +682,10 @@ private Set getCompositeFieldTypesFromMapper() { return compositeMappedFieldTypes; } + public boolean isFieldPartOfCompositeIndex(String field) { + return fieldsPartOfCompositeMappings.contains(field); + } + public ObjectMapper getObjectMapper(String name) { return this.mapper == null ? null : this.mapper.objectMappers().get(name); } diff --git a/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java index 533e6ca73d737..dd984373fc9df 100644 --- a/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java @@ -440,6 +440,15 @@ protected static void parseCompositeField( + " feature flag in the JVM options" ); } + if (StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.get(parserContext.getSettings()) == false) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Set '%s' as true as part of index settings to use star tree index", + StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey() + ) + ); + } Iterator> iterator = compositeNode.entrySet().iterator(); if (compositeNode.size() > StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get(parserContext.getSettings())) { throw new IllegalArgumentException( diff --git a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java index e52d6a621e4e8..17c27ef149e54 100644 --- a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java @@ -22,6 +22,7 @@ import org.opensearch.search.lookup.SearchLookup; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -155,8 +156,20 @@ private List buildDimensions(String fieldName, Map ma String.format(Locale.ROOT, "Atleast two dimensions are required to build star tree index field [%s]", fieldName) ); } + Set dimensionFieldNames = new HashSet<>(); for (Object dim : dimList) { - dimensions.add(getDimension(fieldName, dim, context)); + Dimension dimension = getDimension(fieldName, dim, context); + if (dimensionFieldNames.add(dimension.getField()) == false) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Duplicate dimension [%s] present as part star tree index field [%s]", + dimension.getField(), + fieldName + ) + ); + } + dimensions.add(dimension); } } else { throw new MapperParsingException( @@ -223,6 +236,7 @@ private List buildMetrics(String fieldName, Map map, Map } if (metricsFromInput instanceof List) { List metricsList = (List) metricsFromInput; + Set metricFieldNames = new HashSet<>(); for (Object metric : metricsList) { Map metricMap = (Map) metric; String name = (String) XContentMapValues.extractValue(CompositeDataCubeFieldType.NAME, metricMap); @@ -232,7 +246,18 @@ private List buildMetrics(String fieldName, Map map, Map } metricMap.remove(CompositeDataCubeFieldType.NAME); if (objbuilder == null || objbuilder.mappersBuilders == null) { - metrics.add(getMetric(name, metricMap, context)); + Metric metricFromParser = getMetric(name, metricMap, context); + if (metricFieldNames.add(metricFromParser.getField()) == false) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Duplicate metrics [%s] present as part star tree index field [%s]", + metricFromParser.getField(), + fieldName + ) + ); + } + metrics.add(metricFromParser); } else { Optional meticBuilder = findMapperBuilderByName(name, this.objbuilder.mappersBuilders); if (meticBuilder.isEmpty()) { @@ -243,7 +268,18 @@ private List buildMetrics(String fieldName, Map map, Map String.format(Locale.ROOT, "non-numeric field type is associated with star tree metric [%s]", this.name) ); } - metrics.add(getMetric(name, metricMap, context)); + Metric metricFromParser = getMetric(name, metricMap, context); + if (metricFieldNames.add(metricFromParser.getField()) == false) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Duplicate metrics [%s] present as part star tree index field [%s]", + metricFromParser.getField(), + fieldName + ) + ); + } + metrics.add(metricFromParser); DocumentMapperParser.checkNoRemainingFields( metricMap, context.indexVersionCreated(), @@ -254,6 +290,32 @@ private List buildMetrics(String fieldName, Map map, Map } else { throw new MapperParsingException(String.format(Locale.ROOT, "unable to parse metrics for star tree field [%s]", this.name)); } + int numBaseMetrics = 0; + for (Metric metric : metrics) { + for (MetricStat metricStat : metric.getMetrics()) { + if (metricStat.isDerivedMetric() == false) { + numBaseMetrics++; + } + } + } + if (numBaseMetrics > context.getSettings() + .getAsInt( + StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING.getKey(), + StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_DEFAULT + )) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "There cannot be more than [%s] base metrics for star tree field [%s]", + context.getSettings() + .getAsInt( + StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING.getKey(), + StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_DEFAULT + ), + fieldName + ) + ); + } Metric docCountMetric = new Metric(DocCountFieldMapper.NAME, List.of(MetricStat.DOC_COUNT)); metrics.add(docCountMetric); return metrics; diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index 6d44700a8ce54..c81ad933a8757 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -58,6 +58,8 @@ import org.opensearch.env.Environment; import org.opensearch.index.IndexSettings; import org.opensearch.index.codec.CodecService; +import org.opensearch.index.compositeindex.CompositeIndexSettings; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.engine.EngineConfig; import org.opensearch.index.mapper.MapperParsingException; import org.opensearch.index.mapper.MapperService; @@ -2424,6 +2426,19 @@ public void testAsyncTranslogDurabilityBlocked() { assertThat(throwables.get(0), instanceOf(IllegalArgumentException.class)); } + public void testMaxTranslogFlushSizeWithCompositeIndex() { + Settings clusterSettings = Settings.builder() + .put(CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "130m") + .build(); + PutRequest request = new PutRequest("test", "test_replicas"); + request.patterns(singletonList("test_shards_wait*")); + Settings.Builder settingsBuilder = builder().put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), "true") + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "131m"); + request.settings(settingsBuilder.build()); + List throwables = putTemplate(xContentRegistry(), request, clusterSettings); + assertThat(throwables.get(0), instanceOf(IllegalArgumentException.class)); + } + private static List putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) { return putTemplate(xContentRegistry, request, Settings.EMPTY); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/TranslogFlushIntervalSettingsTests.java b/server/src/test/java/org/opensearch/cluster/metadata/TranslogFlushIntervalSettingsTests.java new file mode 100644 index 0000000000000..f54b6cdf1b152 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/metadata/TranslogFlushIntervalSettingsTests.java @@ -0,0 +1,146 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.metadata; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.index.IndexSettings; +import org.opensearch.index.compositeindex.CompositeIndexSettings; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Optional; + +/** + * Tests for translog flush interval settings update with and without composite index + */ +public class TranslogFlushIntervalSettingsTests extends OpenSearchTestCase { + + Settings settings = Settings.builder() + .put(CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "130mb") + .build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + public void testValidSettings() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "50mb") + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .build(); + + // This should not throw an exception + MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex(requestSettings, clusterSettings); + } + + public void testDefaultTranslogFlushSetting() { + Settings requestSettings = Settings.builder().put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true).build(); + + // This should not throw an exception + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex(requestSettings, clusterSettings) + ); + assertEquals("You can configure 'index.translog.flush_threshold_size' with upto '130mb' for composite index", ex.getMessage()); + } + + public void testMissingCompositeIndexSetting() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "50mb") + .build(); + + // This should not throw an exception + MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex(requestSettings, clusterSettings); + } + + public void testNullTranslogFlushSetting() { + Settings requestSettings = Settings.builder() + .putNull(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey()) + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .build(); + + // This should not throw an exception + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex(requestSettings, clusterSettings) + ); + assertEquals("You can configure 'index.translog.flush_threshold_size' with upto '130mb' for composite index", ex.getMessage()); + } + + public void testExceedingMaxFlushSize() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "150mb") + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .build(); + + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex(requestSettings, clusterSettings) + ); + assertEquals("You can configure 'index.translog.flush_threshold_size' with upto '130mb' for composite index", ex.getMessage()); + } + + public void testEqualToMaxFlushSize() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "100mb") + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .build(); + + // This should not throw an exception + MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex(requestSettings, clusterSettings); + } + + public void testUpdateIndexThresholdFlushSize() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "100mb") + .build(); + + Settings indexSettings = Settings.builder().put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true).build(); + + // This should not throw an exception + assertTrue( + MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex( + requestSettings, + clusterSettings, + indexSettings + ).isEmpty() + ); + } + + public void testUpdateFlushSizeAboveThresholdWithCompositeIndex() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "131mb") + .build(); + + Settings indexSettings = Settings.builder().put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true).build(); + + Optional err = MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex( + requestSettings, + clusterSettings, + indexSettings + ); + assertTrue(err.isPresent()); + assertEquals("You can configure 'index.translog.flush_threshold_size' with upto '130mb' for composite index", err.get()); + } + + public void testUpdateFlushSizeAboveThresholdWithoutCompositeIndex() { + Settings requestSettings = Settings.builder() + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "131mb") + .build(); + + Settings indexSettings = Settings.builder().build(); + + // This should not throw an exception + assertTrue( + MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex( + requestSettings, + clusterSettings, + indexSettings + ).isEmpty() + ); + } +} diff --git a/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java b/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java index fa492e1adec0a..4bbefeba0845b 100644 --- a/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java +++ b/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java @@ -33,14 +33,18 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.IndexSettings; import org.opensearch.index.MapperTestUtils; import org.opensearch.index.codec.composite.CompositeIndexFieldInfo; import org.opensearch.index.codec.composite.CompositeIndexReader; import org.opensearch.index.codec.composite.composite99.Composite99Codec; import org.opensearch.index.compositeindex.datacube.startree.StarTreeDocument; import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils; import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues; @@ -213,20 +217,19 @@ private XContentBuilder topMapping(CheckedConsumer } private void createMapperService(XContentBuilder builder) throws IOException { - IndexMetadata indexMetadata = IndexMetadata.builder("test") - .settings( - Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) - ) - .putMapping(builder.toString()) + Settings settings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) .build(); + IndexMetadata indexMetadata = IndexMetadata.builder("test").settings(settings).putMapping(builder.toString()).build(); IndicesModule indicesModule = new IndicesModule(Collections.emptyList()); mapperService = MapperTestUtils.newMapperServiceWithHelperAnalyzer( new NamedXContentRegistry(ClusterModule.getNamedXWriteables()), createTempDir(), - Settings.EMPTY, + settings, indicesModule, "test" ); diff --git a/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java index 504bc622ec12e..ff0533de4fe8d 100644 --- a/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java @@ -37,7 +37,11 @@ import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.common.bytes.BytesArray; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.index.IndexSettings; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.mapper.MapperService.MergeReason; import org.opensearch.index.mapper.ObjectMapper.Dynamic; import org.opensearch.plugins.Plugin; @@ -544,7 +548,11 @@ public void testCompositeFields() throws Exception { final Settings starTreeEnabledSettings = Settings.builder().put(STAR_TREE_INDEX, "true").build(); FeatureFlags.initializeFeatureFlags(starTreeEnabledSettings); - DocumentMapper documentMapper = createIndex("test").mapperService() + Settings settings = Settings.builder() + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + .build(); + DocumentMapper documentMapper = createIndex("test", settings).mapperService() .documentMapperParser() .parse("tweet", new CompressedXContent(mapping)); diff --git a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java index 81454b210d6be..e06d9889ec905 100644 --- a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java @@ -13,6 +13,8 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.core.common.unit.ByteSizeUnit; +import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.compositeindex.CompositeIndexValidator; @@ -24,6 +26,7 @@ import org.opensearch.index.compositeindex.datacube.ReadDimension; import org.opensearch.index.compositeindex.datacube.startree.StarTreeField; import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.junit.After; import org.junit.Before; @@ -35,6 +38,9 @@ import java.util.List; import java.util.Set; +import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.index.IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING; +import static org.opensearch.index.compositeindex.CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING; import static org.hamcrest.Matchers.containsString; /** @@ -52,7 +58,17 @@ public void teardown() { FeatureFlags.initializeFeatureFlags(Settings.EMPTY); } + @Override + protected Settings getIndexSettings() { + return Settings.builder() + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) + .put(SETTINGS) + .build(); + } + public void testValidStarTree() throws IOException { + MapperService mapperService = createMapperService(getExpandedMappingWithJustAvg("status", "size")); Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); for (CompositeMappedFieldType type : compositeFieldTypes) { @@ -82,6 +98,40 @@ public void testValidStarTree() throws IOException { } } + public void testCompositeIndexWithArraysInCompositeField() throws IOException { + DocumentMapper mapper = createDocumentMapper(getExpandedMappingWithJustAvg("status", "status")); + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> mapper.parse(source(b -> b.startArray("status").value(0).value(1).endArray())) + ); + assertEquals( + "object mapping for [_doc] with array for [status] cannot be accepted as field is also part of composite index mapping which does not accept arrays", + ex.getMessage() + ); + ParsedDocument doc = mapper.parse(source(b -> b.startArray("size").value(0).value(1).endArray())); + // 1 intPoint , 1 SNDV field for each value , so 4 in total + assertEquals(4, doc.rootDoc().getFields("size").length); + } + + public void testValidValueForFlushTresholdSizeWithoutCompositeIndex() { + Settings settings = Settings.builder() + .put(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "256mb") + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), false) + .build(); + + assertEquals(new ByteSizeValue(256, ByteSizeUnit.MB), INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.get(settings)); + } + + public void testValidValueForCompositeIndex() { + Settings settings = Settings.builder() + .put(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "256mb") + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), "512mb") + .build(); + + assertEquals(new ByteSizeValue(256, ByteSizeUnit.MB), INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.get(settings)); + } + public void testMetricsWithJustSum() throws IOException { MapperService mapperService = createMapperService(getExpandedMappingWithJustSum("status", "size")); Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); @@ -291,6 +341,24 @@ public void testInvalidSingleDim() { ); } + public void testDuplicateDimensions() { + XContentBuilder finalMapping = getMappingWithDuplicateFields(true, false); + MapperParsingException ex = expectThrows(MapperParsingException.class, () -> createMapperService(finalMapping)); + assertEquals( + "Failed to parse mapping [_doc]: Duplicate dimension [numeric_dv] present as part star tree index field [startree-1]", + ex.getMessage() + ); + } + + public void testDuplicateMetrics() { + XContentBuilder finalMapping = getMappingWithDuplicateFields(false, true); + MapperParsingException ex = expectThrows(MapperParsingException.class, () -> createMapperService(finalMapping)); + assertEquals( + "Failed to parse mapping [_doc]: Duplicate metrics [numeric_dv] present as part star tree index field [startree-1]", + ex.getMessage() + ); + } + public void testMetric() { List m1 = new ArrayList<>(); m1.add(MetricStat.MAX); @@ -507,6 +575,56 @@ private XContentBuilder getExpandedMappingWithJustAvg(String dim, String metric) }); } + private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, boolean isDuplicateMetric) { + XContentBuilder mapping = null; + try { + mapping = jsonBuilder().startObject() + .startObject("composite") + .startObject("startree-1") + .field("type", "star_tree") + .startObject("config") + .startArray("ordered_dimensions") + .startObject() + .field("name", "timestamp") + .endObject() + .startObject() + .field("name", "numeric_dv") + .endObject() + .startObject() + .field("name", isDuplicateDim ? "numeric_dv" : "numeric_dv1") // Duplicate dimension + .endObject() + .endArray() + .startArray("metrics") + .startObject() + .field("name", "numeric_dv") + .endObject() + .startObject() + .field("name", isDuplicateMetric ? "numeric_dv" : "numeric_dv1") // Duplicate metric + .endObject() + .endArray() + .endObject() + .endObject() + .endObject() + .startObject("properties") + .startObject("timestamp") + .field("type", "date") + .endObject() + .startObject("numeric_dv") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .startObject("numeric_dv1") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .endObject() + .endObject(); + } catch (IOException e) { + fail("Failed to create mapping: " + e.getMessage()); + } + return mapping; + } + private XContentBuilder getExpandedMappingWithJustSum(String dim, String metric) throws IOException { return topMapping(b -> { b.startObject("composite");