Skip to content

Commit

Permalink
Star tree mapping changes (#14605)
Browse files Browse the repository at this point in the history
* Star tree mapping changes with feature flag
---------
Signed-off-by: Bharathwaj G <[email protected]>
(cherry picked from commit 6267e94)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] committed Jul 5, 2024
1 parent 3a1be63 commit 9ad161c
Show file tree
Hide file tree
Showing 39 changed files with 2,951 additions and 15 deletions.
4 changes: 4 additions & 0 deletions distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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_index.star_tree.enabled: true

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,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;
Expand Down Expand Up @@ -1340,6 +1341,10 @@ private static void updateIndexMappingsAndBuildSortOrder(
}
}

if (mapperService.isCompositeIndexPresent()) {
CompositeIndexValidator.validate(mapperService, indexService.getCompositeIndexSettings(), indexService.getIndexSettings());

Check warning on line 1345 in server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java#L1345

Added line #L1345 was not covered by tests
}

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,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;
Expand Down Expand Up @@ -756,7 +757,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
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.opensearch.index.SearchSlowLog;
import org.opensearch.index.TieredMergePolicyProvider;
import org.opensearch.index.cache.bitset.BitsetFilterCache;
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;
Expand Down Expand Up @@ -239,6 +240,15 @@ 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,
StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING,

// validate that built-in similarities don't get redefined
Setting.groupSetting("index.similarity.", (s) -> {
Map<String, Settings> groups = s.getAsGroups();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_index.star_tree.enabled";
public static final Setting<Boolean> STAR_TREE_INDEX_SETTING = Setting.boolSetting(STAR_TREE_INDEX, false, Property.NodeScope);

private static final List<Setting<Boolean>> ALL_FEATURE_FLAG_SETTINGS = List.of(
REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING,
EXTENSIONS_SETTING,
Expand All @@ -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.
Expand Down
10 changes: 8 additions & 2 deletions server/src/main/java/org/opensearch/index/IndexModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -311,6 +312,7 @@ public Iterator<Setting<?>> settings() {
private final BooleanSupplier allowExpensiveQueries;
private final Map<String, IndexStorePlugin.RecoveryStateFactory> 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
Expand All @@ -330,7 +332,8 @@ public IndexModule(
final BooleanSupplier allowExpensiveQueries,
final IndexNameExpressionResolver expressionResolver,
final Map<String, IndexStorePlugin.RecoveryStateFactory> recoveryStateFactories,
final FileCache fileCache
final FileCache fileCache,
final CompositeIndexSettings compositeIndexSettings
) {
this.indexSettings = indexSettings;
this.analysisRegistry = analysisRegistry;
Expand All @@ -343,6 +346,7 @@ public IndexModule(
this.expressionResolver = expressionResolver;
this.recoveryStateFactories = recoveryStateFactories;
this.fileCache = fileCache;
this.compositeIndexSettings = compositeIndexSettings;
}

public IndexModule(
Expand All @@ -364,6 +368,7 @@ public IndexModule(
allowExpensiveQueries,
expressionResolver,
recoveryStateFactories,
null,
null
);
}
Expand Down Expand Up @@ -739,7 +744,8 @@ public IndexService newIndexService(
clusterDefaultRefreshIntervalSupplier,
recoverySettings,
remoteStoreSettings,
fileCache
fileCache,
compositeIndexSettings
);
success = true;
return indexService;
Expand Down
11 changes: 10 additions & 1 deletion server/src/main/java/org/opensearch/index/IndexService.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,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;
Expand Down Expand Up @@ -192,6 +193,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
private final RecoverySettings recoverySettings;
private final RemoteStoreSettings remoteStoreSettings;
private final FileCache fileCache;
private final CompositeIndexSettings compositeIndexSettings;

public IndexService(
IndexSettings indexSettings,
Expand Down Expand Up @@ -228,7 +230,8 @@ public IndexService(
Supplier<TimeValue> clusterDefaultRefreshIntervalSupplier,
RecoverySettings recoverySettings,
RemoteStoreSettings remoteStoreSettings,
FileCache fileCache
FileCache fileCache,
CompositeIndexSettings compositeIndexSettings
) {
super(indexSettings);
this.allowExpensiveQueries = allowExpensiveQueries;
Expand Down Expand Up @@ -306,6 +309,7 @@ public IndexService(
this.translogFactorySupplier = translogFactorySupplier;
this.recoverySettings = recoverySettings;
this.remoteStoreSettings = remoteStoreSettings;
this.compositeIndexSettings = compositeIndexSettings;
this.fileCache = fileCache;
updateFsyncTaskIfNecessary();
}
Expand Down Expand Up @@ -381,6 +385,7 @@ public IndexService(
clusterDefaultRefreshIntervalSupplier,
recoverySettings,
remoteStoreSettings,
null,
null
);
}
Expand Down Expand Up @@ -1110,6 +1115,10 @@ private void rescheduleRefreshTasks() {
}
}

public CompositeIndexSettings getCompositeIndexSettings() {
return compositeIndexSettings;

Check warning on line 1119 in server/src/main/java/org/opensearch/index/IndexService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/IndexService.java#L1119

Added line #L1119 was not covered by tests
}

/**
* Shard Store Deleter Interface
*
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Boolean> STAR_TREE_INDEX_ENABLED_SETTING = Setting.boolSetting(
"indices.composite_index.star_tree.enabled",
false,
value -> {
if (FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING) == false && value == true) {
throw new IllegalArgumentException(

Check warning on line 29 in server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java#L29

Added line #L29 was not covered by tests
"star tree index is under an experimental feature and can be activated only by enabling "
+ FeatureFlags.STAR_TREE_INDEX_SETTING.getKey()

Check warning on line 31 in server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java#L31

Added line #L31 was not covered by tests
+ " 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;
}

Check warning on line 50 in server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java#L49-L50

Added lines #L49 - L50 were not covered by tests

public boolean isStarTreeIndexCreationEnabled() {
return starTreeIndexCreationEnabled;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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.IndexSettings;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeValidator;
import org.opensearch.index.mapper.MapperService;

import java.util.Locale;

/**
* Validation for composite indices as part of mappings
*
* @opensearch.experimental
*/
@ExperimentalApi
public class CompositeIndexValidator {

Check warning on line 24 in server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java#L24

Added line #L24 was not covered by tests

public static void validate(MapperService mapperService, CompositeIndexSettings compositeIndexSettings, IndexSettings indexSettings) {
StarTreeValidator.validate(mapperService, compositeIndexSettings, indexSettings);
}

public static void validate(
MapperService mapperService,
CompositeIndexSettings compositeIndexSettings,
IndexSettings indexSettings,
boolean isCompositeFieldPresent
) {
if (!isCompositeFieldPresent && mapperService.isCompositeIndexPresent()) {
throw new IllegalArgumentException(
String.format(

Check warning on line 38 in server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexValidator.java#L37-L38

Added lines #L37 - L38 were not covered by tests
Locale.ROOT,
"Composite fields must be specified during index creation, addition of new composite fields during update is not supported"
)
);
}
StarTreeValidator.validate(mapperService, compositeIndexSettings, indexSettings);
}
}
Loading

0 comments on commit 9ad161c

Please sign in to comment.