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

[Backport 2.x] Star tree index validations (#15533) #15610

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,17 @@
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.core.index.Index;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.env.Environment;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexNotFoundException;
import org.opensearch.index.IndexService;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.compositeindex.CompositeIndexSettings;
import org.opensearch.index.compositeindex.CompositeIndexValidator;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
import org.opensearch.index.mapper.DocumentMapper;
import org.opensearch.index.mapper.MapperService;
import org.opensearch.index.mapper.MapperService.MergeReason;
Expand Down Expand Up @@ -154,6 +157,7 @@
import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING;
import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findContextTemplateName;
import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
import static org.opensearch.index.IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent;
import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
Expand Down Expand Up @@ -1079,6 +1083,7 @@ static Settings aggregateIndexSettings(
validateTranslogRetentionSettings(indexSettings);
validateStoreTypeSettings(indexSettings);
validateRefreshIntervalSettings(request.settings(), clusterSettings);
validateTranslogFlushIntervalSettingsForCompositeIndex(request.settings(), clusterSettings);
validateTranslogDurabilitySettings(request.settings(), clusterSettings, settings);
return indexSettings;
}
Expand Down Expand Up @@ -1766,6 +1771,71 @@ public static void validateTranslogRetentionSettings(Settings indexSettings) {
}
}

/**
* Validates {@code index.translog.flush_threshold_size} is equal or below the {@code indices.composite_index.translog.max_flush_threshold_size}
* for composite indices based on {{@code index.composite_index}}
*
* @param requestSettings settings passed in during index create/update request
* @param clusterSettings cluster setting
*/
public static void validateTranslogFlushIntervalSettingsForCompositeIndex(Settings requestSettings, ClusterSettings clusterSettings) {
if (StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.exists(requestSettings) == false
|| requestSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey()) == null) {
return;
}
ByteSizeValue translogFlushSize = INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.get(requestSettings);
ByteSizeValue compositeIndexMaxFlushSize = clusterSettings.get(
CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING
);
if (translogFlushSize.compareTo(compositeIndexMaxFlushSize) > 0) {
throw new IllegalArgumentException(
String.format(
Locale.ROOT,
"You can configure '%s' with upto '%s' for composite index",
INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(),
compositeIndexMaxFlushSize
)
);
}
}

/**
* Validates {@code index.translog.flush_threshold_size} is equal or below the {@code indices.composite_index.translog.max_flush_threshold_size}
* for composite indices based on {{@code index.composite_index}}
* This is used during update index settings flow
*
* @param requestSettings settings passed in during index update request
* @param clusterSettings cluster setting
* @param indexSettings index settings
*/
public static Optional<String> validateTranslogFlushIntervalSettingsForCompositeIndex(
Settings requestSettings,
ClusterSettings clusterSettings,
Settings indexSettings
) {
if (INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.exists(requestSettings) == false
|| requestSettings.get(INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey()) == null
|| StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.exists(indexSettings) == false
|| indexSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey()) == null) {
return Optional.empty();
}
ByteSizeValue translogFlushSize = INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.get(requestSettings);
ByteSizeValue compositeIndexMaxFlushSize = clusterSettings.get(
CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING
);
if (translogFlushSize.compareTo(compositeIndexMaxFlushSize) > 0) {
return Optional.of(
String.format(
Locale.ROOT,
"You can configure '%s' with upto '%s' for composite index",
INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(),
compositeIndexMaxFlushSize
)
);
}
return Optional.empty();
}

