From 4af8a1e31364b21b8a599dd05e6cd95b66a1da52 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Thu, 13 Jun 2024 11:35:14 +0530 Subject: [PATCH 1/4] Star tree mapping changes with feature flag Signed-off-by: Bharathwaj G --- distribution/src/config/opensearch.yml | 4 + .../metadata/MetadataCreateIndexService.java | 5 + .../common/settings/ClusterSettings.java | 6 +- .../common/settings/FeatureFlagSettings.java | 3 +- .../common/settings/IndexScopedSettings.java | 9 + .../opensearch/common/util/FeatureFlags.java | 10 +- .../org/opensearch/index/IndexModule.java | 7 +- .../org/opensearch/index/IndexService.java | 10 +- .../CompositeIndexSettings.java | 55 +++ .../CompositeIndexValidator.java | 77 ++++ .../index/compositeindex/DateDimension.java | 80 ++++ .../index/compositeindex/Dimension.java | 41 ++ .../index/compositeindex/Metric.java | 50 +++ .../index/compositeindex/MetricType.java | 44 ++ .../index/compositeindex/package-info.java | 13 + .../startree/StarTreeField.java | 77 ++++ .../startree/StarTreeFieldSpec.java | 88 ++++ .../startree/StarTreeIndexSettings.java | 101 +++++ .../compositeindex/startree/package-info.java | 11 + .../mapper/CompositeDataCubeFieldType.java | 54 +++ .../mapper/CompositeMappedFieldType.java | 56 +++ .../org/opensearch/index/mapper/Mapper.java | 5 + .../index/mapper/MapperService.java | 17 + .../opensearch/index/mapper/ObjectMapper.java | 106 ++++- .../index/mapper/RootObjectMapper.java | 15 +- .../index/mapper/StarTreeMapper.java | 394 ++++++++++++++++++ .../DefaultCompositeIndexSettings.java | 25 ++ .../org/opensearch/indices/IndicesModule.java | 2 + .../opensearch/indices/IndicesService.java | 9 +- .../main/java/org/opensearch/node/Node.java | 5 +- .../opensearch/index/IndexModuleTests.java | 4 +- .../snapshots/SnapshotResiliencyTests.java | 4 +- 32 files changed, 1373 insertions(+), 14 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/Dimension.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/Metric.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/MetricType.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/package-info.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java create mode 100644 server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml index 10bab9b3fce92..e6aa3bf3173fd 100644 --- a/distribution/src/config/opensearch.yml +++ b/distribution/src/config/opensearch.yml @@ -125,3 +125,7 @@ ${path.logs} # Gates the functionality of enabling Opensearch to use pluggable caches with respective store names via setting. # #opensearch.experimental.feature.pluggable.caching.enabled: false +# +# Gates the functionality of star tree index, which improves the performance of search aggregations. +# +#opensearch.experimental.feature.composite.star_tree.enabled: true 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 16edec112f123..efa78fa2ac0af 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -85,6 +85,7 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.IndexService; import org.opensearch.index.IndexSettings; +import org.opensearch.index.compositeindex.CompositeIndexValidator; import org.opensearch.index.mapper.DocumentMapper; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.mapper.MapperService.MergeReason; @@ -1318,6 +1319,10 @@ private static void updateIndexMappingsAndBuildSortOrder( } } + if (mapperService.isCompositeIndexPresent()) { + CompositeIndexValidator.validate(mapperService, indexService.getCompositeIndexSettings()); + } + if (sourceMetadata == null) { // now that the mapping is merged we can validate the index sort. // we cannot validate for index shrinking since the mapping is empty 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 7ea04acf00415..1ffbc6940aba2 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -115,6 +115,7 @@ import org.opensearch.index.ShardIndexingPressureMemoryManager; import org.opensearch.index.ShardIndexingPressureSettings; import org.opensearch.index.ShardIndexingPressureStore; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.remote.RemoteStorePressureSettings; import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory; import org.opensearch.index.store.remote.filecache.FileCacheSettings; @@ -755,7 +756,10 @@ public void apply(Settings value, Settings current, Settings previous) { RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING, RemoteStoreSettings.CLUSTER_REMOTE_MAX_TRANSLOG_READERS, RemoteStoreSettings.CLUSTER_REMOTE_STORE_TRANSLOG_METADATA, - SearchService.CLUSTER_ALLOW_DERIVED_FIELD_SETTING + SearchService.CLUSTER_ALLOW_DERIVED_FIELD_SETTING, + + // Composite index settings + CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING ) ) ); diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java index 238df1bd90113..b6166f5d3cce1 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -37,6 +37,7 @@ protected FeatureFlagSettings( FeatureFlags.TIERED_REMOTE_INDEX_SETTING, FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, FeatureFlags.PLUGGABLE_CACHE_SETTING, - FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL_SETTING + FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL_SETTING, + FeatureFlags.STAR_TREE_INDEX_SETTING ); } 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 6fe8dec9c21b1..54ab787777d94 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -51,6 +51,7 @@ import org.opensearch.index.SearchSlowLog; import org.opensearch.index.TieredMergePolicyProvider; import org.opensearch.index.cache.bitset.BitsetFilterCache; +import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; import org.opensearch.index.engine.EngineConfig; import org.opensearch.index.fielddata.IndexFieldDataService; import org.opensearch.index.mapper.FieldMapper; @@ -238,6 +239,14 @@ public final class IndexScopedSettings extends AbstractScopedSettings { // Settings for concurrent segment search IndexSettings.INDEX_CONCURRENT_SEGMENT_SEARCH_SETTING, IndexSettings.ALLOW_DERIVED_FIELDS, + + // Settings for star tree index + StarTreeIndexSettings.STAR_TREE_DEFAULT_MAX_LEAF_DOCS, + StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING, + StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING, + StarTreeIndexSettings.DEFAULT_METRICS_LIST, + StarTreeIndexSettings.DEFAULT_DATE_INTERVALS, + // validate that built-in similarities don't get redefined Setting.groupSetting("index.similarity.", (s) -> { Map groups = s.getAsGroups(); diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index 6c6e2f2d600f0..c0b61f9cc278d 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -100,6 +100,13 @@ public class FeatureFlags { Property.NodeScope ); + /** + * Gates the functionality of star tree index, which improves the performance of search + * aggregations. + */ + public static final String STAR_TREE_INDEX = "opensearch.experimental.feature.composite.star_tree.enabled"; + public static final Setting STAR_TREE_INDEX_SETTING = Setting.boolSetting(STAR_TREE_INDEX, false, Property.NodeScope); + private static final List> ALL_FEATURE_FLAG_SETTINGS = List.of( REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, EXTENSIONS_SETTING, @@ -108,7 +115,8 @@ public class FeatureFlags { DATETIME_FORMATTER_CACHING_SETTING, TIERED_REMOTE_INDEX_SETTING, PLUGGABLE_CACHE_SETTING, - REMOTE_PUBLICATION_EXPERIMENTAL_SETTING + REMOTE_PUBLICATION_EXPERIMENTAL_SETTING, + STAR_TREE_INDEX_SETTING ); /** * Should store the settings from opensearch.yml. diff --git a/server/src/main/java/org/opensearch/index/IndexModule.java b/server/src/main/java/org/opensearch/index/IndexModule.java index 3c4cb4fd596c1..aaec6bfec2123 100644 --- a/server/src/main/java/org/opensearch/index/IndexModule.java +++ b/server/src/main/java/org/opensearch/index/IndexModule.java @@ -66,6 +66,7 @@ import org.opensearch.index.cache.query.DisabledQueryCache; import org.opensearch.index.cache.query.IndexQueryCache; import org.opensearch.index.cache.query.QueryCache; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.engine.Engine; import org.opensearch.index.engine.EngineConfigFactory; import org.opensearch.index.engine.EngineFactory; @@ -606,7 +607,8 @@ public IndexService newIndexService( BiFunction translogFactorySupplier, Supplier clusterDefaultRefreshIntervalSupplier, RecoverySettings recoverySettings, - RemoteStoreSettings remoteStoreSettings + RemoteStoreSettings remoteStoreSettings, + CompositeIndexSettings compositeIndexSettings ) throws IOException { final IndexEventListener eventListener = freeze(); Function> readerWrapperFactory = indexReaderWrapper @@ -665,7 +667,8 @@ public IndexService newIndexService( translogFactorySupplier, clusterDefaultRefreshIntervalSupplier, recoverySettings, - remoteStoreSettings + remoteStoreSettings, + compositeIndexSettings ); success = true; return indexService; diff --git a/server/src/main/java/org/opensearch/index/IndexService.java b/server/src/main/java/org/opensearch/index/IndexService.java index e501d7eff3f81..54606a0f5d96f 100644 --- a/server/src/main/java/org/opensearch/index/IndexService.java +++ b/server/src/main/java/org/opensearch/index/IndexService.java @@ -72,6 +72,7 @@ import org.opensearch.index.cache.IndexCache; import org.opensearch.index.cache.bitset.BitsetFilterCache; import org.opensearch.index.cache.query.QueryCache; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.engine.Engine; import org.opensearch.index.engine.EngineConfigFactory; import org.opensearch.index.engine.EngineFactory; @@ -188,6 +189,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust private final Supplier clusterDefaultRefreshIntervalSupplier; private final RecoverySettings recoverySettings; private final RemoteStoreSettings remoteStoreSettings; + private final CompositeIndexSettings compositeIndexSettings; public IndexService( IndexSettings indexSettings, @@ -223,7 +225,8 @@ public IndexService( BiFunction translogFactorySupplier, Supplier clusterDefaultRefreshIntervalSupplier, RecoverySettings recoverySettings, - RemoteStoreSettings remoteStoreSettings + RemoteStoreSettings remoteStoreSettings, + CompositeIndexSettings compositeIndexSettings ) { super(indexSettings); this.allowExpensiveQueries = allowExpensiveQueries; @@ -301,6 +304,7 @@ public IndexService( this.translogFactorySupplier = translogFactorySupplier; this.recoverySettings = recoverySettings; this.remoteStoreSettings = remoteStoreSettings; + this.compositeIndexSettings = compositeIndexSettings; updateFsyncTaskIfNecessary(); } @@ -1020,6 +1024,10 @@ private void rescheduleRefreshTasks() { } } + public CompositeIndexSettings getCompositeIndexSettings() { + return compositeIndexSettings; + } + /** * Shard Store Deleter Interface * diff --git a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java new file mode 100644 index 0000000000000..ee55a13c18e1f --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java @@ -0,0 +1,55 @@ +/* + * 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.index.compositeindex; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; + +/** + * Cluster level settings for composite indices + * + * @opensearch.experimental + */ +@ExperimentalApi +public class CompositeIndexSettings { + public static final Setting STAR_TREE_INDEX_ENABLED_SETTING = Setting.boolSetting( + "indices.composite.star_tree.enabled", + false, + value -> { + if (FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING) == false && value == true) { + throw new IllegalArgumentException( + "star tree index is under an experimental feature and can be activated only by enabling " + + FeatureFlags.STAR_TREE_INDEX_SETTING.getKey() + + " feature flag in the JVM options" + ); + } + }, + 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) { + this.starTreeIndexCreationEnabled = value; + } + + public boolean isStarTreeIndexCreationEnabled() { + return starTreeIndexCreationEnabled; + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java new file mode 100644 index 0000000000000..b59bae0092a19 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java @@ -0,0 +1,77 @@ +/* + * 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.index.compositeindex; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.mapper.CompositeDataCubeFieldType; +import org.opensearch.index.mapper.CompositeMappedFieldType; +import org.opensearch.index.mapper.MappedFieldType; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.index.mapper.StarTreeMapper; + +import java.util.Locale; +import java.util.Set; + +/** + * Validation for composite indices + * + * @opensearch.experimental + */ +@ExperimentalApi +public class CompositeIndexValidator { + + public static void validate(MapperService mapperService, CompositeIndexSettings compositeIndexSettings) { + validateStarTreeMappings(mapperService, compositeIndexSettings); + } + + private static void validateStarTreeMappings(MapperService mapperService, CompositeIndexSettings compositeIndexSettings) { + Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); + for (CompositeMappedFieldType compositeFieldType : compositeFieldTypes) { + if (!(compositeFieldType instanceof StarTreeMapper.StarTreeFieldType)) { + return; + } + if (!compositeIndexSettings.isStarTreeIndexCreationEnabled()) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "star tree index cannot be created, enable it using [%s] setting", + CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING.getKey() + ) + ); + } + CompositeDataCubeFieldType dataCubeFieldType = (CompositeDataCubeFieldType) compositeFieldType; + for (Dimension dim : dataCubeFieldType.getDimensions()) { + MappedFieldType ft = mapperService.fieldType(dim.getField()); + if (!ft.isAggregatable()) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Aggregations not supported for the dimension field [%s] with field type [%s] as part of star tree field", + dim.getField(), + ft.typeName() + ) + ); + } + } + for (Metric metric : dataCubeFieldType.getMetrics()) { + MappedFieldType ft = mapperService.fieldType(metric.getField()); + if (!ft.isAggregatable()) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Aggregations not supported for the metrics field [%s] with field type [%s] as part of star tree field", + metric.getField(), + ft.typeName() + ) + ); + } + } + } + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java b/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java new file mode 100644 index 0000000000000..1dd764ac97df0 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java @@ -0,0 +1,80 @@ +/* + * 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.index.compositeindex; + +import org.opensearch.common.Rounding; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.common.xcontent.support.XContentMapValues; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; +import org.opensearch.index.mapper.Mapper; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Date dimension class + * + * @opensearch.experimental + */ +@ExperimentalApi +public class DateDimension extends Dimension { + private final List calendarIntervals; + + public DateDimension(String name, List intervals) { + super(name); + this.calendarIntervals = intervals; + } + + @SuppressWarnings("unchecked") + public DateDimension(Map.Entry dimension, Mapper.TypeParser.ParserContext c) { + super(dimension.getKey()); + List intervalStrings = XContentMapValues.extractRawValues("calendar_interval", (Map) dimension.getValue()) + .stream() + .map(Object::toString) + .collect(Collectors.toList()); + if (intervalStrings == null || intervalStrings.isEmpty()) { + this.calendarIntervals = StarTreeIndexSettings.DEFAULT_DATE_INTERVALS.get(c.getSettings()); + } else { + if (intervalStrings.size() > 3) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "At most 3 calendar intervals are allowed in dimension [%s]", dimension.getKey()) + ); + } + Set calendarIntervals = new HashSet<>(); + for (String interval : intervalStrings) { + calendarIntervals.add(StarTreeIndexSettings.getTimeUnit(interval)); + } + this.calendarIntervals = new ArrayList<>(calendarIntervals); + } + } + + public List getIntervals() { + return calendarIntervals; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(this.getField()); + builder.field("type", "date"); + builder.startArray("calendar_interval"); + for (Rounding.DateTimeUnit interval : calendarIntervals) { + builder.value(interval.shortName()); + } + builder.endArray(); + builder.endObject(); + return builder; + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/Dimension.java b/server/src/main/java/org/opensearch/index/compositeindex/Dimension.java new file mode 100644 index 0000000000000..426508b05b59a --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/Dimension.java @@ -0,0 +1,41 @@ +/* + * 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.index.compositeindex; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; + +import java.io.IOException; + +/** + * Composite index dimension base class + * + * @opensearch.experimental + */ +@ExperimentalApi +public class Dimension implements ToXContent { + private final String field; + + public Dimension(String field) { + this.field = field; + } + + public String getField() { + return field; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(field); + builder.field("type", "numeric"); + builder.endObject(); + return builder; + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/Metric.java b/server/src/main/java/org/opensearch/index/compositeindex/Metric.java new file mode 100644 index 0000000000000..bde16f305bba7 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/Metric.java @@ -0,0 +1,50 @@ +/* + * 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.index.compositeindex; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.List; + +/** + * Holds details of metrics field as part of composite field + */ +@ExperimentalApi +public class Metric implements ToXContent { + private final String field; + private final List metrics; + + public Metric(String field, List metrics) { + this.field = field; + this.metrics = metrics; + } + + public String getField() { + return field; + } + + public List getMetrics() { + return metrics; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(field); + builder.startArray("metrics"); + for (MetricType metricType : metrics) { + builder.value(metricType.getTypeName()); + } + builder.endArray(); + builder.endObject(); + return builder; + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/MetricType.java b/server/src/main/java/org/opensearch/index/compositeindex/MetricType.java new file mode 100644 index 0000000000000..1a39e9f1a5870 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/MetricType.java @@ -0,0 +1,44 @@ +/* + * 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.index.compositeindex; + +import org.opensearch.common.annotation.ExperimentalApi; + +/** + * Supported metric types for composite index + * + * @opensearch.experimental + */ +@ExperimentalApi +public enum MetricType { + COUNT("count"), + AVG("avg"), + SUM("sum"), + MIN("min"), + MAX("max"); + + private final String typeName; + + MetricType(String typeName) { + this.typeName = typeName; + } + + public String getTypeName() { + return typeName; + } + + public static MetricType fromTypeName(String typeName) { + for (MetricType metric : MetricType.values()) { + if (metric.getTypeName().equalsIgnoreCase(typeName)) { + return metric; + } + } + throw new IllegalArgumentException("Invalid metric type: " + typeName); + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/package-info.java b/server/src/main/java/org/opensearch/index/compositeindex/package-info.java new file mode 100644 index 0000000000000..59f18efec26b1 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/package-info.java @@ -0,0 +1,13 @@ +/* + * 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. + */ + +/** + * Core classes for handling composite indices. + * @opensearch.experimental + */ +package org.opensearch.index.compositeindex; diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java new file mode 100644 index 0000000000000..a29f049e1f8fb --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java @@ -0,0 +1,77 @@ +/* + * 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.index.compositeindex.startree; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.compositeindex.Dimension; +import org.opensearch.index.compositeindex.Metric; + +import java.io.IOException; +import java.util.List; + +/** + * Star tree field which contains dimensions, metrics and specs + * + * @opensearch.experimental + */ +@ExperimentalApi +public class StarTreeField implements ToXContent { + private final String name; + private final List dimensionsOrder; + private final List metrics; + private final StarTreeFieldSpec starTreeFieldSpec; + + public StarTreeField(String name, List dimensions, List metrics, StarTreeFieldSpec starTreeFieldSpec) { + this.name = name; + this.dimensionsOrder = dimensions; + this.metrics = metrics; + this.starTreeFieldSpec = starTreeFieldSpec; + } + + public String getName() { + return name; + } + + public List getDimensionsOrder() { + return dimensionsOrder; + } + + public List getMetrics() { + return metrics; + } + + public StarTreeFieldSpec getSpec() { + return starTreeFieldSpec; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field("name", name); + if (dimensionsOrder != null && !dimensionsOrder.isEmpty()) { + builder.startObject("ordered_dimensions"); + for (Dimension dimension : dimensionsOrder) { + dimension.toXContent(builder, params); + } + builder.endObject(); + } + if (metrics != null && !metrics.isEmpty()) { + builder.startObject("metrics"); + for (Metric metric : metrics) { + metric.toXContent(builder, params); + } + builder.endObject(); + } + starTreeFieldSpec.toXContent(builder, params); + builder.endObject(); + return builder; + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java new file mode 100644 index 0000000000000..a7dbd860bd8bc --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java @@ -0,0 +1,88 @@ +/* + * 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.index.compositeindex.startree; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Star tree index specific configuration + * + * @opensearch.experimental + */ +@ExperimentalApi +public class StarTreeFieldSpec implements ToXContent { + + private final AtomicInteger maxLeafDocs = new AtomicInteger(); + private final List skipStarNodeCreationInDims; + private final StarTreeBuildMode buildMode; + + public StarTreeFieldSpec(int maxLeafDocs, List skipStarNodeCreationInDims, StarTreeBuildMode buildMode) { + this.maxLeafDocs.set(maxLeafDocs); + this.skipStarNodeCreationInDims = skipStarNodeCreationInDims; + this.buildMode = buildMode; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.field("max_leaf_docs", maxLeafDocs.get()); + builder.field("build_mode", buildMode.getTypeName()); + builder.startArray("skip_star_node_creation_for_dimensions"); + for (String dim : skipStarNodeCreationInDims) { + builder.value(dim); + } + builder.endArray(); + return builder; + } + + /** + * Star tree build mode using which sorting and aggregations are performed during index creation. + * + * @opensearch.experimental + */ + @ExperimentalApi + public enum StarTreeBuildMode { + ON_HEAP("onheap"), + OFF_HEAP("offheap"); + + private final String typeName; + + StarTreeBuildMode(String typeName) { + this.typeName = typeName; + } + + public String getTypeName() { + return typeName; + } + + public static StarTreeBuildMode fromTypeName(String typeName) { + for (StarTreeBuildMode starTreeBuildMode : StarTreeBuildMode.values()) { + if (starTreeBuildMode.getTypeName().equalsIgnoreCase(typeName)) { + return starTreeBuildMode; + } + } + throw new IllegalArgumentException(String.format(Locale.ROOT, "Invalid star tree build mode: [%s] ", typeName)); + } + } + + @Override + public String toString() { + return buildMode.getTypeName(); + } + + public int maxLeafDocs() { + return maxLeafDocs.get(); + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java new file mode 100644 index 0000000000000..601cf8087f5f2 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java @@ -0,0 +1,101 @@ +/* + * 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.index.compositeindex.startree; + +import org.opensearch.common.Rounding; +import org.opensearch.common.settings.Setting; +import org.opensearch.index.compositeindex.MetricType; +import org.opensearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; + +import java.util.Arrays; +import java.util.List; + +/** + * Index settings for star tree fields + * + * @opensearch.experimental + */ +public class StarTreeIndexSettings { + /** + * 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. + */ + public static final Setting STAR_TREE_MAX_FIELDS_SETTING = Setting.intSetting( + "index.composite.star_tree.max_fields", + 1, + 1, + 1, + Setting.Property.IndexScope, + 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_DIMENSIONS_SETTING = Setting.intSetting( + "index.composite.star_tree.field.max_dimensions", + 10, + 2, + 10, + Setting.Property.IndexScope, + Setting.Property.Final + ); + + /** + * This setting configures the default "maxLeafDocs" setting of star tree. This affects both query performance and + * star tree index size. Lesser the leaves, better the query latency but higher storage size and vice versa + *

