[Star tree] Remove star tree feature flag and add index setting to configure star tree search on index basis

CHANGELOG.md

@@ -44,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Support consumer reset in Resume API for pull-based ingestion. This PR includes a breaking change for the experimental pull-based ingestion feature. ([#18332](https://github.com/opensearch-project/OpenSearch/pull/18332))
 - Add FIPS build tooling ([#4254](https://github.com/opensearch-project/security/issues/4254))
 - [Star-Tree] Support for date-range queries with star-tree supported aggregations ([#17855](https://github.com/opensearch-project/OpenSearch/pull/17855)
+- [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))
 
 ### Changed
 - Create generic DocRequest to better categorize ActionRequests ([#18269](https://github.com/opensearch-project/OpenSearch/pull/18269)))

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

@@ -56,7 +56,6 @@
 import java.util.List;
 
 import static java.util.Collections.singletonList;
-import static org.opensearch.common.util.FeatureFlags.STAR_TREE_INDEX;
 import static org.hamcrest.Matchers.containsString;
 
 public class ScaledFloatFieldMapperTests extends MapperTestCase {
@@ -104,7 +103,6 @@ public void testExistsQueryDocValuesDisabled() throws IOException {
         assertParseMinimalWarnings();
     }
 
-    @LockFeatureFlag(STAR_TREE_INDEX)
     public void testScaledFloatWithStarTree() throws Exception {
 
         double scalingFactorField1 = randomDouble() * 100;

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

@@ -15,7 +15,6 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.Rounding;
 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;
 import org.opensearch.core.index.Index;
@@ -581,11 +580,6 @@ private static String getMetric(boolean hasDocValues, boolean isKeyword) {
         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();

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

@@ -35,7 +35,6 @@ protected FeatureFlagSettings(
         FeatureFlags.DATETIME_FORMATTER_CACHING_SETTING,
         FeatureFlags.WRITABLE_WARM_INDEX_SETTING,
         FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING,
-        FeatureFlags.STAR_TREE_INDEX_SETTING,
         FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING,
         FeatureFlags.TERM_VERSION_PRECOMMIT_ENABLE_SETTING,
         FeatureFlags.ARROW_STREAMS_SETTING,

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

@@ -260,6 +260,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
                 StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING,
                 StarTreeIndexSettings.STAR_TREE_MAX_BASE_METRICS_SETTING,
                 StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING,
+                StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING,
 
                 IndexSettings.INDEX_CONTEXT_CREATED_VERSION,
                 IndexSettings.INDEX_CONTEXT_CURRENT_VERSION,

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

@@ -109,13 +109,6 @@ 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 = FEATURE_FLAG_PREFIX + "composite_index.star_tree.enabled";
-    public static final Setting<Boolean> STAR_TREE_INDEX_SETTING = Setting.boolSetting(STAR_TREE_INDEX, false, Property.NodeScope);
-
     /**
      * Gates the functionality of application based configuration templates.
      */
@@ -154,7 +147,6 @@ static class FeatureFlagsImpl {
                 put(TELEMETRY_SETTING, TELEMETRY_SETTING.getDefault(Settings.EMPTY));
                 put(DATETIME_FORMATTER_CACHING_SETTING, DATETIME_FORMATTER_CACHING_SETTING.getDefault(Settings.EMPTY));
                 put(WRITABLE_WARM_INDEX_SETTING, WRITABLE_WARM_INDEX_SETTING.getDefault(Settings.EMPTY));
-                put(STAR_TREE_INDEX_SETTING, STAR_TREE_INDEX_SETTING.getDefault(Settings.EMPTY));
                 put(
                     APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING,
                     APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING.getDefault(Settings.EMPTY)

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

@@ -930,8 +930,16 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
      */
     private volatile double docIdFuzzySetFalsePositiveProbability;
 
+    /**
+     * Denotes whether this is a composite index i.e star-tree index etc
+     */
     private final boolean isCompositeIndex;
 
+    /**
+     * Denotes whether search via star tree index is enabled for this index
+     */
+    private volatile boolean isStarTreeIndexEnabled;
+
     /**
      * Returns the default search fields for this index.
      */
@@ -1096,6 +1104,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);
+        isStarTreeIndexEnabled = scopedSettings.get(StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING);
         scopedSettings.addSettingsUpdateConsumer(
             TieredMergePolicyProvider.INDEX_COMPOUND_FORMAT_SETTING,
             tieredMergePolicyProvider::setNoCFSRatio
@@ -1220,6 +1229,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
             IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING,
             this::setRemoteStoreTranslogRepository
         );
+        scopedSettings.addSettingsUpdateConsumer(StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING, this::setStarTreeIndexEnabled);
     }
 
     private void setSearchIdleAfter(TimeValue searchIdleAfter) {
@@ -1790,6 +1800,14 @@ public long getGcDeletesInMillis() {
         return gcDeletesInMillis;
     }
 
+    public void setStarTreeIndexEnabled(boolean value) {
+        this.isStarTreeIndexEnabled = value;
+    }
+
+    public boolean getStarTreeIndexEnabled() {
+        return isStarTreeIndexEnabled;
+    }
+
     /**
      * Returns the merge policy that should be used for this index.
      *

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

@@ -12,7 +12,6 @@
 import org.opensearch.common.settings.ClusterSettings;
 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;
 
@@ -25,16 +24,7 @@
 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(
-                    "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"
-                );
-            }
-        },
+        true,
         Setting.Property.NodeScope,
         Setting.Property.Dynamic
     );

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

@@ -134,4 +134,11 @@ public static DateTimeUnitRounding getTimeUnit(String expression) {
         Setting.Property.IndexScope,
         Setting.Property.Final
     );
+
+    public static final Setting<Boolean> STAR_TREE_SEARCH_ENABLED_SETTING = Setting.boolSetting(
+        "index.search.star_tree_index.enabled",
+        true,
+        Setting.Property.IndexScope,
+        Setting.Property.Dynamic
+    );
 }

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

@@ -44,12 +44,18 @@ public static void validate(MapperService mapperService, CompositeIndexSettings
             if (!(compositeFieldType != null && compositeFieldType.unwrap() instanceof StarTreeMapper.StarTreeFieldType)) {
                 continue;
             }
-            if (!compositeIndexSettings.isStarTreeIndexCreationEnabled()) {
+
+            if (indexSettings.getSettings()
+                .getAsBoolean(
+                    StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey(),
+                    compositeIndexSettings.isStarTreeIndexCreationEnabled()
+                ) == false) {
                 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()
+                        "star tree index cannot be created, enable it using [%s] cluster setting or [%s] index setting",
+                        CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING.getKey(),
+                        StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey()
                     )
                 );
             }

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

@@ -43,7 +43,6 @@
 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;
@@ -447,13 +446,6 @@ protected static void parseCompositeField(
             Map<String, Object> 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"
-                );
-            }
             if (StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.get(parserContext.getSettings()) == false) {
                 throw new IllegalArgumentException(
                     String.format(

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

@@ -58,6 +58,8 @@
 import org.opensearch.index.IndexService;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.cache.bitset.BitsetFilterCache;
+import org.opensearch.index.compositeindex.CompositeIndexSettings;
+import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings;
 import org.opensearch.index.engine.Engine;
 import org.opensearch.index.mapper.MappedFieldType;
 import org.opensearch.index.mapper.MapperService;
@@ -1148,6 +1150,16 @@ public boolean shouldUseTimeSeriesDescSortOptimization() {
             && sort.sort.getSort()[0].getReverse() == false;
     }
 
+    @Override
+    public boolean getStarTreeIndexEnabled() {
+        return indexService.getIndexSettings()
+            .getSettings()
+            .getAsBoolean(
+                StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey(),
+                clusterService.getClusterSettings().get(CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING)
+            );
+    }
+
     @Override
     public int maxAggRewriteFilters() {
         return maxAggRewriteFilters;

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

@@ -1557,9 +1557,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
             context.setProfilers(new Profilers(context.searcher(), context.shouldUseConcurrentSearch()));
         }
 
-        if (this.indicesService.getCompositeIndexSettings() != null
-            && this.indicesService.getCompositeIndexSettings().isStarTreeIndexCreationEnabled()
-            && StarTreeQueryHelper.isStarTreeSupported(context)) {
+        if (context.getStarTreeIndexEnabled() && StarTreeQueryHelper.isStarTreeSupported(context)) {
             StarTreeQueryContext starTreeQueryContext = new StarTreeQueryContext(context, source.query());
             boolean consolidated = starTreeQueryContext.consolidateAllFilters(context);
             if (consolidated) {

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

@@ -582,4 +582,9 @@ public int getTargetMaxSliceCount() {
     public boolean shouldUseTimeSeriesDescSortOptimization() {
         return in.shouldUseTimeSeriesDescSortOptimization();
     }
+
+    @Override
+    public boolean getStarTreeIndexEnabled() {
+        return in.getStarTreeIndexEnabled();
+    }
 }

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

@@ -519,6 +519,8 @@ public String toString() {
 
     public abstract boolean shouldUseTimeSeriesDescSortOptimization();
 
+    public abstract boolean getStarTreeIndexEnabled();
+
     public int maxAggRewriteFilters() {
         return 0;
     }

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

@@ -21,7 +21,6 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.CheckedConsumer;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.core.common.unit.ByteSizeUnit;
 import org.opensearch.core.common.unit.ByteSizeValue;
@@ -35,23 +34,18 @@
 import org.opensearch.index.mapper.MapperService;
 import org.opensearch.indices.IndicesModule;
 import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
 
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
-import static org.opensearch.common.util.FeatureFlags.STAR_TREE_INDEX;
-
 /**
  * Abstract star tree doc values Lucene tests
  */
 @LuceneTestCase.SuppressSysoutChecks(bugUrl = "we log a lot on purpose")
 public abstract class AbstractStarTreeDVFormatTests extends BaseDocValuesFormatTestCase {
-    private static FeatureFlags.TestUtils.FlagWriteLock ffLock = null;
     MapperService mapperService = null;
     StarTreeFieldConfiguration.StarTreeBuildMode buildMode;
 
@@ -67,16 +61,6 @@ public static Collection<Object[]> parameters() {
         return parameters;
     }
 
-    @BeforeClass
-    public static void createMapper() {
-        ffLock = new FeatureFlags.TestUtils.FlagWriteLock(STAR_TREE_INDEX);
-    }
-
-    @AfterClass
-    public static void clearMapper() {
-        ffLock.close();
-    }
-
     @After
     public void teardown() throws IOException {
         mapperService.close();

server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java

@@ -35,7 +35,6 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.compress.CompressedXContent;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.unit.ByteSizeUnit;
@@ -56,7 +55,6 @@
 
 import org.mockito.Mockito;
 
-import static org.opensearch.common.util.FeatureFlags.STAR_TREE_INDEX;
 import static org.opensearch.index.mapper.ObjectMapper.Nested.isParent;
 import static org.hamcrest.Matchers.containsString;
 
@@ -543,33 +541,23 @@ public void testCompositeFields() throws Exception {
 
         Settings settings = Settings.builder()
             .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true)
+            .put(StarTreeIndexSettings.STAR_TREE_SEARCH_ENABLED_SETTING.getKey(), false)
             .put(IndexMetadata.INDEX_APPEND_ONLY_ENABLED_SETTING.getKey(), true)
             .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB))
             .build();
 
-        IllegalArgumentException ex = expectThrows(
-            IllegalArgumentException.class,
-            () -> createIndex("invalid", settings).mapperService().documentMapperParser().parse("tweet", new CompressedXContent(mapping))
-        );
-        assertEquals(
-            "star tree index is under an experimental feature and can be activated only by enabling opensearch.experimental.feature.composite_index.star_tree.enabled feature flag in the JVM options",
-            ex.getMessage()
-        );
+        DocumentMapper documentMapper = createIndex("test", settings).mapperService()
+            .documentMapperParser()
+            .parse("tweet", new CompressedXContent(mapping));
 
-        FeatureFlags.TestUtils.with(STAR_TREE_INDEX, () -> {
-            DocumentMapper documentMapper = createIndex("test", settings).mapperService()
-                .documentMapperParser()
-                .parse("tweet", new CompressedXContent(mapping));
-
-            Mapper mapper = documentMapper.root().getMapper("startree");
-            assertTrue(mapper instanceof StarTreeMapper);
-            StarTreeMapper starTreeMapper = (StarTreeMapper) mapper;
-            assertEquals("star_tree", starTreeMapper.fieldType().typeName());
-            // Check that field in properties was parsed correctly as well
-            mapper = documentMapper.root().getMapper("@timestamp");
-            assertNotNull(mapper);
-            assertEquals("date", mapper.typeName());
-        });
+        Mapper mapper = documentMapper.root().getMapper("startree");
+        assertTrue(mapper instanceof StarTreeMapper);
+        StarTreeMapper starTreeMapper = (StarTreeMapper) mapper;
+        assertEquals("star_tree", starTreeMapper.fieldType().typeName());
+        // Check that field in properties was parsed correctly as well
+        mapper = documentMapper.root().getMapper("@timestamp");
+        assertNotNull(mapper);
+        assertEquals("date", mapper.typeName());
     }
 
     public void testNestedIsParent() throws Exception {