Skip to content

Commit bef982a

Browse files
bharath-techiebharath-techie
authored
[Star tree] Remove star tree feature flag and add index setting to configure star tree search on index basis (#18070)
* Removing star tree indexing and queries added till 2.19 out of experimental Signed-off-by: bharath-techie <[email protected]> * Removing star tree feature flag Signed-off-by: bharath-techie <[email protected]> * Adding more tests for feature flag Signed-off-by: bharath-techie <[email protected]> * Adding Changelog Signed-off-by: bharath-techie <[email protected]> * addressing review comment Signed-off-by: bharath-techie <[email protected]> * resolving conflicts Signed-off-by: bharath-techie <[email protected]> --------- Signed-off-by: bharath-techie <[email protected]>
1 parent 807163c commit bef982a

File tree

30 files changed

+133
-235
lines changed

30 files changed

+133
-235
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4545
- Add FIPS build tooling ([#4254](https://github.com/opensearch-project/security/issues/4254))
4646
- Support Nested Aggregations as part of Star-Tree ([#18048](https://github.com/opensearch-project/OpenSearch/pull/18048))
4747
- [Star-Tree] Support for date-range queries with star-tree supported aggregations ([#17855](https://github.com/opensearch-project/OpenSearch/pull/17855)
48+
- [Star tree] Remove star tree feature flag and add index setting to configure star tree search on index basis ([#18070](https://github.com/opensearch-project/OpenSearch/pull/18070))
4849
- Approximation Framework Enhancement: Update the BKD traversal logic to improve the performance on skewed data ([#18439](https://github.com/opensearch-project/OpenSearch/issues/18439))
4950

5051
### Changed

‎distribution/src/config/opensearch.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,3 @@ ${path.logs}
121121
# Once there is no observed impact on performance, this feature flag can be removed.
122122
#
123123
#opensearch.experimental.optimization.datetime_formatter_caching.enabled: false
124-
#
125-
# Gates the functionality of star tree index, which improves the performance of search aggregations.
126-
#
127-
#opensearch.experimental.feature.composite_index.star_tree.enabled: true

‎modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import java.util.List;
5757

5858
import static java.util.Collections.singletonList;
59-
import static org.opensearch.common.util.FeatureFlags.STAR_TREE_INDEX;
6059
import static org.hamcrest.Matchers.containsString;
6160

6261
public class ScaledFloatFieldMapperTests extends MapperTestCase {
@@ -104,7 +103,6 @@ public void testExistsQueryDocValuesDisabled() throws IOException {
104103
assertParseMinimalWarnings();
105104
}
106105

107-
@LockFeatureFlag(STAR_TREE_INDEX)
108106
public void testScaledFloatWithStarTree() throws Exception {
109107

110108
double scalingFactorField1 = randomDouble() * 100;

‎server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import org.opensearch.cluster.metadata.IndexMetadata;
1616
import org.opensearch.common.Rounding;
1717
import org.opensearch.common.settings.Settings;
18-
import org.opensearch.common.util.FeatureFlags;
1918
import org.opensearch.core.common.unit.ByteSizeUnit;
2019
import org.opensearch.core.common.unit.ByteSizeValue;
2120
import org.opensearch.core.index.Index;
@@ -581,11 +580,6 @@ private static String getMetric(boolean hasDocValues, boolean isKeyword) {
581580
return "numeric_dv";
582581
}
583582

584-
@Override
585-
protected Settings featureFlagSettings() {
586-
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.STAR_TREE_INDEX, "true").build();
587-
}
588-
589583
@Before
590584
public final void setupNodeSettings() {
591585
Settings request = Settings.builder().put(CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING.getKey(), true).build();

‎server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ protected FeatureFlagSettings(
3535
FeatureFlags.DATETIME_FORMATTER_CACHING_SETTING,
3636
FeatureFlags.WRITABLE_WARM_INDEX_SETTING,
3737
FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING,
38-
FeatureFlags.STAR_TREE_INDEX_SETTING,
3938
FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING,
4039
FeatureFlags.TERM_VERSION_PRECOMMIT_ENABLE_SETTING,
4140
FeatureFlags.ARROW_STREAMS_SETTING,

‎server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
260260
StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING,
261261
StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING,
262262
StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING,
263+
StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING,
263264

264265
IndexSettings.INDEX_CONTEXT_CREATED_VERSION,
265266
IndexSettings.INDEX_CONTEXT_CURRENT_VERSION,

‎server/src/main/java/org/opensearch/common/util/FeatureFlags.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,6 @@ public class FeatureFlags {
109109
Property.NodeScope
110110
);
111111

112-
/**
113-
* Gates the functionality of star tree index, which improves the performance of search
114-
* aggregations.
115-
*/
116-
public static final String STAR_TREE_INDEX = FEATURE_FLAG_PREFIX + "composite_index.star_tree.enabled";
117-
public static final Setting<Boolean> STAR_TREE_INDEX_SETTING = Setting.boolSetting(STAR_TREE_INDEX, false, Property.NodeScope);
118-
119112
/**
120113
* Gates the functionality of application based configuration templates.
121114
*/
@@ -154,7 +147,6 @@ static class FeatureFlagsImpl {
154147
put(TELEMETRY_SETTING, TELEMETRY_SETTING.getDefault(Settings.EMPTY));
155148
put(DATETIME_FORMATTER_CACHING_SETTING, DATETIME_FORMATTER_CACHING_SETTING.getDefault(Settings.EMPTY));
156149
put(WRITABLE_WARM_INDEX_SETTING, WRITABLE_WARM_INDEX_SETTING.getDefault(Settings.EMPTY));
157-
put(STAR_TREE_INDEX_SETTING, STAR_TREE_INDEX_SETTING.getDefault(Settings.EMPTY));
158150
put(
159151
APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING,
160152
APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING.getDefault(Settings.EMPTY)

‎server/src/main/java/org/opensearch/index/IndexSettings.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,8 +930,16 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
930930
*/
931931
private volatile double docIdFuzzySetFalsePositiveProbability;
932932

933+
/**
934+
* Denotes whether this is a composite index i.e star-tree index etc
935+
*/
933936
private final boolean isCompositeIndex;
934937

938+
/**
939+
* Denotes whether search via star tree index is enabled for this index
940+
*/
941+
private volatile boolean isStarTreeIndexEnabled;
942+
935943
/**
936944
* Returns the default search fields for this index.
937945
*/
@@ -1096,6 +1104,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
10961104
setEnableFuzzySetForDocId(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_ENABLED_SETTING));
10971105
setDocIdFuzzySetFalsePositiveProbability(scopedSettings.get(INDEX_DOC_ID_FUZZY_SET_FALSE_POSITIVE_PROBABILITY_SETTING));
10981106
isCompositeIndex = scopedSettings.get(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING);
1107+
isStarTreeIndexEnabled = scopedSettings.get(StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING);
10991108
scopedSettings.addSettingsUpdateConsumer(
11001109
TieredMergePolicyProvider.INDEX_COMPOUND_FORMAT_SETTING,
11011110
tieredMergePolicyProvider::setNoCFSRatio
@@ -1220,6 +1229,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
12201229
IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING,
12211230
this::setRemoteStoreTranslogRepository
12221231
);
1232+
scopedSettings.addSettingsUpdateConsumer(StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING, this::setStarTreeIndexEnabled);
12231233
}
12241234

12251235
private void setSearchIdleAfter(TimeValue searchIdleAfter) {
@@ -1790,6 +1800,14 @@ public long getGcDeletesInMillis() {
17901800
return gcDeletesInMillis;
17911801
}
17921802

1803+
public void setStarTreeIndexEnabled(boolean value) {
1804+
this.isStarTreeIndexEnabled = value;
1805+
}
1806+
1807+
public boolean getStarTreeIndexEnabled() {
1808+
return isStarTreeIndexEnabled;
1809+
}
1810+
17931811
/**
17941812
* Returns the merge policy that should be used for this index.
17951813
*

‎server/src/main/java/org/opensearch/index/compositeindex/CompositeIndexSettings.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.opensearch.common.settings.ClusterSettings;
1313
import org.opensearch.common.settings.Setting;
1414
import org.opensearch.common.settings.Settings;
15-
import org.opensearch.common.util.FeatureFlags;
1615
import org.opensearch.core.common.unit.ByteSizeUnit;
1716
import org.opensearch.core.common.unit.ByteSizeValue;
1817

@@ -25,16 +24,7 @@
2524
public class CompositeIndexSettings {
2625
public static final Setting<Boolean> STAR_TREE_INDEX_ENABLED_SETTING = Setting.boolSetting(
2726
"indices.composite_index.star_tree.enabled",
28-
false,
29-
value -> {
30-
if (FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING) == false && value == true) {
31-
throw new IllegalArgumentException(
32-
"star tree index is under an experimental feature and can be activated only by enabling "
33-
+ FeatureFlags.STAR_TREE_INDEX_SETTING.getKey()
34-
+ " feature flag in the JVM options"
35-
);
36-
}
37-
},
27+
true,
3828
Setting.Property.NodeScope,
3929
Setting.Property.Dynamic
4030
);

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeIndexSettings.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,11 @@ public static DateTimeUnitRounding getTimeUnit(String expression) {
134134
Setting.Property.IndexScope,
135135
Setting.Property.Final
136136
);
137+
138+
public static final Setting<Boolean> STAR_TREE_SEARCH_ENABLED_SETTING = Setting.boolSetting(
139+
"index.search.star_tree_index.enabled",
140+
true,
141+
Setting.Property.IndexScope,
142+
Setting.Property.Dynamic
143+
);
137144
}

‎server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeValidator.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,18 @@ public static void validate(MapperService mapperService, CompositeIndexSettings
4444
if (!(compositeFieldType != null && compositeFieldType.unwrap() instanceof StarTreeMapper.StarTreeFieldType)) {
4545
continue;
4646
}
47-
if (!compositeIndexSettings.isStarTreeIndexCreationEnabled()) {
47+
48+
if (indexSettings.getSettings()
49+
.getAsBoolean(
50+
StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey(),
51+
compositeIndexSettings.isStarTreeIndexCreationEnabled()
52+
) == false) {
4853
throw new IllegalArgumentException(
4954
String.format(
5055
Locale.ROOT,
51-
"star tree index cannot be created, enable it using [%s] setting",
52-
CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING.getKey()
56+
"star tree index cannot be created, enable it using [%s] cluster setting or [%s] index setting",
57+
CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING.getKey(),
58+
StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey()
5359
)
5460
);
5561
}

‎server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.opensearch.common.collect.CopyOnWriteHashMap;
4444
import org.opensearch.common.logging.DeprecationLogger;
4545
import org.opensearch.common.settings.Settings;
46-
import org.opensearch.common.util.FeatureFlags;
4746
import org.opensearch.common.xcontent.support.XContentMapValues;
4847
import org.opensearch.core.xcontent.ToXContent;
4948
import org.opensearch.core.xcontent.XContentBuilder;
@@ -447,13 +446,6 @@ protected static void parseCompositeField(
447446
Map<String, Object> compositeNode,
448447
ParserContext parserContext
449448
) {
450-
if (!FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING)) {
451-
throw new IllegalArgumentException(
452-
"star tree index is under an experimental feature and can be activated only by enabling "
453-
+ FeatureFlags.STAR_TREE_INDEX_SETTING.getKey()
454-
+ " feature flag in the JVM options"
455-
);
456-
}
457449
if (StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.get(parserContext.getSettings()) == false) {
458450
throw new IllegalArgumentException(
459451
String.format(

‎server/src/main/java/org/opensearch/search/DefaultSearchContext.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@
5858
import org.opensearch.index.IndexService;
5959
import org.opensearch.index.IndexSettings;
6060
import org.opensearch.index.cache.bitset.BitsetFilterCache;
61+
import org.opensearch.index.compositeindex.CompositeIndexSettings;
62+
import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
6163
import org.opensearch.index.engine.Engine;
6264
import org.opensearch.index.mapper.MappedFieldType;
6365
import org.opensearch.index.mapper.MapperService;
@@ -1148,6 +1150,16 @@ public boolean shouldUseTimeSeriesDescSortOptimization() {
11481150
&& sort.sort.getSort()[0].getReverse() == false;
11491151
}
11501152

1153+
@Override
1154+
public boolean getStarTreeIndexEnabled() {
1155+
return indexService.getIndexSettings()
1156+
.getSettings()
1157+
.getAsBoolean(
1158+
StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey(),
1159+
clusterService.getClusterSettings().get(CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING)
1160+
);
1161+
}
1162+
11511163
@Override
11521164
public int maxAggRewriteFilters() {
11531165
return maxAggRewriteFilters;

‎server/src/main/java/org/opensearch/search/SearchService.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,9 +1557,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
15571557
context.setProfilers(new Profilers(context.searcher(), context.shouldUseConcurrentSearch()));
15581558
}
15591559

1560-
if (this.indicesService.getCompositeIndexSettings() != null
1561-
&& this.indicesService.getCompositeIndexSettings().isStarTreeIndexCreationEnabled()
1562-
&& StarTreeQueryHelper.isStarTreeSupported(context)) {
1560+
if (context.getStarTreeIndexEnabled() && StarTreeQueryHelper.isStarTreeSupported(context)) {
15631561
StarTreeQueryContext starTreeQueryContext = new StarTreeQueryContext(context, source.query());
15641562
boolean consolidated = starTreeQueryContext.consolidateAllFilters(context);
15651563
if (consolidated) {

‎server/src/main/java/org/opensearch/search/internal/FilteredSearchContext.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,4 +582,9 @@ public int getTargetMaxSliceCount() {
582582
public boolean shouldUseTimeSeriesDescSortOptimization() {
583583
return in.shouldUseTimeSeriesDescSortOptimization();
584584
}
585+
586+
@Override
587+
public boolean getStarTreeIndexEnabled() {
588+
return in.getStarTreeIndexEnabled();
589+
}
585590
}

‎server/src/main/java/org/opensearch/search/internal/SearchContext.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,8 @@ public String toString() {
519519

520520
public abstract boolean shouldUseTimeSeriesDescSortOptimization();
521521

522+
public abstract boolean getStarTreeIndexEnabled();
523+
522524
public int maxAggRewriteFilters() {
523525
return 0;
524526
}

‎server/src/main/java/org/opensearch/search/startree/StarTreeQueryContext.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.apache.lucene.util.FixedBitSet;
1212
import org.opensearch.common.annotation.ExperimentalApi;
13-
import org.opensearch.common.util.FeatureFlags;
1413
import org.opensearch.index.codec.composite.CompositeIndexFieldInfo;
1514
import org.opensearch.index.compositeindex.datacube.DateDimension;
1615
import org.opensearch.index.compositeindex.datacube.Dimension;
@@ -251,21 +250,20 @@ private static boolean validateNestedAggregationStructure(
251250
AggregatorFactory aggregatorFactory
252251
) {
253252
boolean isValid;
254-
boolean isFeatureFlagEnabled = FeatureFlags.isEnabled(FeatureFlags.STAR_TREE_INDEX_SETTING);
255253

256254
switch (aggregatorFactory) {
257255
case TermsAggregatorFactory termsAggregatorFactory -> isValid = validateKeywordTermsAggregationSupport(
258256
compositeIndexFieldInfo,
259257
termsAggregatorFactory
260-
) && isFeatureFlagEnabled;
258+
);
261259
case DateHistogramAggregatorFactory dateHistogramAggregatorFactory -> isValid = validateDateHistogramSupport(
262260
compositeIndexFieldInfo,
263261
dateHistogramAggregatorFactory
264262
);
265263
case RangeAggregatorFactory rangeAggregatorFactory -> isValid = validateRangeAggregationSupport(
266264
compositeIndexFieldInfo,
267265
rangeAggregatorFactory
268-
) && isFeatureFlagEnabled;
266+
);
269267
case MetricAggregatorFactory metricAggregatorFactory -> {
270268
isValid = validateStarTreeMetricSupport(compositeIndexFieldInfo, metricAggregatorFactory);
271269
return isValid && metricAggregatorFactory.getSubFactories().getFactories().length == 0;

‎server/src/test/java/org/opensearch/index/codec/composite912/datacube/startree/AbstractStarTreeDVFormatTests.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.opensearch.cluster.metadata.IndexMetadata;
2222
import org.opensearch.common.CheckedConsumer;
2323
import org.opensearch.common.settings.Settings;
24-
import org.opensearch.common.util.FeatureFlags;
2524
import org.opensearch.common.xcontent.XContentFactory;
2625
import org.opensearch.core.common.unit.ByteSizeUnit;
2726
import org.opensearch.core.common.unit.ByteSizeValue;
@@ -35,23 +34,18 @@
3534
import org.opensearch.index.mapper.MapperService;
3635
import org.opensearch.indices.IndicesModule;
3736
import org.junit.After;
38-
import org.junit.AfterClass;
39-
import org.junit.BeforeClass;
4037

4138
import java.io.IOException;
4239
import java.util.ArrayList;
4340
import java.util.Collection;
4441
import java.util.Collections;
4542
import java.util.List;
4643

47-
import static org.opensearch.common.util.FeatureFlags.STAR_TREE_INDEX;
48-
4944
/**
5045
* Abstract star tree doc values Lucene tests
5146
*/
5247
@LuceneTestCase.SuppressSysoutChecks(bugUrl = "we log a lot on purpose")
5348
public abstract class AbstractStarTreeDVFormatTests extends BaseDocValuesFormatTestCase {
54-
private static FeatureFlags.TestUtils.FlagWriteLock ffLock = null;
5549
MapperService mapperService = null;
5650
StarTreeFieldConfiguration.StarTreeBuildMode buildMode;
5751

@@ -67,16 +61,6 @@ public static Collection<Object[]> parameters() {
6761
return parameters;
6862
}
6963

70-
@BeforeClass
71-
public static void createMapper() {
72-
ffLock = new FeatureFlags.TestUtils.FlagWriteLock(STAR_TREE_INDEX);
73-
}
74-
75-
@AfterClass
76-
public static void clearMapper() {
77-
ffLock.close();
78-
}
79-
8064
@After
8165
public void teardown() throws IOException {
8266
mapperService.close();

0 commit comments

Comments
 (0)