+ * We can remove this later or change it to an enum based constant setting. + * + * @opensearch.experimental + */ + public static final Setting STAR_TREE_DEFAULT_MAX_LEAF_DOCS = Setting.intSetting( + "index.composite.star_tree.default.max_leaf_docs", + 10000, + 1, + Setting.Property.IndexScope, + Setting.Property.Final + ); + + /** + * Default intervals for date dimension as part of star tree fields + */ + public static final Setting> DEFAULT_DATE_INTERVALS = Setting.listSetting( + "index.composite.star_tree.field.default.date_intervals", + Arrays.asList(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), Rounding.DateTimeUnit.HOUR_OF_DAY.shortName()), + StarTreeIndexSettings::getTimeUnit, + Setting.Property.IndexScope, + Setting.Property.Final + ); + + /** + * Default metrics for metrics as part of star tree fields + */ + public static final Setting> DEFAULT_METRICS_LIST = Setting.listSetting( + "index.composite.star_tree.field.default.metrics", + Arrays.asList( + MetricType.AVG.toString(), + MetricType.COUNT.toString(), + MetricType.SUM.toString(), + MetricType.MAX.toString(), + MetricType.MIN.toString() + ), + MetricType::fromTypeName, + Setting.Property.IndexScope, + Setting.Property.Final + ); + + public static Rounding.DateTimeUnit getTimeUnit(String expression) { + if (!DateHistogramAggregationBuilder.DATE_FIELD_UNITS.containsKey(expression)) { + throw new IllegalArgumentException("unknown calendar interval specified in star tree index config"); + } + return DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expression); + } +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java new file mode 100644 index 0000000000000..988bca357f32b --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java @@ -0,0 +1,11 @@ +/* + * 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. + */ +/** + * Core classes for handling star tree index. + */ +package org.opensearch.index.compositeindex.startree; diff --git a/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java b/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java new file mode 100644 index 0000000000000..99f94cd38b65c --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java @@ -0,0 +1,54 @@ +/* + * 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.index.mapper; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.compositeindex.Dimension; +import org.opensearch.index.compositeindex.Metric; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Base class for multi field data cube fields + * + * @opensearch.experimental + */ +@ExperimentalApi +public abstract class CompositeDataCubeFieldType extends CompositeMappedFieldType { + private final List dimensions; + private final List metrics; + + public CompositeDataCubeFieldType(String name, List dims, List metrics, CompositeFieldType type) { + super(name, getFields(dims, metrics), type); + this.dimensions = dims; + this.metrics = metrics; + } + + private static List getFields(List dims, List metrics) { + Set fields = new HashSet<>(); + for (Dimension dim : dims) { + fields.add(dim.getField()); + } + for (Metric metric : metrics) { + fields.add(metric.getField()); + } + return new ArrayList<>(fields); + } + + public List getDimensions() { + return dimensions; + } + + public List getMetrics() { + return metrics; + } +} diff --git a/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java b/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java new file mode 100644 index 0000000000000..d66ef0c02cc4a --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java @@ -0,0 +1,56 @@ +/* + * 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.index.mapper; + +import org.opensearch.common.annotation.ExperimentalApi; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +/** + * Base class for composite field types + * + * @opensearch.experimental + */ +@ExperimentalApi +public abstract class CompositeMappedFieldType extends MappedFieldType { + private final List fields; + private final CompositeFieldType type; + + public CompositeMappedFieldType( + String name, + boolean isIndexed, + boolean isStored, + boolean hasDocValues, + TextSearchInfo textSearchInfo, + Map meta, + List fields, + CompositeFieldType type + ) { + super(name, isIndexed, isStored, hasDocValues, textSearchInfo, meta); + this.fields = fields; + this.type = type; + } + + public CompositeMappedFieldType(String name, List fields, CompositeFieldType type) { + this(name, false, false, false, TextSearchInfo.NONE, Collections.emptyMap(), fields, type); + } + + /** + * Supported composite field types + */ + public enum CompositeFieldType { + STAR_TREE + } + + public List fields() { + return fields; + } +} diff --git a/server/src/main/java/org/opensearch/index/mapper/Mapper.java b/server/src/main/java/org/opensearch/index/mapper/Mapper.java index bd5d3f15c0706..46a5050d4fc18 100644 --- a/server/src/main/java/org/opensearch/index/mapper/Mapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/Mapper.java @@ -253,6 +253,11 @@ public boolean isWithinMultiField() { } Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException; + + default Mapper.Builder parse(String name, Map node, ParserContext parserContext, ObjectMapper.Builder objBuilder) + throws MapperParsingException { + throw new UnsupportedOperationException("should not be invoked"); + } } private final String simpleName; 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 a1f3894c9f14c..c2e7411a3b47a 100644 --- a/server/src/main/java/org/opensearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/opensearch/index/mapper/MapperService.java @@ -650,6 +650,23 @@ public Iterable fieldTypes() { return this.mapper == null ? Collections.emptySet() : this.mapper.fieldTypes(); } + public boolean isCompositeIndexPresent() { + return this.mapper != null && !getCompositeFieldTypes().isEmpty(); + } + + public Set getCompositeFieldTypes() { + Set compositeMappedFieldTypes = new HashSet<>(); + if (this.mapper == null) { + return Collections.emptySet(); + } + for (MappedFieldType type : this.mapper.fieldTypes()) { + if (type instanceof CompositeMappedFieldType) { + compositeMappedFieldTypes.add((CompositeMappedFieldType) type); + } + } + return compositeMappedFieldTypes; + } + 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 92ffdb60e6cde..103f6e950efe8 100644 --- a/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java @@ -42,9 +42,11 @@ import org.opensearch.common.collect.CopyOnWriteHashMap; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; import org.opensearch.index.mapper.MapperService.MergeReason; import java.io.IOException; @@ -176,6 +178,7 @@ public void setIncludeInRoot(boolean value) { * @opensearch.internal */ @SuppressWarnings("rawtypes") + @PublicApi(since = "1.0.0") public static class Builder extends Mapper.Builder { protected Explicit enabled = new Explicit<>(true, false); @@ -262,14 +265,25 @@ public static class TypeParser implements Mapper.TypeParser { public Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { ObjectMapper.Builder builder = new Builder(name); parseNested(name, node, builder, parserContext); + Object compositeField = null; for (Iterator> iterator = node.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = iterator.next(); String fieldName = entry.getKey(); Object fieldNode = entry.getValue(); - if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder)) { + if (fieldName.equals("composite")) { + compositeField = fieldNode; iterator.remove(); + } else { + if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder)) { + iterator.remove(); + } } } + // Important : Composite field is made up of 2 or more source fields of the index, so this must be called + // after parsing all other properties + if (compositeField != null) { + parseCompositeField(builder, (Map) compositeField, parserContext); + } return builder; } @@ -407,6 +421,96 @@ protected static void parseDerived(ObjectMapper.Builder objBuilder, Map compositeNode, + ParserContext parserContext + ) { + if (!FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING)) { + throw new IllegalArgumentException( + "star tree index is under an experimental feature and can be activated only by enabling " + + FeatureFlags.STAR_TREE_INDEX_SETTING.getKey() + + " feature flag in the JVM options" + ); + } + Iterator> iterator = compositeNode.entrySet().iterator(); + if (compositeNode.size() > StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get(parserContext.getSettings())) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Composite fields cannot have more than [%s] fields", + StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get(parserContext.getSettings()) + ) + ); + } + while (iterator.hasNext()) { + Map.Entry entry = iterator.next(); + String fieldName = entry.getKey(); + // Should accept empty arrays, as a work around for when the + // user can't provide an empty Map. (PHP for example) + boolean isEmptyList = entry.getValue() instanceof List && ((List) entry.getValue()).isEmpty(); + if (entry.getValue() instanceof Map) { + @SuppressWarnings("unchecked") + Map propNode = (Map) entry.getValue(); + String type; + Object typeNode = propNode.get("type"); + if (typeNode != null) { + type = typeNode.toString(); + } else { + // lets see if we can derive this... + throw new MapperParsingException("No type specified for field [" + fieldName + "]"); + } + Mapper.TypeParser typeParser = getSupportedCompositeTypeParser(type, parserContext); + if (typeParser == null) { + throw new MapperParsingException("No handler for type [" + type + "] declared on field [" + fieldName + "]"); + } + String[] fieldNameParts = fieldName.split("\\."); + // field name is just ".", which is invalid + if (fieldNameParts.length < 1) { + throw new MapperParsingException("Invalid field name " + fieldName); + } + String realFieldName = fieldNameParts[fieldNameParts.length - 1]; + Mapper.Builder fieldBuilder = typeParser.parse(realFieldName, propNode, parserContext, objBuilder); + for (int i = fieldNameParts.length - 2; i >= 0; --i) { + ObjectMapper.Builder intermediate = new ObjectMapper.Builder<>(fieldNameParts[i]); + intermediate.add(fieldBuilder); + fieldBuilder = intermediate; + } + objBuilder.add(fieldBuilder); + propNode.remove("type"); + DocumentMapperParser.checkNoRemainingFields(fieldName, propNode, parserContext.indexVersionCreated()); + iterator.remove(); + } else if (isEmptyList) { + iterator.remove(); + } else { + throw new MapperParsingException( + "Expected map for property [fields] on field [" + fieldName + "] but got a " + fieldName.getClass() + ); + } + } + + DocumentMapperParser.checkNoRemainingFields( + compositeNode, + parserContext.indexVersionCreated(), + "DocType mapping definition has unsupported parameters: " + ); + } + + private static Mapper.TypeParser getSupportedCompositeTypeParser(String type, ParserContext parserContext) { + switch (type) { + case StarTreeMapper.CONTENT_TYPE: + return parserContext.typeParser(type); + default: + throw new IllegalArgumentException( + String.format(Locale.ROOT, "Type [%s] isn't supported in composite field context.", type) + ); + } + } + protected static void parseProperties(ObjectMapper.Builder objBuilder, Map propsNode, ParserContext parserContext) { Iterator> iterator = propsNode.entrySet().iterator(); while (iterator.hasNext()) { diff --git a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java index 9504e6eafc046..e06e5be4633f9 100644 --- a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java @@ -177,15 +177,26 @@ public Mapper.Builder parse(String name, Map node, ParserContext RootObjectMapper.Builder builder = new Builder(name); Iterator> iterator = node.entrySet().iterator(); + Object compositeField = null; while (iterator.hasNext()) { Map.Entry entry = iterator.next(); String fieldName = entry.getKey(); Object fieldNode = entry.getValue(); - if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder) - || processField(builder, fieldName, fieldNode, parserContext)) { + if (fieldName.equals("composite")) { + compositeField = fieldNode; iterator.remove(); + } else { + if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder) + || processField(builder, fieldName, fieldNode, parserContext)) { + iterator.remove(); + } } } + // Important : Composite field is made up of 2 or more source properties of the index, so this must be called + // after parsing all other properties + if (compositeField != null) { + parseCompositeField(builder, (Map) compositeField, parserContext); + } return builder; } diff --git a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java new file mode 100644 index 0000000000000..528b56458b592 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java @@ -0,0 +1,394 @@ +/* + * 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.index.mapper; + +import org.apache.lucene.search.Query; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.common.xcontent.support.XContentMapValues; +import org.opensearch.index.compositeindex.DateDimension; +import org.opensearch.index.compositeindex.Dimension; +import org.opensearch.index.compositeindex.Metric; +import org.opensearch.index.compositeindex.MetricType; +import org.opensearch.index.compositeindex.startree.StarTreeField; +import org.opensearch.index.compositeindex.startree.StarTreeFieldSpec; +import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; +import org.opensearch.index.query.QueryShardContext; +import org.opensearch.search.lookup.SearchLookup; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * A field mapper for star tree fields + * + * @opensearch.experimental + */ +@ExperimentalApi +public class StarTreeMapper extends ParametrizedFieldMapper { + public static final String CONTENT_TYPE = "star_tree"; + + @Override + public ParametrizedFieldMapper.Builder getMergeBuilder() { + return new Builder(simpleName(), objBuilder).init(this); + + } + + /** + * Builder for the star tree field mapper + * + * @opensearch.internal + */ + public static class Builder extends ParametrizedFieldMapper.Builder { + private ObjectMapper.Builder objbuilder; + private static final Set> ALLOWED_DIMENSION_MAPPER_BUILDERS = Set.of( + NumberFieldMapper.Builder.class, + DateFieldMapper.Builder.class + ); + private static final Set> ALLOWED_METRIC_MAPPER_BUILDERS = Set.of(NumberFieldMapper.Builder.class); + + @SuppressWarnings("unchecked") + private final Parameter config = new Parameter<>("config", false, () -> null, (name, context, nodeObj) -> { + if (nodeObj instanceof Map) { + Map paramMap = (Map) nodeObj; + int maxLeafDocs = XContentMapValues.nodeIntegerValue( + paramMap.get("max_leaf_docs"), + StarTreeIndexSettings.STAR_TREE_DEFAULT_MAX_LEAF_DOCS.get(context.getSettings()) + ); + if (maxLeafDocs < 1) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "max_leaf_docs [%s] must be greater than 0", maxLeafDocs) + ); + } + List skipStarInDims = Arrays.asList( + XContentMapValues.nodeStringArrayValue( + paramMap.getOrDefault("skip_star_node_creation_for_dimensions", new ArrayList()) + ) + ); + StarTreeFieldSpec.StarTreeBuildMode buildMode = StarTreeFieldSpec.StarTreeBuildMode.fromTypeName( + XContentMapValues.nodeStringValue( + paramMap.get("build_mode"), + StarTreeFieldSpec.StarTreeBuildMode.OFF_HEAP.getTypeName() + ) + ); + List dimensions = buildDimensions(paramMap, context); + List metrics = buildMetrics(paramMap, context); + for (String dim : skipStarInDims) { + if (dimensions.stream().filter(d -> d.getField().equals(dim)).findAny().isEmpty()) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "[%s] in skip_star_node_creation_for_dimensions should be part of ordered_dimensions", + dim + ) + ); + } + } + StarTreeFieldSpec spec = new StarTreeFieldSpec(maxLeafDocs, skipStarInDims, buildMode); + return new StarTreeField(this.name, dimensions, metrics, spec); + + } else { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "unable to parse config for star tree field [%s]", this.name) + ); + } + }, m -> toType(m).starTreeField); + + /** + * Build dimensions from mapping + */ + @SuppressWarnings("unchecked") + private List buildDimensions(Map map, Mapper.TypeParser.ParserContext context) { + Object dims = XContentMapValues.extractValue("ordered_dimensions", map); + if (dims == null) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "ordered_dimensions is required for star tree field [%s]", this.name) + ); + } + List dimensions = new ArrayList<>(); + if (dims instanceof LinkedHashMap) { + if (((LinkedHashMap) dims).size() > context.getSettings() + .getAsInt(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 10)) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "ordered_dimensions cannot have more than %s dimensions for star tree field [%s]", + context.getSettings().getAsInt(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 10), + this.name + ) + ); + } + if (((LinkedHashMap) dims).size() < 2) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "Atleast two dimensions are required to build star tree index field [%s]", this.name) + ); + } + for (Map.Entry dim : ((LinkedHashMap) dims).entrySet()) { + if (this.objbuilder == null || this.objbuilder.mappersBuilders == null) { + if (dim.getValue() instanceof Map) { + Map dimObj = ((Map) dim.getValue()); + String type = XContentMapValues.nodeStringValue(dimObj.get("type")); + dimensions.add(getDimension(type, dim, context)); + } else { + throw new MapperParsingException( + String.format(Locale.ROOT, "unable to parse ordered_dimensions for star tree field [%s]", this.name) + ); + } + } else { + Optional dimBuilder = findMapperBuilderByName(dim.getKey(), this.objbuilder.mappersBuilders); + if (dimBuilder.isEmpty()) { + throw new IllegalArgumentException(String.format(Locale.ROOT, "unknown dimension field [%s]", dim.getKey())); + } + if (!isBuilderAllowedForDimension(dimBuilder.get())) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "unsupported field type associated with dimension [%s] as part of star tree field [%s]", + dim.getKey(), + name + ) + ); + } + dimensions.add(getDimension(dimBuilder.get(), dim, context)); + } + } + } else { + throw new MapperParsingException( + String.format(Locale.ROOT, "unable to parse ordered_dimensions for star tree field [%s]", this.name) + ); + } + return dimensions; + } + + /** + * Get dimension instance based on the builder type + */ + private Dimension getDimension(Mapper.Builder builder, Map.Entry dim, Mapper.TypeParser.ParserContext context) { + String name = dim.getKey(); + Dimension dimension; + if (builder instanceof DateFieldMapper.Builder) { + dimension = new DateDimension(dim, context); + } + // Numeric dimension - default + else if (builder instanceof NumberFieldMapper.Builder) { + dimension = new Dimension(name); + } else { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "unsupported field type associated with star tree dimension [%s]", name) + ); + } + return dimension; + } + + /** + * Get dimension based on field type + */ + private Dimension getDimension(String type, Map.Entry dim, Mapper.TypeParser.ParserContext c) { + String name = dim.getKey(); + Dimension dimension; + if (type.equals("date")) { + dimension = new DateDimension(dim, c); + } + // Numeric dimension - default + else { + dimension = new Dimension(name); + } + return dimension; + } + + /** + * Build metrics from mapping + */ + @SuppressWarnings("unchecked") + private List buildMetrics(Map map, Mapper.TypeParser.ParserContext context) { + List metrics = new ArrayList<>(); + Object metricsFromInput = XContentMapValues.extractValue("metrics", map); + if (metricsFromInput == null) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "metrics section is required for star tree field [%s]", this.name) + ); + } + if (metricsFromInput instanceof LinkedHashMap) { + for (Map.Entry metric : ((LinkedHashMap) metricsFromInput).entrySet()) { + if (objbuilder == null || objbuilder.mappersBuilders == null) { + metrics.add(getMetric(metric, context)); + } else { + Optional meticBuilder = findMapperBuilderByName(metric.getKey(), this.objbuilder.mappersBuilders); + if (meticBuilder.isEmpty()) { + throw new IllegalArgumentException(String.format(Locale.ROOT, "unknown metric field [%s]", metric.getKey())); + } + if (!isBuilderAllowedForMetric(meticBuilder.get())) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "non-numeric field type is associated with star tree metric [%s]", this.name) + ); + } + metrics.add(getMetric(metric, context)); + } + } + } else { + throw new MapperParsingException(String.format(Locale.ROOT, "unable to parse metrics for star tree field [%s]", this.name)); + } + + return metrics; + } + + @SuppressWarnings("unchecked") + private Metric getMetric(Map.Entry map, Mapper.TypeParser.ParserContext context) { + String name = map.getKey(); + List metricTypes; + List metricStrings = XContentMapValues.extractRawValues("metrics", (Map) map.getValue()) + .stream() + .map(Object::toString) + .collect(Collectors.toList()); + + if (metricStrings.isEmpty()) { + metricTypes = new ArrayList<>(StarTreeIndexSettings.DEFAULT_METRICS_LIST.get(context.getSettings())); + } else { + Set metricSet = new HashSet<>(); + for (String metricString : metricStrings) { + metricSet.add(MetricType.fromTypeName(metricString)); + } + metricTypes = new ArrayList<>(metricSet); + } + return new Metric(name, metricTypes); + } + + @Override + protected List> getParameters() { + return List.of(config); + } + + private static boolean isBuilderAllowedForDimension(Mapper.Builder builder) { + return ALLOWED_DIMENSION_MAPPER_BUILDERS.stream().anyMatch(allowedType -> allowedType.isInstance(builder)); + } + + private static boolean isBuilderAllowedForMetric(Mapper.Builder builder) { + return ALLOWED_METRIC_MAPPER_BUILDERS.stream().anyMatch(allowedType -> allowedType.isInstance(builder)); + } + + private Optional findMapperBuilderByName(String field, List mappersBuilders) { + return mappersBuilders.stream().filter(builder -> builder.name().equals(field)).findFirst(); + } + + public Builder(String name, ObjectMapper.Builder objBuilder) { + super(name); + this.objbuilder = objBuilder; + } + + @Override + public ParametrizedFieldMapper build(BuilderContext context) { + StarTreeFieldType type = new StarTreeFieldType(name, this.config.get()); + return new StarTreeMapper(name, type, this, objbuilder); + } + } + + private static StarTreeMapper toType(FieldMapper in) { + return (StarTreeMapper) in; + } + + /** + * Concrete parse for star tree type + * + * @opensearch.internal + */ + public static class TypeParser implements Mapper.TypeParser { + + /** + * default constructor of VectorFieldMapper.TypeParser + */ + public TypeParser() {} + + @Override + public Mapper.Builder parse(String name, Map node, ParserContext context) throws MapperParsingException { + Builder builder = new StarTreeMapper.Builder(name, null); + builder.parse(name, context, node); + return builder; + } + + @Override + public Mapper.Builder parse(String name, Map node, ParserContext context, ObjectMapper.Builder objBuilder) + throws MapperParsingException { + Builder builder = new StarTreeMapper.Builder(name, objBuilder); + builder.parse(name, context, node); + return builder; + } + } + + private final StarTreeField starTreeField; + + private final ObjectMapper.Builder objBuilder; + + protected StarTreeMapper(String simpleName, StarTreeFieldType type, Builder builder, ObjectMapper.Builder objbuilder) { + super(simpleName, type, MultiFields.empty(), CopyTo.empty()); + this.starTreeField = builder.config.get(); + this.objBuilder = objbuilder; + } + + @Override + public StarTreeFieldType fieldType() { + return (StarTreeFieldType) super.fieldType(); + } + + @Override + protected String contentType() { + return CONTENT_TYPE; + } + + @Override + protected void parseCreateField(ParseContext context) { + throw new MapperParsingException( + String.format( + Locale.ROOT, + "Field [%s] is a star tree field and cannot be added inside a document. Use the index API request parameters.", + name() + ) + ); + } + + /** + * Star tree mapped field type containing dimensions, metrics, star tree specs + * + * @opensearch.experimental + */ + @ExperimentalApi + public static final class StarTreeFieldType extends CompositeDataCubeFieldType { + + private final StarTreeFieldSpec starTreeFieldSpec; + + public StarTreeFieldType(String name, StarTreeField starTreeField) { + super(name, starTreeField.getDimensionsOrder(), starTreeField.getMetrics(), CompositeFieldType.STAR_TREE); + this.starTreeFieldSpec = starTreeField.getSpec(); + } + + @Override + public ValueFetcher valueFetcher(QueryShardContext context, SearchLookup searchLookup, String format) { + // TODO : evaluate later + throw new UnsupportedOperationException("Cannot fetch values for star tree field [" + name() + "]."); + } + + @Override + public String typeName() { + return CONTENT_TYPE; + } + + @Override + public Query termQuery(Object value, QueryShardContext context) { + // TODO : evaluate later + throw new UnsupportedOperationException("Cannot perform terms query on star tree field [" + name() + "]."); + } + } + +} diff --git a/server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java b/server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java new file mode 100644 index 0000000000000..2e838ddb44807 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java @@ -0,0 +1,25 @@ +/* + * 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.indices; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.index.compositeindex.CompositeIndexSettings; + +/** + * Utility to provide a {@link CompositeIndexSettings} instance containing all defaults + */ +public class DefaultCompositeIndexSettings { + private DefaultCompositeIndexSettings() {} + + public static final CompositeIndexSettings INSTANCE = new CompositeIndexSettings( + Settings.EMPTY, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); +} diff --git a/server/src/main/java/org/opensearch/indices/IndicesModule.java b/server/src/main/java/org/opensearch/indices/IndicesModule.java index 033b163bb0d67..f7e52ce9fc1ae 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesModule.java +++ b/server/src/main/java/org/opensearch/indices/IndicesModule.java @@ -70,6 +70,7 @@ import org.opensearch.index.mapper.RoutingFieldMapper; import org.opensearch.index.mapper.SeqNoFieldMapper; import org.opensearch.index.mapper.SourceFieldMapper; +import org.opensearch.index.mapper.StarTreeMapper; import org.opensearch.index.mapper.TextFieldMapper; import org.opensearch.index.mapper.VersionFieldMapper; import org.opensearch.index.mapper.WildcardFieldMapper; @@ -174,6 +175,7 @@ public static Map getMappers(List mappe mappers.put(ConstantKeywordFieldMapper.CONTENT_TYPE, new ConstantKeywordFieldMapper.TypeParser()); mappers.put(DerivedFieldMapper.CONTENT_TYPE, DerivedFieldMapper.PARSER); mappers.put(WildcardFieldMapper.CONTENT_TYPE, WildcardFieldMapper.PARSER); + mappers.put(StarTreeMapper.CONTENT_TYPE, new StarTreeMapper.TypeParser()); for (MapperPlugin mapperPlugin : mapperPlugins) { for (Map.Entry entry : mapperPlugin.getMappers().entrySet()) { diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 251be8a990055..b1fb2f77e2981 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -106,6 +106,7 @@ import org.opensearch.index.IndexSettings; import org.opensearch.index.analysis.AnalysisRegistry; import org.opensearch.index.cache.request.ShardRequestCache; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.engine.CommitStats; import org.opensearch.index.engine.EngineConfig; import org.opensearch.index.engine.EngineConfigFactory; @@ -354,6 +355,7 @@ public class IndicesService extends AbstractLifecycleComponent private final BiFunction translogFactorySupplier; private volatile TimeValue clusterDefaultRefreshInterval; private final SearchRequestStats searchRequestStats; + private final CompositeIndexSettings compositeIndexSettings; @Override protected void doStart() { @@ -388,7 +390,8 @@ public IndicesService( @Nullable RemoteStoreStatsTrackerFactory remoteStoreStatsTrackerFactory, RecoverySettings recoverySettings, CacheService cacheService, - RemoteStoreSettings remoteStoreSettings + RemoteStoreSettings remoteStoreSettings, + CompositeIndexSettings compositeIndexSettings ) { this.settings = settings; this.threadPool = threadPool; @@ -495,6 +498,7 @@ protected void closeInternal() { .addSettingsUpdateConsumer(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING, this::onRefreshIntervalUpdate); this.recoverySettings = recoverySettings; this.remoteStoreSettings = remoteStoreSettings; + this.compositeIndexSettings = compositeIndexSettings; } /** @@ -903,7 +907,8 @@ private synchronized IndexService createIndexService( translogFactorySupplier, this::getClusterDefaultRefreshInterval, this.recoverySettings, - this.remoteStoreSettings + this.remoteStoreSettings, + this.compositeIndexSettings ); } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 397949525a3ec..eb05d70066439 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -147,6 +147,7 @@ import org.opensearch.index.IndexingPressureService; import org.opensearch.index.SegmentReplicationStatsTracker; import org.opensearch.index.analysis.AnalysisRegistry; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.engine.EngineFactory; import org.opensearch.index.recovery.RemoteStoreRestoreService; import org.opensearch.index.remote.RemoteIndexPathUploader; @@ -830,6 +831,7 @@ protected Node( final RecoverySettings recoverySettings = new RecoverySettings(settings, settingsModule.getClusterSettings()); final RemoteStoreSettings remoteStoreSettings = new RemoteStoreSettings(settings, settingsModule.getClusterSettings()); + final CompositeIndexSettings compositeIndexSettings = new CompositeIndexSettings(settings, settingsModule.getClusterSettings()); final IndexStorePlugin.DirectoryFactory remoteDirectoryFactory = new RemoteSegmentStoreDirectoryFactory( repositoriesServiceReference::get, @@ -869,7 +871,8 @@ protected Node( remoteStoreStatsTrackerFactory, recoverySettings, cacheService, - remoteStoreSettings + remoteStoreSettings, + compositeIndexSettings ); final IngestService ingestService = new IngestService( diff --git a/server/src/test/java/org/opensearch/index/IndexModuleTests.java b/server/src/test/java/org/opensearch/index/IndexModuleTests.java index 4ce4936c047d9..8f45a872e752c 100644 --- a/server/src/test/java/org/opensearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/opensearch/index/IndexModuleTests.java @@ -99,6 +99,7 @@ import org.opensearch.index.translog.InternalTranslogFactory; import org.opensearch.index.translog.RemoteBlobStoreInternalTranslogFactory; import org.opensearch.index.translog.TranslogFactory; +import org.opensearch.indices.DefaultCompositeIndexSettings; import org.opensearch.indices.DefaultRemoteStoreSettings; import org.opensearch.indices.IndicesModule; import org.opensearch.indices.IndicesQueryCache; @@ -264,7 +265,8 @@ private IndexService newIndexService(IndexModule module) throws IOException { translogFactorySupplier, () -> IndexSettings.DEFAULT_REFRESH_INTERVAL, DefaultRecoverySettings.INSTANCE, - DefaultRemoteStoreSettings.INSTANCE + DefaultRemoteStoreSettings.INSTANCE, + DefaultCompositeIndexSettings.INSTANCE ); } diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 9c58fc8fde084..01ed36e0f67d8 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -192,6 +192,7 @@ import org.opensearch.index.shard.PrimaryReplicaSyncer; import org.opensearch.index.store.RemoteSegmentStoreDirectoryFactory; import org.opensearch.index.store.remote.filecache.FileCacheStats; +import org.opensearch.indices.DefaultCompositeIndexSettings; import org.opensearch.indices.DefaultRemoteStoreSettings; import org.opensearch.indices.IndicesModule; import org.opensearch.indices.IndicesService; @@ -2078,7 +2079,8 @@ public void onFailure(final Exception e) { new RemoteStoreStatsTrackerFactory(clusterService, settings), DefaultRecoverySettings.INSTANCE, new CacheModule(new ArrayList<>(), settings).getCacheService(), - DefaultRemoteStoreSettings.INSTANCE + DefaultRemoteStoreSettings.INSTANCE, + DefaultCompositeIndexSettings.INSTANCE ); final RecoverySettings recoverySettings = new RecoverySettings(settings, clusterSettings); snapshotShardsService = new SnapshotShardsService( From 9560d1951b1358290f39a302801954df2ef7e3b5 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Thu, 13 Jun 2024 15:29:50 +0530 Subject: [PATCH 2/4] Fixes and tests Signed-off-by: Bharathwaj G --- .../index/compositeindex/DateDimension.java | 4 +- .../startree/StarTreeFieldSpec.java | 8 + .../index/mapper/StarTreeMapper.java | 13 +- .../index/mapper/StarTreeMapperTests.java | 425 ++++++++++++++++++ .../index/mapper/MapperTestCase.java | 2 +- .../aggregations/AggregatorTestCase.java | 2 + 6 files changed, 447 insertions(+), 7 deletions(-) create mode 100644 server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java diff --git a/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java b/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java index 1dd764ac97df0..087817420440e 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java @@ -17,7 +17,7 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -53,7 +53,7 @@ public DateDimension(Map.Entry dimension, Mapper.TypeParser.Pars String.format(Locale.ROOT, "At most 3 calendar intervals are allowed in dimension [%s]", dimension.getKey()) ); } - Set calendarIntervals = new HashSet<>(); + Set calendarIntervals = new LinkedHashSet<>(); for (String interval : intervalStrings) { calendarIntervals.add(StarTreeIndexSettings.getTimeUnit(interval)); } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java index a7dbd860bd8bc..09a133f4a6fe5 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java @@ -85,4 +85,12 @@ public String toString() { public int maxLeafDocs() { return maxLeafDocs.get(); } + + public StarTreeBuildMode getBuildMode() { + return buildMode; + } + + public List getSkipStarNodeCreationInDims() { + return skipStarNodeCreationInDims; + } } 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 528b56458b592..da5aaf763ad6a 100644 --- a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java @@ -23,8 +23,9 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; import java.util.Locale; import java.util.Map; @@ -118,7 +119,7 @@ private List buildDimensions(Map map, Mapper.TypePars String.format(Locale.ROOT, "ordered_dimensions is required for star tree field [%s]", this.name) ); } - List dimensions = new ArrayList<>(); + List dimensions = new LinkedList<>(); if (dims instanceof LinkedHashMap) { if (((LinkedHashMap) dims).size() > context.getSettings() .getAsInt(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 10)) { @@ -214,7 +215,7 @@ private Dimension getDimension(String type, Map.Entry dim, Mappe */ @SuppressWarnings("unchecked") private List buildMetrics(Map map, Mapper.TypeParser.ParserContext context) { - List metrics = new ArrayList<>(); + List metrics = new LinkedList<>(); Object metricsFromInput = XContentMapValues.extractValue("metrics", map); if (metricsFromInput == null) { throw new IllegalArgumentException( @@ -257,7 +258,7 @@ private Metric getMetric(Map.Entry map, Mapper.TypeParser.Parser if (metricStrings.isEmpty()) { metricTypes = new ArrayList<>(StarTreeIndexSettings.DEFAULT_METRICS_LIST.get(context.getSettings())); } else { - Set metricSet = new HashSet<>(); + Set metricSet = new LinkedHashSet<>(); for (String metricString : metricStrings) { metricSet.add(MetricType.fromTypeName(metricString)); } @@ -373,6 +374,10 @@ public StarTreeFieldType(String name, StarTreeField starTreeField) { this.starTreeFieldSpec = starTreeField.getSpec(); } + public StarTreeFieldSpec getStarTreeFieldSpec() { + return starTreeFieldSpec; + } + @Override public ValueFetcher valueFetcher(QueryShardContext context, SearchLookup searchLookup, String format) { // TODO : evaluate later diff --git a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java new file mode 100644 index 0000000000000..66f9a19c5b8b8 --- /dev/null +++ b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java @@ -0,0 +1,425 @@ +/* + * 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.index.mapper; + +import org.opensearch.common.CheckedConsumer; +import org.opensearch.common.Rounding; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.compositeindex.DateDimension; +import org.opensearch.index.compositeindex.MetricType; +import org.opensearch.index.compositeindex.startree.StarTreeFieldSpec; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import static org.hamcrest.Matchers.containsString; + +/** + * Tests for {@link StarTreeMapper}. + */ +public class StarTreeMapperTests extends MapperTestCase { + + @Before + public void setup() { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.STAR_TREE_INDEX, true).build()); + } + + @After + public void teardown() { + FeatureFlags.initializeFeatureFlags(Settings.EMPTY); + } + + public void testValidStarTree() throws IOException { + MapperService mapperService = createMapperService(getExpandedMapping("status", "size")); + Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); + for (CompositeMappedFieldType type : compositeFieldTypes) { + StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) type; + assertEquals("@timestamp", starTreeFieldType.getDimensions().get(0).getField()); + assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); + DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); + List expectedTimeUnits = Arrays.asList( + Rounding.DateTimeUnit.DAY_OF_MONTH, + Rounding.DateTimeUnit.MONTH_OF_YEAR + ); + assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); + assertEquals("size", starTreeFieldType.getMetrics().get(0).getField()); + List expectedMetrics = Arrays.asList(MetricType.SUM, MetricType.AVG); + assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); + assertEquals(100, starTreeFieldType.getStarTreeFieldSpec().maxLeafDocs()); + assertEquals(StarTreeFieldSpec.StarTreeBuildMode.ON_HEAP, starTreeFieldType.getStarTreeFieldSpec().getBuildMode()); + assertEquals(Arrays.asList("@timestamp", "status"), starTreeFieldType.getStarTreeFieldSpec().getSkipStarNodeCreationInDims()); + } + } + + public void testValidStarTreeDefaults() throws IOException { + MapperService mapperService = createMapperService(getMinMapping()); + Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); + for (CompositeMappedFieldType type : compositeFieldTypes) { + StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) type; + assertEquals("@timestamp", starTreeFieldType.getDimensions().get(0).getField()); + assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); + DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); + List expectedTimeUnits = Arrays.asList( + Rounding.DateTimeUnit.MINUTES_OF_HOUR, + Rounding.DateTimeUnit.HOUR_OF_DAY + ); + assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); + assertEquals("status", starTreeFieldType.getMetrics().get(0).getField()); + List expectedMetrics = Arrays.asList( + MetricType.AVG, + MetricType.COUNT, + MetricType.SUM, + MetricType.MAX, + MetricType.MIN + ); + assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); + assertEquals(10000, starTreeFieldType.getStarTreeFieldSpec().maxLeafDocs()); + assertEquals(StarTreeFieldSpec.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeFieldSpec().getBuildMode()); + assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeFieldSpec().getSkipStarNodeCreationInDims()); + } + } + + public void testInvalidDim() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getExpandedMapping("invalid", "size")) + ); + assertEquals("Failed to parse mapping [_doc]: unknown dimension field [invalid]", ex.getMessage()); + } + + public void testInvalidMetric() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getExpandedMapping("status", "invalid")) + ); + assertEquals("Failed to parse mapping [_doc]: unknown metric field [invalid]", ex.getMessage()); + } + + public void testNoMetrics() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getMinMapping(false, true, false, false)) + ); + assertThat( + ex.getMessage(), + containsString("Failed to parse mapping [_doc]: metrics section is required for star tree field [startree]") + ); + } + + public void testNoDims() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getMinMapping(true, false, false, false)) + ); + assertThat( + ex.getMessage(), + containsString("Failed to parse mapping [_doc]: ordered_dimensions is required for star tree field [startree]") + ); + } + + public void testMissingDims() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getMinMapping(false, false, true, false)) + ); + assertThat(ex.getMessage(), containsString("Failed to parse mapping [_doc]: unknown dimension field [@timestamp]")); + } + + public void testMissingMetrics() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getMinMapping(false, false, false, true)) + ); + assertThat(ex.getMessage(), containsString("Failed to parse mapping [_doc]: unknown metric field [metric_field]")); + } + + public void testInvalidMetricType() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getInvalidMapping(false, false, false, true)) + ); + assertEquals( + "Failed to parse mapping [_doc]: non-numeric field type is associated with star tree metric [startree]", + ex.getMessage() + ); + } + + public void testInvalidDimType() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getInvalidMapping(false, false, true, false)) + ); + assertEquals( + "Failed to parse mapping [_doc]: unsupported field type associated with dimension [@timestamp] as part of star tree field [startree]", + ex.getMessage() + ); + } + + public void testInvalidSkipDim() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getInvalidMapping(false, true, false, false)) + ); + assertEquals( + "Failed to parse mapping [_doc]: [invalid] in skip_star_node_creation_for_dimensions should be part of ordered_dimensions", + ex.getMessage() + ); + } + + public void testInvalidSingleDim() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getInvalidMapping(true, false, false, false)) + ); + assertEquals( + "Failed to parse mapping [_doc]: Atleast two dimensions are required to build star tree index field [startree]", + ex.getMessage() + ); + } + + private XContentBuilder getExpandedMapping(String dim, String metric) throws IOException { + return topMapping(b -> { + b.startObject("composite"); + b.startObject("startree"); + b.field("type", "star_tree"); + b.startObject("config"); + b.field("build_mode", "onheap"); + b.field("max_leaf_docs", 100); + b.startArray("skip_star_node_creation_for_dimensions"); + { + b.value("@timestamp"); + b.value("status"); + } + b.endArray(); + b.startObject("ordered_dimensions"); + b.startObject("@timestamp"); + b.startArray("calendar_interval"); + b.value("day"); + b.value("month"); + b.endArray(); + b.endObject(); + b.startObject(dim); + b.endObject(); + b.endObject(); + b.startObject("metrics"); + b.startObject(metric); + b.startArray("metrics"); + b.value("sum"); + b.value("avg"); + b.endArray(); + b.endObject(); + b.endObject(); + b.endObject(); + b.endObject(); + b.endObject(); + b.startObject("properties"); + b.startObject("@timestamp"); + b.field("type", "date"); + b.endObject(); + b.startObject("status"); + b.field("type", "integer"); + b.endObject(); + b.startObject("size"); + b.field("type", "integer"); + b.endObject(); + b.endObject(); + }); + } + + private XContentBuilder getMinMapping() throws IOException { + return getMinMapping(false, false, false, false); + } + + private XContentBuilder getMinMapping(boolean isEmptyDims, boolean isEmptyMetrics, boolean missingDim, boolean missingMetric) + throws IOException { + return topMapping(b -> { + b.startObject("composite"); + b.startObject("startree"); + b.field("type", "star_tree"); + b.startObject("config"); + if (!isEmptyDims) { + b.startObject("ordered_dimensions"); + b.startObject("@timestamp"); + b.endObject(); + b.startObject("status"); + b.endObject(); + b.endObject(); + } + if (!isEmptyMetrics) { + b.startObject("metrics"); + b.startObject("status"); + b.endObject(); + b.startObject("metric_field"); + b.endObject(); + b.endObject(); + } + b.endObject(); + b.endObject(); + b.endObject(); + b.startObject("properties"); + if (!missingDim) { + b.startObject("@timestamp"); + b.field("type", "date"); + b.endObject(); + } + b.startObject("status"); + b.field("type", "integer"); + b.endObject(); + if (!missingMetric) { + b.startObject("metric_field"); + b.field("type", "integer"); + b.endObject(); + } + b.endObject(); + }); + } + + private XContentBuilder getInvalidMapping(boolean singleDim, boolean invalidSkipDims, boolean invalidDimType, boolean invalidMetricType) + throws IOException { + return topMapping(b -> { + b.startObject("composite"); + b.startObject("startree"); + b.field("type", "star_tree"); + b.startObject("config"); + + b.startArray("skip_star_node_creation_for_dimensions"); + { + if (invalidSkipDims) { + b.value("invalid"); + } + b.value("status"); + } + b.endArray(); + b.startObject("ordered_dimensions"); + if (!singleDim) { + b.startObject("@timestamp"); + b.endObject(); + } + b.startObject("status"); + b.endObject(); + b.endObject(); + b.startObject("metrics"); + b.startObject("status"); + b.endObject(); + b.startObject("metric_field"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endObject(); + b.endObject(); + b.startObject("properties"); + b.startObject("@timestamp"); + if (!invalidDimType) { + b.field("type", "date"); + } else { + b.field("type", "keyword"); + } + b.endObject(); + + b.startObject("status"); + b.field("type", "integer"); + b.endObject(); + b.startObject("metric_field"); + if (invalidMetricType) { + b.field("type", "date"); + } else { + b.field("type", "integer"); + } + b.endObject(); + b.endObject(); + }); + } + + protected boolean supportsOrIgnoresBoost() { + return false; + } + + protected boolean supportsMeta() { + return false; + } + + @Override + protected void assertExistsQuery(MapperService mapperService) {} + + // Overriding fieldMapping to make it create composite mappings by default. + // This way, the parent tests are checking the right behavior for this Mapper. + @Override + protected final XContentBuilder fieldMapping(CheckedConsumer buildField) throws IOException { + return topMapping(b -> { + b.startObject("composite"); + b.startObject("startree"); + buildField.accept(b); + b.endObject(); + b.endObject(); + b.startObject("properties"); + b.startObject("size"); + b.field("type", "integer"); + b.endObject(); + b.startObject("status"); + b.field("type", "integer"); + b.endObject(); + b.endObject(); + }); + } + + @Override + public void testEmptyName() { + MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(topMapping(b -> { + b.startObject("composite"); + b.startObject(""); + minimalMapping(b); + b.endObject(); + b.endObject(); + b.startObject("properties"); + b.startObject("size"); + b.field("type", "integer"); + b.endObject(); + b.startObject("status"); + b.field("type", "integer"); + b.endObject(); + b.endObject(); + }))); + assertThat(e.getMessage(), containsString("name cannot be empty string")); + assertParseMinimalWarnings(); + } + + @Override + protected void minimalMapping(XContentBuilder b) throws IOException { + b.field("type", "star_tree"); + b.startObject("config"); + b.startObject("ordered_dimensions"); + b.startObject("size"); + b.endObject(); + b.startObject("status"); + b.endObject(); + b.endObject(); + b.startObject("metrics"); + b.startObject("status"); + b.endObject(); + b.endObject(); + b.endObject(); + } + + @Override + protected void writeFieldValue(XContentBuilder builder) throws IOException {} + + @Override + protected void registerParameters(ParameterChecker checker) throws IOException { + + } +} diff --git a/test/framework/src/main/java/org/opensearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/opensearch/index/mapper/MapperTestCase.java index dc5954907a4fa..01a4005255f29 100644 --- a/test/framework/src/main/java/org/opensearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/opensearch/index/mapper/MapperTestCase.java @@ -174,7 +174,7 @@ protected static void assertNoDocValuesField(ParseContext.Document doc, String f } } - public final void testEmptyName() { + public void testEmptyName() { MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping(b -> { b.startObject(""); minimalMapping(b); diff --git a/test/framework/src/main/java/org/opensearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/opensearch/search/aggregations/AggregatorTestCase.java index 50b27ec000615..37ce1951e677e 100644 --- a/test/framework/src/main/java/org/opensearch/search/aggregations/AggregatorTestCase.java +++ b/test/framework/src/main/java/org/opensearch/search/aggregations/AggregatorTestCase.java @@ -114,6 +114,7 @@ import org.opensearch.index.mapper.ObjectMapper.Nested; import org.opensearch.index.mapper.RangeFieldMapper; import org.opensearch.index.mapper.RangeType; +import org.opensearch.index.mapper.StarTreeMapper; import org.opensearch.index.mapper.TextFieldMapper; import org.opensearch.index.query.QueryShardContext; import org.opensearch.index.shard.IndexShard; @@ -200,6 +201,7 @@ public abstract class AggregatorTestCase extends OpenSearchTestCase { denylist.add(CompletionFieldMapper.CONTENT_TYPE); // TODO support completion denylist.add(FieldAliasMapper.CONTENT_TYPE); // TODO support alias denylist.add(DerivedFieldMapper.CONTENT_TYPE); // TODO support derived fields + denylist.add(StarTreeMapper.CONTENT_TYPE); // TODO evaluate support for star tree fields TYPE_TEST_DENYLIST = denylist; } From a83794b41c9195c64974ed2b54668d82d2e4016e Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Mon, 17 Jun 2024 23:54:21 +0530 Subject: [PATCH 3/4] addressing review comments and adding validations, integ tests Signed-off-by: Bharathwaj G --- distribution/src/config/opensearch.yml | 2 +- .../metadata/MetadataCreateIndexService.java | 2 +- .../metadata/MetadataMappingService.java | 12 +- .../common/settings/IndexScopedSettings.java | 3 +- .../opensearch/common/util/FeatureFlags.java | 2 +- .../CompositeIndexSettings.java | 14 +- .../CompositeIndexValidator.java | 52 ++- .../{ => datacube}/DateDimension.java | 49 ++- .../{ => datacube}/Dimension.java | 23 +- .../compositeindex/{ => datacube}/Metric.java | 29 +- .../MetricStat.java} | 12 +- .../compositeindex/datacube/package-info.java | 11 + .../startree/StarTreeField.java | 43 ++- .../startree/StarTreeFieldConfiguration.java} | 28 +- .../startree/StarTreeIndexSettings.java | 45 ++- .../{ => datacube}/startree/package-info.java | 2 +- .../mapper/CompositeDataCubeFieldType.java | 4 +- .../mapper/CompositeMappedFieldType.java | 21 +- .../opensearch/index/mapper/ObjectMapper.java | 2 +- .../index/mapper/StarTreeMapper.java | 242 +++++++----- .../DefaultCompositeIndexSettings.java | 25 -- .../opensearch/indices/IndicesService.java | 4 + .../opensearch/index/IndexModuleTests.java | 4 +- .../startree/StarTreeMappingIntegTests.java | 364 ++++++++++++++++++ .../index/mapper/StarTreeMapperTests.java | 138 ++++--- .../snapshots/SnapshotResiliencyTests.java | 4 +- 26 files changed, 872 insertions(+), 265 deletions(-) rename server/src/main/java/org/opensearch/index/compositeindex/{ => datacube}/DateDimension.java (56%) rename server/src/main/java/org/opensearch/index/compositeindex/{ => datacube}/Dimension.java (56%) rename server/src/main/java/org/opensearch/index/compositeindex/{ => datacube}/Metric.java (56%) rename server/src/main/java/org/opensearch/index/compositeindex/{MetricType.java => datacube/MetricStat.java} (71%) create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/datacube/package-info.java rename server/src/main/java/org/opensearch/index/compositeindex/{ => datacube}/startree/StarTreeField.java (57%) rename server/src/main/java/org/opensearch/index/compositeindex/{startree/StarTreeFieldSpec.java => datacube/startree/StarTreeFieldConfiguration.java} (74%) rename server/src/main/java/org/opensearch/index/compositeindex/{ => datacube}/startree/StarTreeIndexSettings.java (68%) rename server/src/main/java/org/opensearch/index/compositeindex/{ => datacube}/startree/package-info.java (80%) delete mode 100644 server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java create mode 100644 server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml index e6aa3bf3173fd..4115601f62ada 100644 --- a/distribution/src/config/opensearch.yml +++ b/distribution/src/config/opensearch.yml @@ -128,4 +128,4 @@ ${path.logs} # # Gates the functionality of star tree index, which improves the performance of search aggregations. # -#opensearch.experimental.feature.composite.star_tree.enabled: true +#opensearch.experimental.feature.composite_index.star_tree.enabled: true 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 efa78fa2ac0af..7973745ce84b3 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1320,7 +1320,7 @@ private static void updateIndexMappingsAndBuildSortOrder( } if (mapperService.isCompositeIndexPresent()) { - CompositeIndexValidator.validate(mapperService, indexService.getCompositeIndexSettings()); + CompositeIndexValidator.validate(mapperService, indexService.getCompositeIndexSettings(), indexService.getIndexSettings()); } if (sourceMetadata == null) { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java index 1406287149e8d..43894db86c512 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java @@ -55,6 +55,7 @@ import org.opensearch.core.common.Strings; import org.opensearch.core.index.Index; import org.opensearch.index.IndexService; +import org.opensearch.index.compositeindex.CompositeIndexValidator; import org.opensearch.index.mapper.DocumentMapper; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.mapper.MapperService.MergeReason; @@ -282,6 +283,7 @@ private ClusterState applyRequest( // first, simulate: just call merge and ignore the result existingMapper.merge(newMapper.mapping(), MergeReason.MAPPING_UPDATE); } + } Metadata.Builder builder = Metadata.builder(metadata); boolean updated = false; @@ -291,7 +293,7 @@ private ClusterState applyRequest( // we use the exact same indexService and metadata we used to validate above here to actually apply the update final Index index = indexMetadata.getIndex(); final MapperService mapperService = indexMapperServices.get(index); - + boolean isCompositeFieldPresent = !mapperService.getCompositeFieldTypes().isEmpty(); CompressedXContent existingSource = null; DocumentMapper existingMapper = mapperService.documentMapper(); if (existingMapper != null) { @@ -302,6 +304,14 @@ private ClusterState applyRequest( mappingUpdateSource, MergeReason.MAPPING_UPDATE ); + + CompositeIndexValidator.validate( + mapperService, + indicesService.getCompositeIndexSettings(), + mapperService.getIndexSettings(), + isCompositeFieldPresent + ); + CompressedXContent updatedSource = mergedMapper.mappingSource(); if (existingSource != null) { 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 54ab787777d94..193edccb95f3c 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -51,7 +51,7 @@ import org.opensearch.index.SearchSlowLog; import org.opensearch.index.TieredMergePolicyProvider; import org.opensearch.index.cache.bitset.BitsetFilterCache; -import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.engine.EngineConfig; import org.opensearch.index.fielddata.IndexFieldDataService; import org.opensearch.index.mapper.FieldMapper; @@ -246,6 +246,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING, StarTreeIndexSettings.DEFAULT_METRICS_LIST, StarTreeIndexSettings.DEFAULT_DATE_INTERVALS, + StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING, // validate that built-in similarities don't get redefined Setting.groupSetting("index.similarity.", (s) -> { diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index c0b61f9cc278d..ceb2559a0e16c 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -104,7 +104,7 @@ public class FeatureFlags { * Gates the functionality of star tree index, which improves the performance of search * aggregations. */ - public static final String STAR_TREE_INDEX = "opensearch.experimental.feature.composite.star_tree.enabled"; + public static final String STAR_TREE_INDEX = "opensearch.experimental.feature.composite_index.star_tree.enabled"; public static final Setting STAR_TREE_INDEX_SETTING = Setting.boolSetting(STAR_TREE_INDEX, false, Property.NodeScope); private static final List> ALL_FEATURE_FLAG_SETTINGS = List.of( 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 ee55a13c18e1f..761f3c76ac15e 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java @@ -22,7 +22,7 @@ @ExperimentalApi public class CompositeIndexSettings { public static final Setting STAR_TREE_INDEX_ENABLED_SETTING = Setting.boolSetting( - "indices.composite.star_tree.enabled", + "indices.composite_index.star_tree.enabled", false, value -> { if (FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING) == false && value == true) { @@ -52,4 +52,16 @@ private void starTreeIndexCreationEnabled(boolean value) { public boolean isStarTreeIndexCreationEnabled() { return starTreeIndexCreationEnabled; } + + /** + * Utility to provide a {@link CompositeIndexSettings} instance containing all defaults + */ + public static class DefaultCompositeIndexSettings { + private DefaultCompositeIndexSettings() {} + + public static final CompositeIndexSettings INSTANCE = new CompositeIndexSettings( + Settings.EMPTY, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java index b59bae0092a19..26971f96677af 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java @@ -9,6 +9,10 @@ package org.opensearch.index.compositeindex; import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.index.IndexSettings; +import org.opensearch.index.compositeindex.datacube.Dimension; +import org.opensearch.index.compositeindex.datacube.Metric; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.mapper.CompositeDataCubeFieldType; import org.opensearch.index.mapper.CompositeMappedFieldType; import org.opensearch.index.mapper.MappedFieldType; @@ -26,12 +30,44 @@ @ExperimentalApi public class CompositeIndexValidator { - public static void validate(MapperService mapperService, CompositeIndexSettings compositeIndexSettings) { - validateStarTreeMappings(mapperService, compositeIndexSettings); + public static void validate(MapperService mapperService, CompositeIndexSettings compositeIndexSettings, IndexSettings indexSettings) { + validateStarTreeMappings(mapperService, compositeIndexSettings, indexSettings); } - private static void validateStarTreeMappings(MapperService mapperService, CompositeIndexSettings compositeIndexSettings) { + public static void validate( + MapperService mapperService, + CompositeIndexSettings compositeIndexSettings, + IndexSettings indexSettings, + boolean isCompositeFieldPresent + ) { + if (!isCompositeFieldPresent && mapperService.isCompositeIndexPresent()) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Composite fields must be specified during index creation, addition of new composite fields during update is not supported" + ) + ); + } + validateStarTreeMappings(mapperService, compositeIndexSettings, indexSettings); + } + + private static void validateStarTreeMappings( + MapperService mapperService, + CompositeIndexSettings compositeIndexSettings, + IndexSettings indexSettings + ) { Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); + if (mapperService.getCompositeFieldTypes().size() > StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get( + indexSettings.getSettings() + )) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "Index cannot have more than [%s] star tree fields", + StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get(indexSettings.getSettings()) + ) + ); + } for (CompositeMappedFieldType compositeFieldType : compositeFieldTypes) { if (!(compositeFieldType instanceof StarTreeMapper.StarTreeFieldType)) { return; @@ -48,6 +84,11 @@ private static void validateStarTreeMappings(MapperService mapperService, Compos CompositeDataCubeFieldType dataCubeFieldType = (CompositeDataCubeFieldType) compositeFieldType; for (Dimension dim : dataCubeFieldType.getDimensions()) { MappedFieldType ft = mapperService.fieldType(dim.getField()); + if (ft == null) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "unknown dimension field [%s] as part of star tree field", dim.getField()) + ); + } if (!ft.isAggregatable()) { throw new IllegalArgumentException( String.format( @@ -61,6 +102,11 @@ private static void validateStarTreeMappings(MapperService mapperService, Compos } for (Metric metric : dataCubeFieldType.getMetrics()) { MappedFieldType ft = mapperService.fieldType(metric.getField()); + if (ft == null) { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "unknown metric field [%s] as part of star tree field", metric.getField()) + ); + } if (!ft.isAggregatable()) { throw new IllegalArgumentException( String.format( diff --git a/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DateDimension.java similarity index 56% rename from server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/DateDimension.java index 087817420440e..14fcfc56f6354 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/DateDimension.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DateDimension.java @@ -6,14 +6,15 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex; +package org.opensearch.index.compositeindex.datacube; import org.opensearch.common.Rounding; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.mapper.Mapper; +import org.opensearch.index.mapper.StarTreeMapper; import java.io.IOException; import java.util.ArrayList; @@ -21,6 +22,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -32,25 +34,26 @@ @ExperimentalApi public class DateDimension extends Dimension { private final List calendarIntervals; + public static final String CALENDAR_INTERVALS = "calendar_intervals"; + public static final String DATE = "date"; - public DateDimension(String name, List intervals) { + public DateDimension(String name, Map dimensionMap, Mapper.TypeParser.ParserContext c) { super(name); - this.calendarIntervals = intervals; - } - - @SuppressWarnings("unchecked") - public DateDimension(Map.Entry dimension, Mapper.TypeParser.ParserContext c) { - super(dimension.getKey()); - List intervalStrings = XContentMapValues.extractRawValues("calendar_interval", (Map) dimension.getValue()) + List intervalStrings = XContentMapValues.extractRawValues(CALENDAR_INTERVALS, dimensionMap) .stream() .map(Object::toString) .collect(Collectors.toList()); if (intervalStrings == null || intervalStrings.isEmpty()) { this.calendarIntervals = StarTreeIndexSettings.DEFAULT_DATE_INTERVALS.get(c.getSettings()); } else { - if (intervalStrings.size() > 3) { + if (intervalStrings.size() > StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING.get(c.getSettings())) { throw new IllegalArgumentException( - String.format(Locale.ROOT, "At most 3 calendar intervals are allowed in dimension [%s]", dimension.getKey()) + String.format( + Locale.ROOT, + "At most [%s] calendar intervals are allowed in dimension [%s]", + StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING.get(c.getSettings()), + name + ) ); } Set calendarIntervals = new LinkedHashSet<>(); @@ -59,6 +62,7 @@ public DateDimension(Map.Entry dimension, Mapper.TypeParser.Pars } this.calendarIntervals = new ArrayList<>(calendarIntervals); } + dimensionMap.remove(CALENDAR_INTERVALS); } public List getIntervals() { @@ -67,9 +71,10 @@ public List getIntervals() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(this.getField()); - builder.field("type", "date"); - builder.startArray("calendar_interval"); + builder.startObject(); + builder.field(StarTreeMapper.NAME, this.getField()); + builder.field(StarTreeMapper.TYPE, DATE); + builder.startArray(CALENDAR_INTERVALS); for (Rounding.DateTimeUnit interval : calendarIntervals) { builder.value(interval.shortName()); } @@ -77,4 +82,18 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + DateDimension that = (DateDimension) o; + return Objects.equals(calendarIntervals, that.calendarIntervals); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), calendarIntervals); + } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/Dimension.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/Dimension.java similarity index 56% rename from server/src/main/java/org/opensearch/index/compositeindex/Dimension.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/Dimension.java index 426508b05b59a..cc3ce3b88e757 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/Dimension.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/Dimension.java @@ -6,13 +6,15 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex; +package org.opensearch.index.compositeindex.datacube; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.mapper.StarTreeMapper; import java.io.IOException; +import java.util.Objects; /** * Composite index dimension base class @@ -21,6 +23,7 @@ */ @ExperimentalApi public class Dimension implements ToXContent { + public static final String NUMERIC = "numeric"; private final String field; public Dimension(String field) { @@ -33,9 +36,23 @@ public String getField() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(field); - builder.field("type", "numeric"); + builder.startObject(); + builder.field(StarTreeMapper.NAME, field); + builder.field(StarTreeMapper.TYPE, NUMERIC); builder.endObject(); return builder; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Dimension dimension = (Dimension) o; + return Objects.equals(field, dimension.field); + } + + @Override + public int hashCode() { + return Objects.hash(field); + } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/Metric.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/Metric.java similarity index 56% rename from server/src/main/java/org/opensearch/index/compositeindex/Metric.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/Metric.java index bde16f305bba7..9accb0201170a 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/Metric.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/Metric.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex; +package org.opensearch.index.compositeindex.datacube; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.core.xcontent.ToXContent; @@ -14,6 +14,7 @@ import java.io.IOException; import java.util.List; +import java.util.Objects; /** * Holds details of metrics field as part of composite field @@ -21,9 +22,9 @@ @ExperimentalApi public class Metric implements ToXContent { private final String field; - private final List metrics; + private final List metrics; - public Metric(String field, List metrics) { + public Metric(String field, List metrics) { this.field = field; this.metrics = metrics; } @@ -32,19 +33,33 @@ public String getField() { return field; } - public List getMetrics() { + public List getMetrics() { return metrics; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(field); - builder.startArray("metrics"); - for (MetricType metricType : metrics) { + builder.startObject(); + builder.field("name", field); + builder.startArray("stats"); + for (MetricStat metricType : metrics) { builder.value(metricType.getTypeName()); } builder.endArray(); builder.endObject(); return builder; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Metric metric = (Metric) o; + return Objects.equals(field, metric.field) && Objects.equals(metrics, metric.metrics); + } + + @Override + public int hashCode() { + return Objects.hash(field, metrics); + } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/MetricType.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/MetricStat.java similarity index 71% rename from server/src/main/java/org/opensearch/index/compositeindex/MetricType.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/MetricStat.java index 1a39e9f1a5870..fbde296b15f7e 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/MetricType.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/MetricStat.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex; +package org.opensearch.index.compositeindex.datacube; import org.opensearch.common.annotation.ExperimentalApi; @@ -16,7 +16,7 @@ * @opensearch.experimental */ @ExperimentalApi -public enum MetricType { +public enum MetricStat { COUNT("count"), AVG("avg"), SUM("sum"), @@ -25,7 +25,7 @@ public enum MetricType { private final String typeName; - MetricType(String typeName) { + MetricStat(String typeName) { this.typeName = typeName; } @@ -33,12 +33,12 @@ public String getTypeName() { return typeName; } - public static MetricType fromTypeName(String typeName) { - for (MetricType metric : MetricType.values()) { + public static MetricStat fromTypeName(String typeName) { + for (MetricStat metric : MetricStat.values()) { if (metric.getTypeName().equalsIgnoreCase(typeName)) { return metric; } } - throw new IllegalArgumentException("Invalid metric type: " + typeName); + throw new IllegalArgumentException("Invalid metric stat: " + typeName); } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/package-info.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/package-info.java new file mode 100644 index 0000000000000..320876ea937bf --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/package-info.java @@ -0,0 +1,11 @@ +/* + * 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. + */ +/** + * Core classes for handling data cube indices such as star tree index. + */ +package org.opensearch.index.compositeindex.datacube; diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeField.java similarity index 57% rename from server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeField.java index a29f049e1f8fb..922ddcbea4fe2 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeField.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeField.java @@ -6,16 +6,17 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex.startree; +package org.opensearch.index.compositeindex.datacube.startree; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.index.compositeindex.Dimension; -import org.opensearch.index.compositeindex.Metric; +import org.opensearch.index.compositeindex.datacube.Dimension; +import org.opensearch.index.compositeindex.datacube.Metric; import java.io.IOException; import java.util.List; +import java.util.Objects; /** * Star tree field which contains dimensions, metrics and specs @@ -27,13 +28,13 @@ public class StarTreeField implements ToXContent { private final String name; private final List dimensionsOrder; private final List metrics; - private final StarTreeFieldSpec starTreeFieldSpec; + private final StarTreeFieldConfiguration starTreeConfig; - public StarTreeField(String name, List dimensions, List metrics, StarTreeFieldSpec starTreeFieldSpec) { + public StarTreeField(String name, List dimensions, List metrics, StarTreeFieldConfiguration starTreeConfig) { this.name = name; this.dimensionsOrder = dimensions; this.metrics = metrics; - this.starTreeFieldSpec = starTreeFieldSpec; + this.starTreeConfig = starTreeConfig; } public String getName() { @@ -48,8 +49,8 @@ public List getMetrics() { return metrics; } - public StarTreeFieldSpec getSpec() { - return starTreeFieldSpec; + public StarTreeFieldConfiguration getStarTreeConfig() { + return starTreeConfig; } @Override @@ -57,21 +58,37 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(); builder.field("name", name); if (dimensionsOrder != null && !dimensionsOrder.isEmpty()) { - builder.startObject("ordered_dimensions"); + builder.startArray("ordered_dimensions"); for (Dimension dimension : dimensionsOrder) { dimension.toXContent(builder, params); } - builder.endObject(); + builder.endArray(); } if (metrics != null && !metrics.isEmpty()) { - builder.startObject("metrics"); + builder.startArray("metrics"); for (Metric metric : metrics) { metric.toXContent(builder, params); } - builder.endObject(); + builder.endArray(); } - starTreeFieldSpec.toXContent(builder, params); + starTreeConfig.toXContent(builder, params); builder.endObject(); return builder; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + StarTreeField that = (StarTreeField) o; + return Objects.equals(name, that.name) + && Objects.equals(dimensionsOrder, that.dimensionsOrder) + && Objects.equals(metrics, that.metrics) + && Objects.equals(starTreeConfig, that.starTreeConfig); + } + + @Override + public int hashCode() { + return Objects.hash(name, dimensionsOrder, metrics, starTreeConfig); + } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java similarity index 74% rename from server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java index 09a133f4a6fe5..21a709ab010d6 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeFieldSpec.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex.startree; +package org.opensearch.index.compositeindex.datacube.startree; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.core.xcontent.ToXContent; @@ -15,6 +15,7 @@ import java.io.IOException; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; /** @@ -23,13 +24,13 @@ * @opensearch.experimental */ @ExperimentalApi -public class StarTreeFieldSpec implements ToXContent { +public class StarTreeFieldConfiguration implements ToXContent { private final AtomicInteger maxLeafDocs = new AtomicInteger(); private final List skipStarNodeCreationInDims; private final StarTreeBuildMode buildMode; - public StarTreeFieldSpec(int maxLeafDocs, List skipStarNodeCreationInDims, StarTreeBuildMode buildMode) { + public StarTreeFieldConfiguration(int maxLeafDocs, List skipStarNodeCreationInDims, StarTreeBuildMode buildMode) { this.maxLeafDocs.set(maxLeafDocs); this.skipStarNodeCreationInDims = skipStarNodeCreationInDims; this.buildMode = buildMode; @@ -54,6 +55,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws */ @ExperimentalApi public enum StarTreeBuildMode { + // TODO : remove onheap support unless this proves useful ON_HEAP("onheap"), OFF_HEAP("offheap"); @@ -77,11 +79,6 @@ public static StarTreeBuildMode fromTypeName(String typeName) { } } - @Override - public String toString() { - return buildMode.getTypeName(); - } - public int maxLeafDocs() { return maxLeafDocs.get(); } @@ -93,4 +90,19 @@ public StarTreeBuildMode getBuildMode() { public List getSkipStarNodeCreationInDims() { return skipStarNodeCreationInDims; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + StarTreeFieldConfiguration that = (StarTreeFieldConfiguration) o; + return Objects.equals(maxLeafDocs.get(), that.maxLeafDocs.get()) + && Objects.equals(skipStarNodeCreationInDims, that.skipStarNodeCreationInDims) + && buildMode == that.buildMode; + } + + @Override + public int hashCode() { + return Objects.hash(maxLeafDocs.get(), skipStarNodeCreationInDims, buildMode); + } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java similarity index 68% rename from server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java index 601cf8087f5f2..191d4912d06a8 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/StarTreeIndexSettings.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java @@ -6,18 +6,19 @@ * compatible open source license. */ -package org.opensearch.index.compositeindex.startree; +package org.opensearch.index.compositeindex.datacube.startree; import org.opensearch.common.Rounding; import org.opensearch.common.settings.Setting; -import org.opensearch.index.compositeindex.MetricType; +import org.opensearch.index.compositeindex.datacube.MetricStat; import org.opensearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; import java.util.Arrays; import java.util.List; /** - * Index settings for star tree fields + * Index settings for star tree fields. The settings are final as right now + * there is no support for update of star tree mapping. * * @opensearch.experimental */ @@ -27,7 +28,7 @@ public class StarTreeIndexSettings { * star tree field, we will generate associated star tree index. */ public static final Setting STAR_TREE_MAX_FIELDS_SETTING = Setting.intSetting( - "index.composite.star_tree.max_fields", + "index.composite_index.star_tree.max_fields", 1, 1, 1, @@ -40,7 +41,7 @@ public class StarTreeIndexSettings { * dimensions and associated cardinality has direct effect of star tree index size and query performance. */ public static final Setting STAR_TREE_MAX_DIMENSIONS_SETTING = Setting.intSetting( - "index.composite.star_tree.field.max_dimensions", + "index.composite_index.star_tree.field.max_dimensions", 10, 2, 10, @@ -48,6 +49,18 @@ public class StarTreeIndexSettings { Setting.Property.Final ); + /** + * This setting determines the max number of date intervals that can be part of star tree date field. + */ + public static final Setting STAR_TREE_MAX_DATE_INTERVALS_SETTING = Setting.intSetting( + "index.composite_index.star_tree.field.max_date_intervals", + 3, + 1, + 3, + Setting.Property.IndexScope, + Setting.Property.Final + ); + /** * This setting configures the default "maxLeafDocs" setting of star tree. This affects both query performance and * star tree index size. Lesser the leaves, better the query latency but higher storage size and vice versa @@ -57,7 +70,7 @@ public class StarTreeIndexSettings { * @opensearch.experimental */ public static final Setting STAR_TREE_DEFAULT_MAX_LEAF_DOCS = Setting.intSetting( - "index.composite.star_tree.default.max_leaf_docs", + "index.composite_index.star_tree.default.max_leaf_docs", 10000, 1, Setting.Property.IndexScope, @@ -68,7 +81,7 @@ public class StarTreeIndexSettings { * Default intervals for date dimension as part of star tree fields */ public static final Setting> DEFAULT_DATE_INTERVALS = Setting.listSetting( - "index.composite.star_tree.field.default.date_intervals", + "index.composite_index.star_tree.field.default.date_intervals", Arrays.asList(Rounding.DateTimeUnit.MINUTES_OF_HOUR.shortName(), Rounding.DateTimeUnit.HOUR_OF_DAY.shortName()), StarTreeIndexSettings::getTimeUnit, Setting.Property.IndexScope, @@ -78,23 +91,23 @@ public class StarTreeIndexSettings { /** * Default metrics for metrics as part of star tree fields */ - public static final Setting> DEFAULT_METRICS_LIST = Setting.listSetting( - "index.composite.star_tree.field.default.metrics", + public static final Setting> DEFAULT_METRICS_LIST = Setting.listSetting( + "index.composite_index.star_tree.field.default.metrics", Arrays.asList( - MetricType.AVG.toString(), - MetricType.COUNT.toString(), - MetricType.SUM.toString(), - MetricType.MAX.toString(), - MetricType.MIN.toString() + MetricStat.AVG.toString(), + MetricStat.COUNT.toString(), + MetricStat.SUM.toString(), + MetricStat.MAX.toString(), + MetricStat.MIN.toString() ), - MetricType::fromTypeName, + MetricStat::fromTypeName, Setting.Property.IndexScope, Setting.Property.Final ); public static Rounding.DateTimeUnit getTimeUnit(String expression) { if (!DateHistogramAggregationBuilder.DATE_FIELD_UNITS.containsKey(expression)) { - throw new IllegalArgumentException("unknown calendar interval specified in star tree index config"); + throw new IllegalArgumentException("unknown calendar intervals specified in star tree index mapping"); } return DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expression); } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/package-info.java similarity index 80% rename from server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java rename to server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/package-info.java index 988bca357f32b..4f4e670478e2f 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/startree/package-info.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/package-info.java @@ -8,4 +8,4 @@ /** * Core classes for handling star tree index. */ -package org.opensearch.index.compositeindex.startree; +package org.opensearch.index.compositeindex.datacube.startree; diff --git a/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java b/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java index 99f94cd38b65c..b01555260e760 100644 --- a/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java +++ b/server/src/main/java/org/opensearch/index/mapper/CompositeDataCubeFieldType.java @@ -9,8 +9,8 @@ package org.opensearch.index.mapper; import org.opensearch.common.annotation.ExperimentalApi; -import org.opensearch.index.compositeindex.Dimension; -import org.opensearch.index.compositeindex.Metric; +import org.opensearch.index.compositeindex.datacube.Dimension; +import org.opensearch.index.compositeindex.datacube.Metric; import java.util.ArrayList; import java.util.HashSet; diff --git a/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java b/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java index d66ef0c02cc4a..f52ce29a86dd2 100644 --- a/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java +++ b/server/src/main/java/org/opensearch/index/mapper/CompositeMappedFieldType.java @@ -47,7 +47,26 @@ public CompositeMappedFieldType(String name, List fields, CompositeField * Supported composite field types */ public enum CompositeFieldType { - STAR_TREE + STAR_TREE("star_tree"); + + private final String name; + + CompositeFieldType(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public static CompositeFieldType fromName(String name) { + for (CompositeFieldType metric : CompositeFieldType.values()) { + if (metric.getName().equalsIgnoreCase(name)) { + return metric; + } + } + throw new IllegalArgumentException("Invalid metric stat: " + name); + } } public List fields() { 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 103f6e950efe8..be3adfe8b2c4e 100644 --- a/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java @@ -46,7 +46,7 @@ import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.mapper.MapperService.MergeReason; import java.io.IOException; 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 da5aaf763ad6a..fe9cd598a88fc 100644 --- a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java @@ -11,19 +11,18 @@ import org.apache.lucene.search.Query; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.xcontent.support.XContentMapValues; -import org.opensearch.index.compositeindex.DateDimension; -import org.opensearch.index.compositeindex.Dimension; -import org.opensearch.index.compositeindex.Metric; -import org.opensearch.index.compositeindex.MetricType; -import org.opensearch.index.compositeindex.startree.StarTreeField; -import org.opensearch.index.compositeindex.startree.StarTreeFieldSpec; -import org.opensearch.index.compositeindex.startree.StarTreeIndexSettings; +import org.opensearch.index.compositeindex.datacube.DateDimension; +import org.opensearch.index.compositeindex.datacube.Dimension; +import org.opensearch.index.compositeindex.datacube.Metric; +import org.opensearch.index.compositeindex.datacube.MetricStat; +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.opensearch.index.query.QueryShardContext; import org.opensearch.search.lookup.SearchLookup; import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -41,6 +40,15 @@ @ExperimentalApi public class StarTreeMapper extends ParametrizedFieldMapper { public static final String CONTENT_TYPE = "star_tree"; + public static final String CONFIG = "config"; + public static final String MAX_LEAF_DOCS = "max_leaf_docs"; + public static final String SKIP_STAR_NODE_IN_DIMS = "skip_star_node_creation_for_dimensions"; + public static final String BUILD_MODE = "build_mode"; + public static final String ORDERED_DIMENSIONS = "ordered_dimensions"; + public static final String METRICS = "metrics"; + public static final String NAME = "name"; + public static final String TYPE = "type"; + public static final String STATS = "stats"; @Override public ParametrizedFieldMapper.Builder getMergeBuilder() { @@ -62,31 +70,37 @@ public static class Builder extends ParametrizedFieldMapper.Builder { private static final Set> ALLOWED_METRIC_MAPPER_BUILDERS = Set.of(NumberFieldMapper.Builder.class); @SuppressWarnings("unchecked") - private final Parameter config = new Parameter<>("config", false, () -> null, (name, context, nodeObj) -> { + private final Parameter config = new Parameter<>(CONFIG, false, () -> null, (name, context, nodeObj) -> { if (nodeObj instanceof Map) { Map paramMap = (Map) nodeObj; int maxLeafDocs = XContentMapValues.nodeIntegerValue( - paramMap.get("max_leaf_docs"), + paramMap.get(MAX_LEAF_DOCS), StarTreeIndexSettings.STAR_TREE_DEFAULT_MAX_LEAF_DOCS.get(context.getSettings()) ); if (maxLeafDocs < 1) { throw new IllegalArgumentException( - String.format(Locale.ROOT, "max_leaf_docs [%s] must be greater than 0", maxLeafDocs) + String.format(Locale.ROOT, "%s [%s] must be greater than 0", MAX_LEAF_DOCS, maxLeafDocs) ); } + paramMap.remove(MAX_LEAF_DOCS); List skipStarInDims = Arrays.asList( - XContentMapValues.nodeStringArrayValue( - paramMap.getOrDefault("skip_star_node_creation_for_dimensions", new ArrayList()) - ) + XContentMapValues.nodeStringArrayValue(paramMap.getOrDefault(SKIP_STAR_NODE_IN_DIMS, new ArrayList())) ); - StarTreeFieldSpec.StarTreeBuildMode buildMode = StarTreeFieldSpec.StarTreeBuildMode.fromTypeName( + paramMap.remove(SKIP_STAR_NODE_IN_DIMS); + StarTreeFieldConfiguration.StarTreeBuildMode buildMode = StarTreeFieldConfiguration.StarTreeBuildMode.fromTypeName( XContentMapValues.nodeStringValue( - paramMap.get("build_mode"), - StarTreeFieldSpec.StarTreeBuildMode.OFF_HEAP.getTypeName() + paramMap.get(BUILD_MODE), + StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP.getTypeName() ) ); - List dimensions = buildDimensions(paramMap, context); - List metrics = buildMetrics(paramMap, context); + paramMap.remove(BUILD_MODE); + List dimensions = buildDimensions(name, paramMap, context); + paramMap.remove(ORDERED_DIMENSIONS); + List metrics = buildMetrics(name, paramMap, context); + paramMap.remove(METRICS); + if (paramMap.containsKey(NAME)) { + paramMap.remove(NAME); + } for (String dim : skipStarInDims) { if (dimensions.stream().filter(d -> d.getField().equals(dim)).findAny().isEmpty()) { throw new IllegalArgumentException( @@ -98,7 +112,12 @@ public static class Builder extends ParametrizedFieldMapper.Builder { ); } } - StarTreeFieldSpec spec = new StarTreeFieldSpec(maxLeafDocs, skipStarInDims, buildMode); + StarTreeFieldConfiguration spec = new StarTreeFieldConfiguration(maxLeafDocs, skipStarInDims, buildMode); + DocumentMapperParser.checkNoRemainingFields( + paramMap, + context.indexVersionCreated(), + "Star tree mapping definition has unsupported parameters: " + ); return new StarTreeField(this.name, dimensions, metrics, spec); } else { @@ -112,101 +131,106 @@ public static class Builder extends ParametrizedFieldMapper.Builder { * Build dimensions from mapping */ @SuppressWarnings("unchecked") - private List buildDimensions(Map map, Mapper.TypeParser.ParserContext context) { + private List buildDimensions(String fieldName, Map map, Mapper.TypeParser.ParserContext context) { Object dims = XContentMapValues.extractValue("ordered_dimensions", map); if (dims == null) { throw new IllegalArgumentException( - String.format(Locale.ROOT, "ordered_dimensions is required for star tree field [%s]", this.name) + String.format(Locale.ROOT, "ordered_dimensions is required for star tree field [%s]", fieldName) ); } List dimensions = new LinkedList<>(); - if (dims instanceof LinkedHashMap) { - if (((LinkedHashMap) dims).size() > context.getSettings() - .getAsInt(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 10)) { + if (dims instanceof List) { + List dimList = (List) dims; + if (dimList.size() > context.getSettings().getAsInt(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 10)) { throw new IllegalArgumentException( String.format( Locale.ROOT, "ordered_dimensions cannot have more than %s dimensions for star tree field [%s]", context.getSettings().getAsInt(StarTreeIndexSettings.STAR_TREE_MAX_DIMENSIONS_SETTING.getKey(), 10), - this.name + fieldName ) ); } - if (((LinkedHashMap) dims).size() < 2) { + if (dimList.size() < 2) { throw new IllegalArgumentException( - String.format(Locale.ROOT, "Atleast two dimensions are required to build star tree index field [%s]", this.name) + String.format(Locale.ROOT, "Atleast two dimensions are required to build star tree index field [%s]", fieldName) ); } - for (Map.Entry dim : ((LinkedHashMap) dims).entrySet()) { - if (this.objbuilder == null || this.objbuilder.mappersBuilders == null) { - if (dim.getValue() instanceof Map) { - Map dimObj = ((Map) dim.getValue()); - String type = XContentMapValues.nodeStringValue(dimObj.get("type")); - dimensions.add(getDimension(type, dim, context)); - } else { - throw new MapperParsingException( - String.format(Locale.ROOT, "unable to parse ordered_dimensions for star tree field [%s]", this.name) - ); - } - } else { - Optional dimBuilder = findMapperBuilderByName(dim.getKey(), this.objbuilder.mappersBuilders); - if (dimBuilder.isEmpty()) { - throw new IllegalArgumentException(String.format(Locale.ROOT, "unknown dimension field [%s]", dim.getKey())); - } - if (!isBuilderAllowedForDimension(dimBuilder.get())) { - throw new IllegalArgumentException( - String.format( - Locale.ROOT, - "unsupported field type associated with dimension [%s] as part of star tree field [%s]", - dim.getKey(), - name - ) - ); - } - dimensions.add(getDimension(dimBuilder.get(), dim, context)); - } + for (Object dim : dimList) { + dimensions.add(getDimension(fieldName, dim, context)); } } else { throw new MapperParsingException( - String.format(Locale.ROOT, "unable to parse ordered_dimensions for star tree field [%s]", this.name) + String.format(Locale.ROOT, "unable to parse ordered_dimensions for star tree field [%s]", fieldName) ); } return dimensions; } /** - * Get dimension instance based on the builder type + * Get dimension based on mapping */ - private Dimension getDimension(Mapper.Builder builder, Map.Entry dim, Mapper.TypeParser.ParserContext context) { - String name = dim.getKey(); + @SuppressWarnings("unchecked") + private Dimension getDimension(String fieldName, Object dimensionMapping, Mapper.TypeParser.ParserContext context) { Dimension dimension; - if (builder instanceof DateFieldMapper.Builder) { - dimension = new DateDimension(dim, context); - } - // Numeric dimension - default - else if (builder instanceof NumberFieldMapper.Builder) { - dimension = new Dimension(name); + Map dimensionMap = (Map) dimensionMapping; + String name = (String) XContentMapValues.extractValue(NAME, dimensionMap); + dimensionMap.remove(NAME); + if (this.objbuilder == null || this.objbuilder.mappersBuilders == null) { + String type = (String) XContentMapValues.extractValue(TYPE, dimensionMap); + dimensionMap.remove(TYPE); + if (type == null) { + throw new MapperParsingException( + String.format(Locale.ROOT, "unable to parse ordered_dimensions for star tree field [%s]", fieldName) + ); + } + if (type.equals(DateDimension.DATE)) { + dimension = new DateDimension(name, dimensionMap, context); + } else if (type.equals(Dimension.NUMERIC)) { + dimension = new Dimension(name); + } else { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "unsupported field type associated with dimension [%s] as part of star tree field [%s]", + name, + fieldName + ) + ); + } + return dimension; } else { - throw new IllegalArgumentException( - String.format(Locale.ROOT, "unsupported field type associated with star tree dimension [%s]", name) - ); - } - return dimension; - } - - /** - * Get dimension based on field type - */ - private Dimension getDimension(String type, Map.Entry dim, Mapper.TypeParser.ParserContext c) { - String name = dim.getKey(); - Dimension dimension; - if (type.equals("date")) { - dimension = new DateDimension(dim, c); - } - // Numeric dimension - default - else { - dimension = new Dimension(name); + Optional dimBuilder = findMapperBuilderByName(name, this.objbuilder.mappersBuilders); + if (dimBuilder.isEmpty()) { + throw new IllegalArgumentException(String.format(Locale.ROOT, "unknown dimension field [%s]", name)); + } + if (!isBuilderAllowedForDimension(dimBuilder.get())) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "unsupported field type associated with dimension [%s] as part of star tree field [%s]", + name, + fieldName + ) + ); + } + if (dimBuilder.get() instanceof DateFieldMapper.Builder) { + dimension = new DateDimension(name, dimensionMap, context); + } + // Numeric dimension - default + else if (dimBuilder.get() instanceof NumberFieldMapper.Builder) { + dimension = new Dimension(name); + } else { + throw new IllegalArgumentException( + String.format(Locale.ROOT, "unsupported field type associated with star tree dimension [%s]", name) + ); + } } + DocumentMapperParser.checkNoRemainingFields( + dimensionMap, + context.indexVersionCreated(), + "Star tree mapping definition has unsupported parameters: " + ); return dimension; } @@ -214,29 +238,38 @@ private Dimension getDimension(String type, Map.Entry dim, Mappe * Build metrics from mapping */ @SuppressWarnings("unchecked") - private List buildMetrics(Map map, Mapper.TypeParser.ParserContext context) { + private List buildMetrics(String fieldName, Map map, Mapper.TypeParser.ParserContext context) { List metrics = new LinkedList<>(); - Object metricsFromInput = XContentMapValues.extractValue("metrics", map); + Object metricsFromInput = XContentMapValues.extractValue(METRICS, map); if (metricsFromInput == null) { throw new IllegalArgumentException( - String.format(Locale.ROOT, "metrics section is required for star tree field [%s]", this.name) + String.format(Locale.ROOT, "metrics section is required for star tree field [%s]", fieldName) ); } - if (metricsFromInput instanceof LinkedHashMap) { - for (Map.Entry metric : ((LinkedHashMap) metricsFromInput).entrySet()) { + if (metricsFromInput instanceof List) { + List metricsList = (List) metricsFromInput; + for (Object metric : metricsList) { + Map metricMap = (Map) metric; + String name = (String) XContentMapValues.extractValue(NAME, metricMap); + metricMap.remove(NAME); if (objbuilder == null || objbuilder.mappersBuilders == null) { - metrics.add(getMetric(metric, context)); + metrics.add(getMetric(name, metricMap, context)); } else { - Optional meticBuilder = findMapperBuilderByName(metric.getKey(), this.objbuilder.mappersBuilders); + Optional meticBuilder = findMapperBuilderByName(name, this.objbuilder.mappersBuilders); if (meticBuilder.isEmpty()) { - throw new IllegalArgumentException(String.format(Locale.ROOT, "unknown metric field [%s]", metric.getKey())); + throw new IllegalArgumentException(String.format(Locale.ROOT, "unknown metric field [%s]", name)); } if (!isBuilderAllowedForMetric(meticBuilder.get())) { throw new IllegalArgumentException( String.format(Locale.ROOT, "non-numeric field type is associated with star tree metric [%s]", this.name) ); } - metrics.add(getMetric(metric, context)); + metrics.add(getMetric(name, metricMap, context)); + DocumentMapperParser.checkNoRemainingFields( + metricMap, + context.indexVersionCreated(), + "Star tree mapping definition has unsupported parameters: " + ); } } } else { @@ -247,20 +280,19 @@ private List buildMetrics(Map map, Mapper.TypeParser.Par } @SuppressWarnings("unchecked") - private Metric getMetric(Map.Entry map, Mapper.TypeParser.ParserContext context) { - String name = map.getKey(); - List metricTypes; - List metricStrings = XContentMapValues.extractRawValues("metrics", (Map) map.getValue()) + private Metric getMetric(String name, Map metric, Mapper.TypeParser.ParserContext context) { + List metricTypes; + List metricStrings = XContentMapValues.extractRawValues(STATS, metric) .stream() .map(Object::toString) .collect(Collectors.toList()); - + metric.remove(STATS); if (metricStrings.isEmpty()) { metricTypes = new ArrayList<>(StarTreeIndexSettings.DEFAULT_METRICS_LIST.get(context.getSettings())); } else { - Set metricSet = new LinkedHashSet<>(); + Set metricSet = new LinkedHashSet<>(); for (String metricString : metricStrings) { - metricSet.add(MetricType.fromTypeName(metricString)); + metricSet.add(MetricStat.fromTypeName(metricString)); } metricTypes = new ArrayList<>(metricSet); } @@ -367,15 +399,15 @@ protected void parseCreateField(ParseContext context) { @ExperimentalApi public static final class StarTreeFieldType extends CompositeDataCubeFieldType { - private final StarTreeFieldSpec starTreeFieldSpec; + private final StarTreeFieldConfiguration starTreeConfig; public StarTreeFieldType(String name, StarTreeField starTreeField) { super(name, starTreeField.getDimensionsOrder(), starTreeField.getMetrics(), CompositeFieldType.STAR_TREE); - this.starTreeFieldSpec = starTreeField.getSpec(); + this.starTreeConfig = starTreeField.getStarTreeConfig(); } - public StarTreeFieldSpec getStarTreeFieldSpec() { - return starTreeFieldSpec; + public StarTreeFieldConfiguration getStarTreeConfig() { + return starTreeConfig; } @Override diff --git a/server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java b/server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java deleted file mode 100644 index 2e838ddb44807..0000000000000 --- a/server/src/main/java/org/opensearch/indices/DefaultCompositeIndexSettings.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * 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.indices; - -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.index.compositeindex.CompositeIndexSettings; - -/** - * Utility to provide a {@link CompositeIndexSettings} instance containing all defaults - */ -public class DefaultCompositeIndexSettings { - private DefaultCompositeIndexSettings() {} - - public static final CompositeIndexSettings INSTANCE = new CompositeIndexSettings( - Settings.EMPTY, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); -} diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index b1fb2f77e2981..bce33dd24dcc1 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -2036,4 +2036,8 @@ private TimeValue getClusterDefaultRefreshInterval() { public RemoteStoreSettings getRemoteStoreSettings() { return this.remoteStoreSettings; } + + public CompositeIndexSettings getCompositeIndexSettings() { + return this.compositeIndexSettings; + } } diff --git a/server/src/test/java/org/opensearch/index/IndexModuleTests.java b/server/src/test/java/org/opensearch/index/IndexModuleTests.java index 8f45a872e752c..3f5aacc5d2523 100644 --- a/server/src/test/java/org/opensearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/opensearch/index/IndexModuleTests.java @@ -80,6 +80,7 @@ import org.opensearch.index.cache.query.DisabledQueryCache; import org.opensearch.index.cache.query.IndexQueryCache; import org.opensearch.index.cache.query.QueryCache; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.engine.Engine; import org.opensearch.index.engine.EngineConfigFactory; import org.opensearch.index.engine.InternalEngineFactory; @@ -99,7 +100,6 @@ import org.opensearch.index.translog.InternalTranslogFactory; import org.opensearch.index.translog.RemoteBlobStoreInternalTranslogFactory; import org.opensearch.index.translog.TranslogFactory; -import org.opensearch.indices.DefaultCompositeIndexSettings; import org.opensearch.indices.DefaultRemoteStoreSettings; import org.opensearch.indices.IndicesModule; import org.opensearch.indices.IndicesQueryCache; @@ -266,7 +266,7 @@ private IndexService newIndexService(IndexModule module) throws IOException { () -> IndexSettings.DEFAULT_REFRESH_INTERVAL, DefaultRecoverySettings.INSTANCE, DefaultRemoteStoreSettings.INSTANCE, - DefaultCompositeIndexSettings.INSTANCE + CompositeIndexSettings.DefaultCompositeIndexSettings.INSTANCE ); } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java new file mode 100644 index 0000000000000..3691abea599ac --- /dev/null +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java @@ -0,0 +1,364 @@ +/* + * 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.index.compositeindex.datacube.startree; + +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.index.Index; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.IndexService; +import org.opensearch.index.compositeindex.CompositeIndexSettings; +import org.opensearch.index.compositeindex.datacube.DateDimension; +import org.opensearch.index.compositeindex.datacube.MetricStat; +import org.opensearch.index.mapper.CompositeMappedFieldType; +import org.opensearch.index.mapper.MapperParsingException; +import org.opensearch.index.mapper.StarTreeMapper; +import org.opensearch.indices.IndicesService; +import org.opensearch.test.OpenSearchIntegTestCase; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; + +public class StarTreeMappingIntegTests extends OpenSearchIntegTestCase { + private static final String TEST_INDEX = "test"; + + private static XContentBuilder createMinimalTestMapping(boolean invalidDim, boolean invalidMetric, boolean keywordDim) { + try { + return jsonBuilder().startObject() + .startObject("composite") + .startObject("startree-1") + .field("type", "star_tree") + .startObject("config") + .startArray("ordered_dimensions") + .startObject() + .field("name", "timestamp") + .endObject() + .startObject() + .field("name", getDim(invalidDim, keywordDim)) + .endObject() + .endArray() + .startArray("metrics") + .startObject() + .field("name", getDim(invalidMetric, false)) + .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") + .field("type", "integer") + .field("doc_values", false) + .endObject() + .startObject("keyword_dv") + .field("type", "keyword") + .field("doc_values", true) + .endObject() + .startObject("keyword") + .field("type", "keyword") + .field("doc_values", false) + .endObject() + .endObject() + .endObject(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private static XContentBuilder createTestMappingWithoutStarTree(boolean invalidDim, boolean invalidMetric, boolean keywordDim) { + try { + return jsonBuilder().startObject() + .startObject("properties") + .startObject("timestamp") + .field("type", "date") + .endObject() + .startObject("numeric_dv") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .startObject("numeric") + .field("type", "integer") + .field("doc_values", false) + .endObject() + .startObject("keyword_dv") + .field("type", "keyword") + .field("doc_values", true) + .endObject() + .startObject("keyword") + .field("type", "keyword") + .field("doc_values", false) + .endObject() + .endObject() + .endObject(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private static XContentBuilder createUpdateTestMapping(boolean changeDim, boolean sameStarTree) { + try { + return jsonBuilder().startObject() + .startObject("composite") + .startObject(sameStarTree ? "startree-1" : "startree-2") + .field("type", "star_tree") + .startObject("config") + .startArray("ordered_dimensions") + .startObject() + .field("name", "timestamp") + .endObject() + .startObject() + .field("name", changeDim ? "numeric_new" : getDim(false, false)) + .endObject() + .endArray() + .startArray("metrics") + .startObject() + .field("name", getDim(false, false)) + .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") + .field("type", "integer") + .field("doc_values", false) + .endObject() + .startObject("numeric_new") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .startObject("keyword_dv") + .field("type", "keyword") + .field("doc_values", true) + .endObject() + .startObject("keyword") + .field("type", "keyword") + .field("doc_values", false) + .endObject() + .endObject() + .endObject(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private static String getDim(boolean hasDocValues, boolean isKeyword) { + if (hasDocValues) { + return "numeric"; + } else if (isKeyword) { + return "keyword"; + } + return "numeric_dv"; + } + + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.STAR_TREE_INDEX, "true").build(); + } + + @Before + public final void setupNodeSettings() { + Settings request = Settings.builder().put(CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING.getKey(), true).build(); + assertAcked(client().admin().cluster().prepareUpdateSettings().setPersistentSettings(request).get()); + } + + public void testValidCompositeIndex() { + prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + Iterable dataNodeInstances = internalCluster().getDataNodeInstances(IndicesService.class); + for (IndicesService service : dataNodeInstances) { + final Index index = resolveIndex("test"); + if (service.hasIndex(index)) { + IndexService indexService = service.indexService(index); + Set fts = indexService.mapperService().getCompositeFieldTypes(); + + for (CompositeMappedFieldType ft : fts) { + assertTrue(ft instanceof StarTreeMapper.StarTreeFieldType); + StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) ft; + assertEquals("timestamp", starTreeFieldType.getDimensions().get(0).getField()); + assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); + DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); + List expectedTimeUnits = Arrays.asList( + Rounding.DateTimeUnit.MINUTES_OF_HOUR, + Rounding.DateTimeUnit.HOUR_OF_DAY + ); + assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("numeric_dv", starTreeFieldType.getDimensions().get(1).getField()); + assertEquals("numeric_dv", starTreeFieldType.getMetrics().get(0).getField()); + List expectedMetrics = Arrays.asList( + MetricStat.AVG, + MetricStat.COUNT, + MetricStat.SUM, + MetricStat.MAX, + MetricStat.MIN + ); + assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); + assertEquals(10000, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); + assertEquals( + StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, + starTreeFieldType.getStarTreeConfig().getBuildMode() + ); + assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); + } + } + } + } + + public void testUpdateIndexWithAdditionOfStarTree() { + prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().preparePutMapping(TEST_INDEX).setSource(createUpdateTestMapping(false, false)).get() + ); + assertEquals("Index cannot have more than [1] star tree fields", ex.getMessage()); + } + + public void testUpdateIndexWithNewerStarTree() { + prepareCreate(TEST_INDEX).setMapping(createTestMappingWithoutStarTree(false, false, false)).get(); + + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().preparePutMapping(TEST_INDEX).setSource(createUpdateTestMapping(false, false)).get() + ); + assertEquals( + "Composite fields must be specified during index creation, addition of new composite fields during update is not supported", + ex.getMessage() + ); + } + + public void testUpdateIndexWhenMappingIsDifferent() { + prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + + // update some field in the mapping + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> client().admin().indices().preparePutMapping(TEST_INDEX).setSource(createUpdateTestMapping(true, true)).get() + ); + assertTrue(ex.getMessage().contains("Cannot update parameter [config] from")); + } + + public void testUpdateIndexWhenMappingIsSame() { + prepareCreate(TEST_INDEX).setMapping(createMinimalTestMapping(false, false, false)).get(); + + // update some field in the mapping + AcknowledgedResponse putMappingResponse = client().admin() + .indices() + .preparePutMapping(TEST_INDEX) + .setSource(createMinimalTestMapping(false, false, false)) + .get(); + assertAcked(putMappingResponse); + + Iterable dataNodeInstances = internalCluster().getDataNodeInstances(IndicesService.class); + for (IndicesService service : dataNodeInstances) { + final Index index = resolveIndex("test"); + if (service.hasIndex(index)) { + IndexService indexService = service.indexService(index); + Set fts = indexService.mapperService().getCompositeFieldTypes(); + + for (CompositeMappedFieldType ft : fts) { + assertTrue(ft instanceof StarTreeMapper.StarTreeFieldType); + StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) ft; + assertEquals("timestamp", starTreeFieldType.getDimensions().get(0).getField()); + assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); + DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); + List expectedTimeUnits = Arrays.asList( + Rounding.DateTimeUnit.MINUTES_OF_HOUR, + Rounding.DateTimeUnit.HOUR_OF_DAY + ); + assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("numeric_dv", starTreeFieldType.getDimensions().get(1).getField()); + assertEquals("numeric_dv", starTreeFieldType.getMetrics().get(0).getField()); + List expectedMetrics = Arrays.asList( + MetricStat.AVG, + MetricStat.COUNT, + MetricStat.SUM, + MetricStat.MAX, + MetricStat.MIN + ); + assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); + assertEquals(10000, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); + assertEquals( + StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, + starTreeFieldType.getStarTreeConfig().getBuildMode() + ); + assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); + } + } + } + } + + public void testInvalidDimCompositeIndex() { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> prepareCreate(TEST_INDEX).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", + ex.getMessage() + ); + } + + public void testUnsupportedDim() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> prepareCreate(TEST_INDEX).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]", + ex.getMessage() + ); + } + + public void testInvalidMetric() { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> prepareCreate(TEST_INDEX).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", + ex.getMessage() + ); + } + + @After + public final void cleanupNodeSettings() { + assertAcked( + client().admin() + .cluster() + .prepareUpdateSettings() + .setPersistentSettings(Settings.builder().putNull("*")) + .setTransientSettings(Settings.builder().putNull("*")) + ); + } + +} 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 66f9a19c5b8b8..6bb9925a7c4d8 100644 --- a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java @@ -13,9 +13,9 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.index.compositeindex.DateDimension; -import org.opensearch.index.compositeindex.MetricType; -import org.opensearch.index.compositeindex.startree.StarTreeFieldSpec; +import org.opensearch.index.compositeindex.datacube.DateDimension; +import org.opensearch.index.compositeindex.datacube.MetricStat; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; import org.junit.After; import org.junit.Before; @@ -57,11 +57,11 @@ public void testValidStarTree() throws IOException { assertEquals(expectedTimeUnits, dateDim.getIntervals()); assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); assertEquals("size", starTreeFieldType.getMetrics().get(0).getField()); - List expectedMetrics = Arrays.asList(MetricType.SUM, MetricType.AVG); + List expectedMetrics = Arrays.asList(MetricStat.SUM, MetricStat.AVG); assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); - assertEquals(100, starTreeFieldType.getStarTreeFieldSpec().maxLeafDocs()); - assertEquals(StarTreeFieldSpec.StarTreeBuildMode.ON_HEAP, starTreeFieldType.getStarTreeFieldSpec().getBuildMode()); - assertEquals(Arrays.asList("@timestamp", "status"), starTreeFieldType.getStarTreeFieldSpec().getSkipStarNodeCreationInDims()); + assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); + assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.ON_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); + assertEquals(Arrays.asList("@timestamp", "status"), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); } } @@ -80,17 +80,17 @@ public void testValidStarTreeDefaults() throws IOException { assertEquals(expectedTimeUnits, dateDim.getIntervals()); assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); assertEquals("status", starTreeFieldType.getMetrics().get(0).getField()); - List expectedMetrics = Arrays.asList( - MetricType.AVG, - MetricType.COUNT, - MetricType.SUM, - MetricType.MAX, - MetricType.MIN + List expectedMetrics = Arrays.asList( + MetricStat.AVG, + MetricStat.COUNT, + MetricStat.SUM, + MetricStat.MAX, + MetricStat.MIN ); assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); - assertEquals(10000, starTreeFieldType.getStarTreeFieldSpec().maxLeafDocs()); - assertEquals(StarTreeFieldSpec.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeFieldSpec().getBuildMode()); - assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeFieldSpec().getSkipStarNodeCreationInDims()); + assertEquals(10000, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); + assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); + assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); } } @@ -121,6 +121,17 @@ public void testNoMetrics() { ); } + public void testInvalidParam() { + MapperParsingException ex = expectThrows( + MapperParsingException.class, + () -> createMapperService(getInvalidMapping(false, false, false, false, true)) + ); + assertEquals( + "Failed to parse mapping [_doc]: Star tree mapping definition has unsupported parameters: [invalid : {invalid=invalid}]", + ex.getMessage() + ); + } + public void testNoDims() { MapperParsingException ex = expectThrows( MapperParsingException.class, @@ -206,24 +217,27 @@ private XContentBuilder getExpandedMapping(String dim, String metric) throws IOE b.value("status"); } b.endArray(); - b.startObject("ordered_dimensions"); - b.startObject("@timestamp"); - b.startArray("calendar_interval"); + b.startArray("ordered_dimensions"); + b.startObject(); + b.field("name", "@timestamp"); + b.startArray("calendar_intervals"); b.value("day"); b.value("month"); b.endArray(); b.endObject(); - b.startObject(dim); + b.startObject(); + b.field("name", dim); b.endObject(); - b.endObject(); - b.startObject("metrics"); - b.startObject(metric); + b.endArray(); b.startArray("metrics"); + b.startObject(); + b.field("name", metric); + b.startArray("stats"); b.value("sum"); b.value("avg"); b.endArray(); b.endObject(); - b.endObject(); + b.endArray(); b.endObject(); b.endObject(); b.endObject(); @@ -253,20 +267,24 @@ private XContentBuilder getMinMapping(boolean isEmptyDims, boolean isEmptyMetric b.field("type", "star_tree"); b.startObject("config"); if (!isEmptyDims) { - b.startObject("ordered_dimensions"); - b.startObject("@timestamp"); - b.endObject(); - b.startObject("status"); + b.startArray("ordered_dimensions"); + b.startObject(); + b.field("name", "@timestamp"); b.endObject(); + b.startObject(); + b.field("name", "status"); b.endObject(); + b.endArray(); } if (!isEmptyMetrics) { - b.startObject("metrics"); - b.startObject("status"); - b.endObject(); - b.startObject("metric_field"); + b.startArray("metrics"); + b.startObject(); + b.field("name", "status"); b.endObject(); + b.startObject(); + b.field("name", "metric_field"); b.endObject(); + b.endArray(); } b.endObject(); b.endObject(); @@ -289,8 +307,13 @@ private XContentBuilder getMinMapping(boolean isEmptyDims, boolean isEmptyMetric }); } - private XContentBuilder getInvalidMapping(boolean singleDim, boolean invalidSkipDims, boolean invalidDimType, boolean invalidMetricType) - throws IOException { + private XContentBuilder getInvalidMapping( + boolean singleDim, + boolean invalidSkipDims, + boolean invalidDimType, + boolean invalidMetricType, + boolean invalidParam + ) throws IOException { return topMapping(b -> { b.startObject("composite"); b.startObject("startree"); @@ -305,20 +328,29 @@ private XContentBuilder getInvalidMapping(boolean singleDim, boolean invalidSkip b.value("status"); } b.endArray(); - b.startObject("ordered_dimensions"); + if (invalidParam) { + b.startObject("invalid"); + b.field("invalid", "invalid"); + b.endObject(); + } + b.startArray("ordered_dimensions"); if (!singleDim) { - b.startObject("@timestamp"); + b.startObject(); + b.field("name", "@timestamp"); b.endObject(); } - b.startObject("status"); + b.startObject(); + b.field("name", "status"); b.endObject(); + b.endArray(); + b.startArray("metrics"); + b.startObject(); + b.field("name", "status"); b.endObject(); - b.startObject("metrics"); - b.startObject("status"); - b.endObject(); - b.startObject("metric_field"); - b.endObject(); + b.startObject(); + b.field("name", "metric_field"); b.endObject(); + b.endArray(); b.endObject(); b.endObject(); b.endObject(); @@ -345,6 +377,11 @@ private XContentBuilder getInvalidMapping(boolean singleDim, boolean invalidSkip }); } + private XContentBuilder getInvalidMapping(boolean singleDim, boolean invalidSkipDims, boolean invalidDimType, boolean invalidMetricType) + throws IOException { + return getInvalidMapping(singleDim, invalidSkipDims, invalidDimType, invalidMetricType, false); + } + protected boolean supportsOrIgnoresBoost() { return false; } @@ -402,16 +439,19 @@ public void testEmptyName() { protected void minimalMapping(XContentBuilder b) throws IOException { b.field("type", "star_tree"); b.startObject("config"); - b.startObject("ordered_dimensions"); - b.startObject("size"); - b.endObject(); - b.startObject("status"); - b.endObject(); + b.startArray("ordered_dimensions"); + b.startObject(); + b.field("name", "size"); b.endObject(); - b.startObject("metrics"); - b.startObject("status"); + b.startObject(); + b.field("name", "status"); b.endObject(); + b.endArray(); + b.startArray("metrics"); + b.startObject(); + b.field("name", "status"); b.endObject(); + b.endArray(); b.endObject(); } diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 01ed36e0f67d8..15170ba9388ac 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -185,6 +185,7 @@ import org.opensearch.index.SegmentReplicationPressureService; import org.opensearch.index.SegmentReplicationStatsTracker; import org.opensearch.index.analysis.AnalysisRegistry; +import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.remote.RemoteStorePressureService; import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory; import org.opensearch.index.seqno.GlobalCheckpointSyncAction; @@ -192,7 +193,6 @@ import org.opensearch.index.shard.PrimaryReplicaSyncer; import org.opensearch.index.store.RemoteSegmentStoreDirectoryFactory; import org.opensearch.index.store.remote.filecache.FileCacheStats; -import org.opensearch.indices.DefaultCompositeIndexSettings; import org.opensearch.indices.DefaultRemoteStoreSettings; import org.opensearch.indices.IndicesModule; import org.opensearch.indices.IndicesService; @@ -2080,7 +2080,7 @@ public void onFailure(final Exception e) { DefaultRecoverySettings.INSTANCE, new CacheModule(new ArrayList<>(), settings).getCacheService(), DefaultRemoteStoreSettings.INSTANCE, - DefaultCompositeIndexSettings.INSTANCE + CompositeIndexSettings.DefaultCompositeIndexSettings.INSTANCE ); final RecoverySettings recoverySettings = new RecoverySettings(settings, clusterSettings); snapshotShardsService = new SnapshotShardsService( From 680875e80bed191455c09b14d7d54ba364266d54 Mon Sep 17 00:00:00 2001 From: Bharathwaj G Date: Tue, 25 Jun 2024 14:02:22 +0530 Subject: [PATCH 4/4] addressing review comments Signed-off-by: Bharathwaj G --- .../main/java/org/opensearch/index/IndexModule.java | 9 ++++++--- .../compositeindex/CompositeIndexSettings.java | 12 ------------ .../compositeindex/CompositeIndexValidator.java | 13 +++++-------- .../startree/StarTreeFieldConfiguration.java | 8 ++++---- .../org/opensearch/index/mapper/StarTreeMapper.java | 5 ++--- .../java/org/opensearch/indices/IndicesService.java | 9 +++++---- .../java/org/opensearch/index/IndexModuleTests.java | 4 +--- .../startree/StarTreeMappingIntegTests.java | 4 ++-- .../index/mapper/StarTreeMapperTests.java | 8 ++++++-- .../snapshots/SnapshotResiliencyTests.java | 4 +--- 10 files changed, 32 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/IndexModule.java b/server/src/main/java/org/opensearch/index/IndexModule.java index a586c198cae58..09b904394ee09 100644 --- a/server/src/main/java/org/opensearch/index/IndexModule.java +++ b/server/src/main/java/org/opensearch/index/IndexModule.java @@ -312,6 +312,7 @@ public Iterator> settings() { private final BooleanSupplier allowExpensiveQueries; private final Map recoveryStateFactories; private final FileCache fileCache; + private final CompositeIndexSettings compositeIndexSettings; /** * Construct the index module for the index with the specified index settings. The index module contains extension points for plugins @@ -331,7 +332,8 @@ public IndexModule( final BooleanSupplier allowExpensiveQueries, final IndexNameExpressionResolver expressionResolver, final Map recoveryStateFactories, - final FileCache fileCache + final FileCache fileCache, + final CompositeIndexSettings compositeIndexSettings ) { this.indexSettings = indexSettings; this.analysisRegistry = analysisRegistry; @@ -344,6 +346,7 @@ public IndexModule( this.expressionResolver = expressionResolver; this.recoveryStateFactories = recoveryStateFactories; this.fileCache = fileCache; + this.compositeIndexSettings = compositeIndexSettings; } public IndexModule( @@ -365,6 +368,7 @@ public IndexModule( allowExpensiveQueries, expressionResolver, recoveryStateFactories, + null, null ); } @@ -680,8 +684,7 @@ public IndexService newIndexService( BiFunction translogFactorySupplier, Supplier clusterDefaultRefreshIntervalSupplier, RecoverySettings recoverySettings, - RemoteStoreSettings remoteStoreSettings, - CompositeIndexSettings compositeIndexSettings + RemoteStoreSettings remoteStoreSettings ) throws IOException { final IndexEventListener eventListener = freeze(); Function> readerWrapperFactory = indexReaderWrapper 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 761f3c76ac15e..014dd22426a10 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java @@ -52,16 +52,4 @@ private void starTreeIndexCreationEnabled(boolean value) { public boolean isStarTreeIndexCreationEnabled() { return starTreeIndexCreationEnabled; } - - /** - * Utility to provide a {@link CompositeIndexSettings} instance containing all defaults - */ - public static class DefaultCompositeIndexSettings { - private DefaultCompositeIndexSettings() {} - - public static final CompositeIndexSettings INSTANCE = new CompositeIndexSettings( - Settings.EMPTY, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - } } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java index 26971f96677af..9547d2ac9cfce 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java @@ -13,7 +13,6 @@ import org.opensearch.index.compositeindex.datacube.Dimension; import org.opensearch.index.compositeindex.datacube.Metric; import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; -import org.opensearch.index.mapper.CompositeDataCubeFieldType; import org.opensearch.index.mapper.CompositeMappedFieldType; import org.opensearch.index.mapper.MappedFieldType; import org.opensearch.index.mapper.MapperService; @@ -57,9 +56,7 @@ private static void validateStarTreeMappings( IndexSettings indexSettings ) { Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); - if (mapperService.getCompositeFieldTypes().size() > StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get( - indexSettings.getSettings() - )) { + if (compositeFieldTypes.size() > StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get(indexSettings.getSettings())) { throw new IllegalArgumentException( String.format( Locale.ROOT, @@ -70,7 +67,7 @@ private static void validateStarTreeMappings( } for (CompositeMappedFieldType compositeFieldType : compositeFieldTypes) { if (!(compositeFieldType instanceof StarTreeMapper.StarTreeFieldType)) { - return; + continue; } if (!compositeIndexSettings.isStarTreeIndexCreationEnabled()) { throw new IllegalArgumentException( @@ -81,7 +78,7 @@ private static void validateStarTreeMappings( ) ); } - CompositeDataCubeFieldType dataCubeFieldType = (CompositeDataCubeFieldType) compositeFieldType; + StarTreeMapper.StarTreeFieldType dataCubeFieldType = (StarTreeMapper.StarTreeFieldType) compositeFieldType; for (Dimension dim : dataCubeFieldType.getDimensions()) { MappedFieldType ft = mapperService.fieldType(dim.getField()); if (ft == null) { @@ -89,7 +86,7 @@ private static void validateStarTreeMappings( String.format(Locale.ROOT, "unknown dimension field [%s] as part of star tree field", dim.getField()) ); } - if (!ft.isAggregatable()) { + if (ft.isAggregatable() == false) { throw new IllegalArgumentException( String.format( Locale.ROOT, @@ -107,7 +104,7 @@ private static void validateStarTreeMappings( String.format(Locale.ROOT, "unknown metric field [%s] as part of star tree field", metric.getField()) ); } - if (!ft.isAggregatable()) { + if (ft.isAggregatable() == false) { throw new IllegalArgumentException( String.format( Locale.ROOT, diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java index 21a709ab010d6..5dd066b34f108 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeFieldConfiguration.java @@ -13,9 +13,9 @@ import org.opensearch.core.xcontent.XContentBuilder; import java.io.IOException; -import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; /** @@ -27,10 +27,10 @@ public class StarTreeFieldConfiguration implements ToXContent { private final AtomicInteger maxLeafDocs = new AtomicInteger(); - private final List skipStarNodeCreationInDims; + private final Set skipStarNodeCreationInDims; private final StarTreeBuildMode buildMode; - public StarTreeFieldConfiguration(int maxLeafDocs, List skipStarNodeCreationInDims, StarTreeBuildMode buildMode) { + public StarTreeFieldConfiguration(int maxLeafDocs, Set skipStarNodeCreationInDims, StarTreeBuildMode buildMode) { this.maxLeafDocs.set(maxLeafDocs); this.skipStarNodeCreationInDims = skipStarNodeCreationInDims; this.buildMode = buildMode; @@ -87,7 +87,7 @@ public StarTreeBuildMode getBuildMode() { return buildMode; } - public List getSkipStarNodeCreationInDims() { + public Set getSkipStarNodeCreationInDims() { return skipStarNodeCreationInDims; } 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 fe9cd598a88fc..fbd3fa6b8b1d3 100644 --- a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java @@ -22,7 +22,6 @@ import org.opensearch.search.lookup.SearchLookup; import java.util.ArrayList; -import java.util.Arrays; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -83,8 +82,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder { ); } paramMap.remove(MAX_LEAF_DOCS); - List skipStarInDims = Arrays.asList( - XContentMapValues.nodeStringArrayValue(paramMap.getOrDefault(SKIP_STAR_NODE_IN_DIMS, new ArrayList())) + Set skipStarInDims = new LinkedHashSet<>( + List.of(XContentMapValues.nodeStringArrayValue(paramMap.getOrDefault(SKIP_STAR_NODE_IN_DIMS, new ArrayList()))) ); paramMap.remove(SKIP_STAR_NODE_IN_DIMS); StarTreeFieldConfiguration.StarTreeBuildMode buildMode = StarTreeFieldConfiguration.StarTreeBuildMode.fromTypeName( diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 4be9105d8001c..902ca95643625 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -944,7 +944,8 @@ private synchronized IndexService createIndexService( () -> allowExpensiveQueries, indexNameExpressionResolver, recoveryStateFactories, - fileCache + fileCache, + compositeIndexSettings ); for (IndexingOperationListener operationListener : indexingOperationListeners) { indexModule.addIndexOperationListener(operationListener); @@ -974,8 +975,7 @@ private synchronized IndexService createIndexService( translogFactorySupplier, this::getClusterDefaultRefreshInterval, this.recoverySettings, - this.remoteStoreSettings, - this.compositeIndexSettings + this.remoteStoreSettings ); } @@ -1036,7 +1036,8 @@ public synchronized MapperService createIndexMapperService(IndexMetadata indexMe () -> allowExpensiveQueries, indexNameExpressionResolver, recoveryStateFactories, - fileCache + fileCache, + compositeIndexSettings ); pluginsService.onIndexModule(indexModule); return indexModule.newIndexMapperService(xContentRegistry, mapperRegistry, scriptService); diff --git a/server/src/test/java/org/opensearch/index/IndexModuleTests.java b/server/src/test/java/org/opensearch/index/IndexModuleTests.java index 3f5aacc5d2523..4ce4936c047d9 100644 --- a/server/src/test/java/org/opensearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/opensearch/index/IndexModuleTests.java @@ -80,7 +80,6 @@ import org.opensearch.index.cache.query.DisabledQueryCache; import org.opensearch.index.cache.query.IndexQueryCache; import org.opensearch.index.cache.query.QueryCache; -import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.engine.Engine; import org.opensearch.index.engine.EngineConfigFactory; import org.opensearch.index.engine.InternalEngineFactory; @@ -265,8 +264,7 @@ private IndexService newIndexService(IndexModule module) throws IOException { translogFactorySupplier, () -> IndexSettings.DEFAULT_REFRESH_INTERVAL, DefaultRecoverySettings.INSTANCE, - DefaultRemoteStoreSettings.INSTANCE, - CompositeIndexSettings.DefaultCompositeIndexSettings.INSTANCE + DefaultRemoteStoreSettings.INSTANCE ); } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java index 3691abea599ac..0097574a19b85 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeMappingIntegTests.java @@ -227,7 +227,7 @@ public void testValidCompositeIndex() { StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode() ); - assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); + assertEquals(Collections.emptySet(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); } } } @@ -311,7 +311,7 @@ public void testUpdateIndexWhenMappingIsSame() { StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode() ); - assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); + assertEquals(Collections.emptySet(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); } } } 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 6bb9925a7c4d8..e2fef4c360e19 100644 --- a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -61,7 +62,10 @@ public void testValidStarTree() throws IOException { assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.ON_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); - assertEquals(Arrays.asList("@timestamp", "status"), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); + assertEquals( + new HashSet<>(Arrays.asList("@timestamp", "status")), + starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims() + ); } } @@ -90,7 +94,7 @@ public void testValidStarTreeDefaults() throws IOException { assertEquals(expectedMetrics, starTreeFieldType.getMetrics().get(0).getMetrics()); assertEquals(10000, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); - assertEquals(Collections.emptyList(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); + assertEquals(Collections.emptySet(), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims()); } } diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 15170ba9388ac..9c58fc8fde084 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -185,7 +185,6 @@ import org.opensearch.index.SegmentReplicationPressureService; import org.opensearch.index.SegmentReplicationStatsTracker; import org.opensearch.index.analysis.AnalysisRegistry; -import org.opensearch.index.compositeindex.CompositeIndexSettings; import org.opensearch.index.remote.RemoteStorePressureService; import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory; import org.opensearch.index.seqno.GlobalCheckpointSyncAction; @@ -2079,8 +2078,7 @@ public void onFailure(final Exception e) { new RemoteStoreStatsTrackerFactory(clusterService, settings), DefaultRecoverySettings.INSTANCE, new CacheModule(new ArrayList<>(), settings).getCacheService(), - DefaultRemoteStoreSettings.INSTANCE, - CompositeIndexSettings.DefaultCompositeIndexSettings.INSTANCE + DefaultRemoteStoreSettings.INSTANCE ); final RecoverySettings recoverySettings = new RecoverySettings(settings, clusterSettings); snapshotShardsService = new SnapshotShardsService(