iter

Author: lkts

server/src/main/java/org/elasticsearch/index/IndexModule.java

@@ -370,7 +370,7 @@ public void addSimilarity(String name, TriFunction<Settings, IndexVersion, Scrip
     /**
      * Adds a new instance of factory creating new {@link DirectoryReader} wrapper instances.
      * The factory ({@link Function}) is called once the IndexService is fully constructed.
-     * All added wrappers are applied in the order they have been added.
+     * The order of execution of wrappers is not guaranteed.
      * <p>
      * The {@link CheckedFunction} is invoked each time a {@link Engine.Searcher} is requested to do an operation,
      * for example search, and must return a new directory reader wrapping the provided directory reader or if no
@@ -499,13 +499,25 @@ public IndexService newIndexService(
         QueryRewriteInterceptor queryRewriteInterceptor
     ) throws IOException {
         final IndexEventListener eventListener = freeze();
-        Function<IndexService, CheckedFunction<DirectoryReader, DirectoryReader, IOException>> readerWrapperFactory = indexReaderWrappers
-            .isEmpty() ? indexService -> null : indexService -> (directoryReader) -> {
-                var wrapped = indexReaderWrappers.get(0).apply(indexService).apply(directoryReader);
-                for (int i = 1; i < indexReaderWrappers.size(); i++) {
-                    wrapped = indexReaderWrappers.get(i).apply(indexService).apply(wrapped);
-                }
-                return wrapped;
+        Function<IndexService, CheckedFunction<DirectoryReader, DirectoryReader, IOException>> readerWrapperFactory =
+            switch (indexReaderWrappers.size()) {
+                case 0 -> indexService -> null;
+                case 1 -> indexReaderWrappers.get(0);
+                default -> indexService -> {
+                    // Call factories only one time when creating index service to avoid duplicate work.
+                    var wrappers = new ArrayList<CheckedFunction<DirectoryReader, DirectoryReader, IOException>>();
+                    for (var indexReaderWrapper : indexReaderWrappers) {
+                        wrappers.add(indexReaderWrapper.apply(indexService));
+                    }
+
+                    return directoryReader -> {
+                        var wrapped = wrappers.get(0).apply(directoryReader);
+                        for (int i = 1; i < wrappers.size(); i++) {
+                            wrapped = wrappers.get(i).apply(wrapped);
+                        }
+                        return wrapped;
+                    };
+                };
             };
         eventListener.beforeIndexCreated(indexSettings.getIndex(), indexSettings.getSettings());
         final IndexStorePlugin.DirectoryFactory directoryFactory = getDirectoryFactory(indexSettings, directoryFactories);

server/src/test/java/org/elasticsearch/index/IndexModuleTests.java

@@ -12,7 +12,10 @@
 import org.apache.lucene.analysis.standard.StandardTokenizer;
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.FieldInvertState;
+import org.apache.lucene.index.FilterDirectoryReader;
 import org.apache.lucene.index.IndexCommit;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.search.CollectionStatistics;
 import org.apache.lucene.search.QueryCachingPolicy;
 import org.apache.lucene.search.TermStatistics;
@@ -275,6 +278,70 @@ public void testWrapperIsBound() throws IOException {
         closeIndexService(indexService);
     }
 
+    public void testMultipleReaderWrappers() throws IOException {
+        final MockEngineFactory engineFactory = new MockEngineFactory(AssertingDirectoryReader.class);
+        IndexModule module = new IndexModule(
+            indexSettings,
+            emptyAnalysisRegistry,
+            engineFactory,
+            Collections.emptyMap(),
+            () -> true,
+            indexNameExpressionResolver,
+            Collections.emptyMap(),
+            mock(SlowLogFieldProvider.class),
+            MapperMetrics.NOOP,
+            emptyList(),
+            new IndexingStatsSettings(ClusterSettings.createBuiltInClusterSettings()),
+            new SearchStatsSettings(ClusterSettings.createBuiltInClusterSettings()),
+            MergeMetrics.NOOP
+        );
+
+        class Wrapper extends FilterDirectoryReader {
+            final String name;
+            final DirectoryReader wrappedReader;
+
+            Wrapper(String name, DirectoryReader in) throws IOException {
+                super(in, new SubReaderWrapper() {
+                    @Override
+                    public LeafReader wrap(LeafReader reader) {
+                        return reader;
+                    }
+                });
+                this.name = name;
+                this.wrappedReader = in;
+            }
+
+            @Override
+            protected DirectoryReader doWrapDirectoryReader(DirectoryReader in) throws IOException {
+                return null;
+            }
+
+            @Override
+            public CacheHelper getReaderCacheHelper() {
+                return null;
+            }
+        }
+
+        module.addReaderWrapper(s -> reader -> new Wrapper("A", reader));
+        module.addReaderWrapper(s -> reader -> new Wrapper("B", reader));
+
+        IndexService indexService = newIndexService(module);
+
+        var wrapper = indexService.getReaderWrapper();
+
+        try (var directory = newDirectory(); var reader = new DummyDirectoryReader(directory)) {
+            var wrapped = wrapper.apply(reader);
+            // B is the outermost wrapper since it was added last.
+            assertTrue(wrapped instanceof Wrapper w && w.name.equals("B"));
+            var secondLevel = ((Wrapper) wrapped).wrappedReader;
+            assertTrue(secondLevel instanceof Wrapper w && w.name.equals("A"));
+            var thirdLevel = ((Wrapper) secondLevel).wrappedReader;
+            assertTrue(thirdLevel instanceof DummyDirectoryReader);
+        }
+
+        closeIndexService(indexService);
+    }
+
     public void testRegisterIndexStore() throws IOException {
         final Settings settings = Settings.builder()
             .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
@@ -894,4 +961,50 @@ protected WrappedDirectory(Directory in, ShardRouting shardRouting) {
             this.shardRouting = shardRouting;
         }
     }
+
+    private static class DummyDirectoryReader extends DirectoryReader {
+        DummyDirectoryReader(Directory directory) throws IOException {
+            super(directory, new LeafReader[0], null);
+        }
+
+        @Override
+        protected DirectoryReader doOpenIfChanged() throws IOException {
+            return null;
+        }
+
+        @Override
+        protected DirectoryReader doOpenIfChanged(IndexCommit commit) throws IOException {
+            return null;
+        }
+
+        @Override
+        protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws IOException {
+            return null;
+        }
+
+        @Override
+        public long getVersion() {
+            return 0;
+        }
+
+        @Override
+        public boolean isCurrent() throws IOException {
+            return false;
+        }
+
+        @Override
+        public IndexCommit getIndexCommit() throws IOException {
+            return null;
+        }
+
+        @Override
+        protected void doClose() throws IOException {
+
+        }
+
+        @Override
+        public CacheHelper getReaderCacheHelper() {
+            return null;
+        }
+    }
 }