/**
* Validates {@code index.refresh_interval} is equal or below the {@code cluster.minimum.index.refresh_interval}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@

import static org.opensearch.cluster.metadata.MetadataCreateDataStreamService.validateTimestampFieldMapping;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex;
import static org.opensearch.common.util.concurrent.ThreadContext.ACTION_ORIGIN_TRANSIENT_NAME;
import static org.opensearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED;

Expand Down Expand Up @@ -1639,6 +1640,7 @@ private void validate(String name, @Nullable Settings settings, List<String> ind

// validate index refresh interval and translog durability settings
validateRefreshIntervalSettings(settings, clusterService.getClusterSettings());
validateTranslogFlushIntervalSettingsForCompositeIndex(settings, clusterService.getClusterSettings());
validateTranslogDurabilitySettingsInTemplate(settings, clusterService.getClusterSettings());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateOverlap;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogDurabilitySettings;
import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogFlushIntervalSettingsForCompositeIndex;
import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findComponentTemplate;
import static org.opensearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX;
import static org.opensearch.index.IndexSettings.same;
Expand Down Expand Up @@ -221,6 +222,12 @@ public ClusterState execute(ClusterState currentState) {
index.getName()
).ifPresent(validationErrors::add);
}
validateTranslogFlushIntervalSettingsForCompositeIndex(
normalizedSettings,
clusterService.getClusterSettings(),
metadata.getSettings()
).ifPresent(validationErrors::add);

}

if (validationErrors.size() > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,10 @@ public void apply(Settings value, Settings current, Settings previous) {
RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_LOOKBACK_INTERVAL,
RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED,

// Composite index settings
CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING,
CompositeIndexSettings.COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING,

SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED,

// WorkloadManagement settings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
StarTreeIndexSettings.DEFAULT_METRICS_LIST,
StarTreeIndexSettings.DEFAULT_DATE_INTERVALS,
StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING,
StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING,
StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING,

IndexSettings.INDEX_CONTEXT_CREATED_VERSION,
IndexSettings.INDEX_CONTEXT_CURRENT_VERSION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.opensearch.core.common.unit.ByteSizeUnit;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.core.index.Index;
import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
import org.opensearch.index.remote.RemoteStorePathStrategy;
import org.opensearch.index.remote.RemoteStoreUtils;
import org.opensearch.index.translog.Translog;
Expand Down Expand Up @@ -880,6 +881,8 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
*/
private volatile double docIdFuzzySetFalsePositiveProbability;

private final boolean isCompositeIndex;

/**
* Returns the default search fields for this index.
*/
Expand Down Expand Up @@ -1042,7 +1045,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti

setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING));
setDocIdFuzzySetFalsePositiveProbability(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING));

isCompositeIndex = scopedSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING);
scopedSettings.addSettingsUpdateConsumer(
TieredMergePolicyProvider.INDEX_COMPOUND_FORMAT_SETTING,
tieredMergePolicyProvider::setNoCFSRatio
Expand Down Expand Up @@ -1287,6 +1290,10 @@ public int getNumberOfReplicas() {
return settings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, null);
}

public boolean isCompositeIndex() {
return isCompositeIndex;
}

/**
* Returns true if segment replication is enabled on the index.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.common.unit.ByteSizeUnit;
import org.opensearch.core.common.unit.ByteSizeValue;

/**
* Cluster level settings for composite indices
Expand All @@ -37,12 +39,23 @@ public class CompositeIndexSettings {
Setting.Property.Dynamic
);

/**
* This sets the max flush threshold size for composite index
*/
public static final Setting<ByteSizeValue> COMPOSITE_INDEX_MAX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING = Setting.byteSizeSetting(
"indices.composite_index.translog.max_flush_threshold_size",
new ByteSizeValue(512, ByteSizeUnit.MB),
new ByteSizeValue(128, ByteSizeUnit.MB),
new ByteSizeValue(Long.MAX_VALUE, ByteSizeUnit.BYTES),
Setting.Property.NodeScope,
Setting.Property.Dynamic
);

private volatile boolean starTreeIndexCreationEnabled;

public CompositeIndexSettings(Settings settings, ClusterSettings clusterSettings) {
this.starTreeIndexCreationEnabled = STAR_TREE_INDEX_ENABLED_SETTING.get(settings);
clusterSettings.addSettingsUpdateConsumer(STAR_TREE_INDEX_ENABLED_SETTING, this::starTreeIndexCreationEnabled);

}

