Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Star tree mapping changes #14261

Closed
1 change: 1 addition & 0 deletions distribution/src/config/jvm.options
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,4 @@ ${error.file}

# HDFS ForkJoinPool.common() support by SecurityManager
-Djava.util.concurrent.ForkJoinPool.common.threadFactory=org.opensearch.secure_sm.SecuredForkJoinWorkerThreadFactory
-Dopensearch.experimental.feature.composite_index.star_tree.enabled=true
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 @@ -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;
Expand Down Expand Up @@ -1318,6 +1319,10 @@
}
}

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

Check warning on line 1323 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#L1323

Added line #L1323 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 @@ -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;
Expand Down Expand Up @@ -754,7 +755,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 @@
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 @@
Supplier<TimeValue> clusterDefaultRefreshIntervalSupplier,
RecoverySettings recoverySettings,
RemoteStoreSettings remoteStoreSettings,
FileCache fileCache
FileCache fileCache,
CompositeIndexSettings compositeIndexSettings
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
) {
super(indexSettings);
this.allowExpensiveQueries = allowExpensiveQueries;
Expand Down Expand Up @@ -306,6 +309,7 @@
this.translogFactorySupplier = translogFactorySupplier;
this.recoverySettings = recoverySettings;
this.remoteStoreSettings = remoteStoreSettings;
this.compositeIndexSettings = compositeIndexSettings;
this.fileCache = fileCache;
updateFsyncTaskIfNecessary();
}
Expand Down Expand Up @@ -381,6 +385,7 @@
clusterDefaultRefreshIntervalSupplier,
recoverySettings,
remoteStoreSettings,
null,
null
);
}
Expand Down Expand Up @@ -1110,6 +1115,10 @@
}
}

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",
dblock marked this conversation as resolved.
Show resolved Hide resolved
false,
dblock marked this conversation as resolved.
Show resolved Hide resolved
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
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
);

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;
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
}

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;

Check warning on line 53 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#L53

Added line #L53 was not covered by tests
}
}
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);
}

Check warning on line 28 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#L27-L28

Added lines #L27 - L28 were not covered by tests

public static void validate(
MapperService mapperService,
CompositeIndexSettings compositeIndexSettings,
IndexSettings indexSettings,
boolean isCompositeFieldPresent
) {
if (!isCompositeFieldPresent && mapperService.isCompositeIndexPresent()) {
bharath-techie marked this conversation as resolved.
Show resolved Hide resolved
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
Loading