Time series based workload desc order optimization through reverse segment read

CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add getter for path field in NestedQueryBuilder ([#4636](https://github.com/opensearch-project/OpenSearch/pull/4636))
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add events correlation engine plugin ([#6854](https://github.com/opensearch-project/OpenSearch/issues/6854))
+- Time series based workload desc order optimization through reverse segment read. ([#7244](https://github.com/opensearch-project/OpenSearch/pull/7244))
 
 ### Dependencies
 - Bump `log4j-core` from 2.18.0 to 2.19.0

sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/profile/query/QueryProfilerTests.java

@@ -96,7 +96,8 @@ public void setUp() throws Exception {
             IndexSearcher.getDefaultQueryCache(),
             ALWAYS_CACHE_POLICY,
             true,
-            executor
+            executor,
+            false
         );
     }
 

sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryPhaseTests.java

@@ -1152,7 +1152,8 @@ private static ContextIndexSearcher newContextSearcher(IndexReader reader, Execu
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            executor
+            executor,
+            false
         );
     }
 
@@ -1164,7 +1165,8 @@ private static ContextIndexSearcher newEarlyTerminationContextSearcher(IndexRead
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            executor
+            executor,
+            false
         ) {
 
             @Override

sandbox/plugins/concurrent-search/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java

@@ -1164,7 +1164,8 @@ private static ContextIndexSearcher newContextSearcher(IndexReader reader, Execu
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            executor
+            executor,
+            false
         );
     }
 
@@ -1176,7 +1177,8 @@ private static ContextIndexSearcher newEarlyTerminationContextSearcher(IndexRead
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            executor
+            executor,
+            false
         ) {
 
             @Override

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

@@ -153,6 +153,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
                 IndexSettings.MAX_SLICES_PER_SCROLL,
                 IndexSettings.MAX_SLICES_PER_PIT,
                 IndexSettings.MAX_REGEX_LENGTH_SETTING,
+                IndexSettings.SEARCH_SEGMENTS_REVERSE_ORDER_OPTIMIZATION_SETTING,
                 ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING,
                 IndexSettings.INDEX_GC_DELETES_SETTING,
                 IndexSettings.INDEX_SOFT_DELETES_SETTING,

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

@@ -578,6 +578,16 @@ public final class IndexSettings {
         Property.InternalIndex
     );
 
+    /**
+     * Setting to enable/disable desc leaf reader optimization
+     */
+    public static final Setting<Boolean> SEARCH_SEGMENTS_REVERSE_ORDER_OPTIMIZATION_SETTING = Setting.boolSetting(
+        "index.search_segments_reverse_order_optimization.enabled",
+        true,
+        Property.Dynamic,
+        Property.IndexScope
+    );
+
     private final Index index;
     private final Version version;
     private final Logger logger;
@@ -653,6 +663,7 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
     private volatile long mappingTotalFieldsLimit;
     private volatile long mappingDepthLimit;
     private volatile long mappingFieldNameLengthLimit;
+    private volatile boolean searchSegmentOrderReversed;
 
     /**
      * The maximum number of refresh listeners allows on this shard.
@@ -822,6 +833,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         maxFullFlushMergeWaitTime = scopedSettings.get(INDEX_MERGE_ON_FLUSH_MAX_FULL_FLUSH_MERGE_WAIT_TIME);
         mergeOnFlushEnabled = scopedSettings.get(INDEX_MERGE_ON_FLUSH_ENABLED);
         setMergeOnFlushPolicy(scopedSettings.get(INDEX_MERGE_ON_FLUSH_POLICY));
+        searchSegmentOrderReversed = scopedSettings.get(SEARCH_SEGMENTS_REVERSE_ORDER_OPTIMIZATION_SETTING);
 
         scopedSettings.addSettingsUpdateConsumer(MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING, mergePolicyConfig::setNoCFSRatio);
         scopedSettings.addSettingsUpdateConsumer(
@@ -895,6 +907,11 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         scopedSettings.addSettingsUpdateConsumer(INDEX_MERGE_ON_FLUSH_MAX_FULL_FLUSH_MERGE_WAIT_TIME, this::setMaxFullFlushMergeWaitTime);
         scopedSettings.addSettingsUpdateConsumer(INDEX_MERGE_ON_FLUSH_ENABLED, this::setMergeOnFlushEnabled);
         scopedSettings.addSettingsUpdateConsumer(INDEX_MERGE_ON_FLUSH_POLICY, this::setMergeOnFlushPolicy);
+        scopedSettings.addSettingsUpdateConsumer(SEARCH_SEGMENTS_REVERSE_ORDER_OPTIMIZATION_SETTING, this::setSearchSegmentOrderReversed);
+    }
+
+    private void setSearchSegmentOrderReversed(boolean reversed) {
+        this.searchSegmentOrderReversed = reversed;
     }
 
     private void setSearchIdleAfter(TimeValue searchIdleAfter) {
@@ -1068,6 +1085,13 @@ public Settings getNodeSettings() {
         return nodeSettings;
     }
 
+    /**
+     * Returns true if index level setting for leaf reverse order search optimization is enabled
+     */
+    public boolean getSearchSegmentOrderReversed() {
+        return this.searchSegmentOrderReversed;
+    }
+
     /**
      * Updates the settings and index metadata and notifies all registered settings consumers with the new settings iff at least one
      * setting has changed.

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

@@ -89,6 +89,7 @@
 import org.opensearch.search.rescore.RescoreContext;
 import org.opensearch.search.slice.SliceBuilder;
 import org.opensearch.search.sort.SortAndFormats;
+import org.opensearch.search.sort.SortOrder;
 import org.opensearch.search.suggest.SuggestionSearchContext;
 
 import java.io.IOException;
@@ -210,7 +211,8 @@ final class DefaultSearchContext extends SearchContext {
             engineSearcher.getQueryCache(),
             engineSearcher.getQueryCachingPolicy(),
             lowLevelCancellation,
-            executor
+            executor,
+            shouldReverseLeafReaderContexts()
         );
         this.relativeTimeSupplier = relativeTimeSupplier;
         this.timeout = timeout;
@@ -885,4 +887,24 @@ public boolean isCancelled() {
     public ReaderContext readerContext() {
         return readerContext;
     }
+
+    private boolean shouldReverseLeafReaderContexts() {
+        // By default, if search query is on desc order, we will search leaves in reverse order.
+        // This is beneficial for desc order sort queries on time series based workload,
+        // where recent data is always on new segments which are in last.
+        // This won't regress or impact other type of workload where data is randomly distributed
+        // across segments. So turning it on by default.
+        // searchSegmentOrderReversed is true by default
+        if (this.indexService.getIndexSettings() != null && this.indexService.getIndexSettings().getSearchSegmentOrderReversed()) {
+            // Only reverse order for desc order sort queries
+            if (request != null
+                && request.source() != null
+                && request.source().sorts() != null
+                && request.source().sorts().size() > 0
+                && request.source().sorts().get(0).order() == SortOrder.DESC) {
+                return true;
+            }
+        }
+        return false;
+    }
 }

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

@@ -98,15 +98,31 @@ public class ContextIndexSearcher extends IndexSearcher implements Releasable {
     private QueryProfiler profiler;
     private MutableQueryTimeout cancellable;
 
+    /**
+     * Certain queries can benefit if we reverse the segment read order,
+     * for example time series based queries if searched for desc sort order
+     */
+    private final boolean reverseLeafReaderContexts;
+
     public ContextIndexSearcher(
         IndexReader reader,
         Similarity similarity,
         QueryCache queryCache,
         QueryCachingPolicy queryCachingPolicy,
         boolean wrapWithExitableDirectoryReader,
-        Executor executor
+        Executor executor,
+        boolean reverseLeafReaderContexts
     ) throws IOException {
-        this(reader, similarity, queryCache, queryCachingPolicy, new MutableQueryTimeout(), wrapWithExitableDirectoryReader, executor);
+        this(
+            reader,
+            similarity,
+            queryCache,
+            queryCachingPolicy,
+            new MutableQueryTimeout(),
+            wrapWithExitableDirectoryReader,
+            executor,
+            reverseLeafReaderContexts
+        );
     }
 
     private ContextIndexSearcher(
@@ -116,13 +132,15 @@ private ContextIndexSearcher(
         QueryCachingPolicy queryCachingPolicy,
         MutableQueryTimeout cancellable,
         boolean wrapWithExitableDirectoryReader,
-        Executor executor
+        Executor executor,
+        boolean reverseLeafReaderContexts
     ) throws IOException {
         super(wrapWithExitableDirectoryReader ? new ExitableDirectoryReader((DirectoryReader) reader, cancellable) : reader, executor);
         setSimilarity(similarity);
         setQueryCache(queryCache);
         setQueryCachingPolicy(queryCachingPolicy);
         this.cancellable = cancellable;
+        this.reverseLeafReaderContexts = reverseLeafReaderContexts;
     }
 
     public void setProfiler(QueryProfiler profiler) {
@@ -246,8 +264,15 @@ public void search(
 
     @Override
     protected void search(List<LeafReaderContext> leaves, Weight weight, Collector collector) throws IOException {
-        for (LeafReaderContext ctx : leaves) { // search each subreader
-            searchLeaf(ctx, weight, collector);
+        if (reverseLeafReaderContexts) {
+            // reverse the segment search order if this flag is true.
+            for (int i = leaves.size() - 1; i >= 0; i--) {
+                searchLeaf(leaves.get(i), weight, collector);
+            }
+        } else {
+            for (int i = 0; i < leaves.size(); i++) {
+                searchLeaf(leaves.get(i), weight, collector);
+            }
         }
     }
 

server/src/test/java/org/opensearch/search/SearchCancellationTests.java

@@ -109,7 +109,8 @@ public void testAddingCancellationActions() throws IOException {
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         );
         NullPointerException npe = expectThrows(NullPointerException.class, () -> searcher.addQueryCancellation(null));
         assertEquals("cancellation runnable should not be null", npe.getMessage());
@@ -129,7 +130,8 @@ public void testCancellableCollector() throws IOException {
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         );
 
         searcher.search(new MatchAllDocsQuery(), collector1);
@@ -157,7 +159,8 @@ public void testExitableDirectoryReader() throws IOException {
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         );
         searcher.addQueryCancellation(cancellation);
         CompiledAutomaton automaton = new CompiledAutomaton(new RegExp("a.*").toAutomaton());

server/src/test/java/org/opensearch/search/internal/ContextIndexSearcherTests.java

@@ -259,7 +259,8 @@ public void onRemoval(ShardId shardId, Accountable accountable) {
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         );
 
         for (LeafReaderContext context : searcher.getIndexReader().leaves()) {

server/src/test/java/org/opensearch/search/profile/query/QueryProfilerTests.java

@@ -102,7 +102,8 @@ public void setUp() throws Exception {
             IndexSearcher.getDefaultQueryCache(),
             ALWAYS_CACHE_POLICY,
             true,
-            null
+            null,
+            false
         );
     }
 

server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java

@@ -1170,7 +1170,8 @@ private static ContextIndexSearcher newContextSearcher(IndexReader reader) throw
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         );
     }
 
@@ -1181,7 +1182,8 @@ private static ContextIndexSearcher newEarlyTerminationContextSearcher(IndexRead
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         ) {
 
             @Override

server/src/test/java/org/opensearch/search/query/QueryProfilePhaseTests.java

@@ -1136,7 +1136,8 @@ private static ContextIndexSearcher newContextSearcher(IndexReader reader) throw
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         );
     }
 
@@ -1147,7 +1148,8 @@ private static ContextIndexSearcher newEarlyTerminationContextSearcher(IndexRead
             IndexSearcher.getDefaultQueryCache(),
             IndexSearcher.getDefaultQueryCachingPolicy(),
             true,
-            null
+            null,
+            false
         ) {
 
             @Override

test/framework/src/main/java/org/opensearch/search/aggregations/AggregatorTestCase.java

@@ -335,7 +335,8 @@ public boolean shouldCache(Query query) {
             queryCache,
             queryCachingPolicy,
             false,
-            null
+            null,
+            false
         );
 
         SearchContext searchContext = mock(SearchContext.class);

test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java

@@ -202,6 +202,7 @@
 import static org.opensearch.discovery.DiscoveryModule.DISCOVERY_SEED_PROVIDERS_SETTING;
 import static org.opensearch.discovery.SettingsBasedSeedHostsProvider.DISCOVERY_SEED_HOSTS_SETTING;
 import static org.opensearch.index.IndexSettings.INDEX_SOFT_DELETES_RETENTION_LEASE_PERIOD_SETTING;
+import static org.opensearch.index.IndexSettings.SEARCH_SEGMENTS_REVERSE_ORDER_OPTIMIZATION_SETTING;
 import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
 import static org.opensearch.test.XContentTestUtils.convertToMap;
 import static org.opensearch.test.XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder;
@@ -761,6 +762,10 @@ public Settings indexSettings() {
                 ).getStringRep()
             );
         }
+
+        // Randomly disable desc sort optimizations to verify non-optimized flow
+        builder.put(SEARCH_SEGMENTS_REVERSE_ORDER_OPTIMIZATION_SETTING.getKey(), randomBoolean());
+
         return builder.build();
     }