private void starTreeIndexCreationEnabled(boolean value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
public class StarTreeIndexSettings {

public static int STAR_TREE_MAX_DIMENSIONS_DEFAULT = 10;
public static int STAR_TREE_MAX_BASE_METRICS_DEFAULT = 100;
/**
* This setting determines the max number of star tree fields that can be part of composite index mapping. For each
* star tree field, we will generate associated star tree index.
Expand All @@ -52,6 +53,19 @@ public class StarTreeIndexSettings {
Setting.Property.Final
);

/**
* This setting determines the max number of dimensions that can be part of star tree index field. Number of
* dimensions and associated cardinality has direct effect of star tree index size and query performance.
*/
public static final Setting<Integer> STAR_TREE_MAX_BASE_METRICS_SETTING = Setting.intSetting(
"index.composite_index.star_tree.field.max_base_metrics",
STAR_TREE_MAX_BASE_METRICS_DEFAULT,
4,
100,
Setting.Property.IndexScope,
Setting.Property.Final
);

/**
* This setting determines the max number of date intervals that can be part of star tree date field.
*/
Expand Down Expand Up @@ -108,4 +122,11 @@ public static Rounding.DateTimeUnit getTimeUnit(String expression) {
}
return DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expression);
}

public static final Setting<Boolean> IS_COMPOSITE_INDEX_SETTING = Setting.boolSetting(
"index.composite_index",
false,
Setting.Property.IndexScope,
Setting.Property.Final
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,17 @@ private static void parseNonDynamicArray(ParseContext context, ObjectMapper mapp
throws IOException {
XContentParser parser = context.parser();
XContentParser.Token token;
// block array values for composite index fields
if (context.indexSettings().isCompositeIndex() && context.mapperService().isFieldPartOfCompositeIndex(arrayFieldName)) {
throw new MapperParsingException(
String.format(
Locale.ROOT,
"object mapping for [%s] with array for [%s] cannot be accepted as field is also part of composite index mapping which does not accept arrays",
mapper.name(),
arrayFieldName
)
);
}
final String[] paths = splitAndValidatePath(lastFieldName);
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.START_OBJECT) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ public enum MergeReason {
private final BooleanSupplier idFieldDataEnabled;

private volatile Set<CompositeMappedFieldType> compositeMappedFieldTypes;
private volatile Set<String> fieldsPartOfCompositeMappings;

public MapperService(
IndexSettings indexSettings,
Expand Down Expand Up @@ -543,9 +544,18 @@ private synchronized Map<String, DocumentMapper> internalMerge(DocumentMapper ma

// initialize composite fields post merge
this.compositeMappedFieldTypes = getCompositeFieldTypesFromMapper();
buildCompositeFieldLookup();
return results;
}

private void buildCompositeFieldLookup() {
Set<String> fieldsPartOfCompositeMappings = new HashSet<>();
for (CompositeMappedFieldType fieldType : compositeMappedFieldTypes) {
fieldsPartOfCompositeMappings.addAll(fieldType.fields());
}
this.fieldsPartOfCompositeMappings = fieldsPartOfCompositeMappings;
}

private boolean assertSerialization(DocumentMapper mapper) {
// capture the source now, it may change due to concurrent parsing
final CompressedXContent mappingSource = mapper.mappingSource();
Expand Down Expand Up @@ -672,6 +682,10 @@ private Set<CompositeMappedFieldType> getCompositeFieldTypesFromMapper() {
return compositeMappedFieldTypes;
}

public boolean isFieldPartOfCompositeIndex(String field) {
return fieldsPartOfCompositeMappings.contains(field);
}

public ObjectMapper getObjectMapper(String name) {
return this.mapper == null ? null : this.mapper.objectMappers().get(name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,15 @@
+ " feature flag in the JVM options"
);
}
if (StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.get(parserContext.getSettings()) == false) {
throw new IllegalArgumentException(
String.format(

Check warning on line 445 in server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java#L444-L445

Added lines #L444 - L445 were not covered by tests
Locale.ROOT,
"Set '%s' as true as part of index settings to use star tree index",
StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey()

Check warning on line 448 in server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java#L448

Added line #L448 was not covered by tests
)
);
}
Iterator<Map.Entry<String, Object>> iterator = compositeNode.entrySet().iterator();
if (compositeNode.size() > StarTreeIndexSettings.STAR_TREE_MAX_FIELDS_SETTING.get(parserContext.getSettings())) {
throw new IllegalArgumentException(
Expand Down
Loading
Loading