make sure all IndexSearcher instances use an executor (#4008)

Author: vladak

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/IndexSearcherFactory.java

@@ -0,0 +1,42 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * See LICENSE.txt included in this distribution for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at LICENSE.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ */
+package org.opengrok.indexer.configuration;
+
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.SearcherFactory;
+
+/**
+ * Factory for IndexSearcher objects with search executor.
+ */
+public class IndexSearcherFactory extends SearcherFactory {
+    public IndexSearcher newSearcher(IndexReader reader) {
+        return newSearcher(reader, null);
+    }
+
+    @Override
+    public IndexSearcher newSearcher(IndexReader reader, IndexReader prev) {
+        // The previous IndexReader is not used here.
+        return new IndexSearcher(reader, RuntimeEnvironment.getInstance().getSearchExecutor());
+    }
+}

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java

@@ -104,6 +104,9 @@ public final class RuntimeEnvironment {
     private final LazilyInstantiate<ExecutorService> lzRevisionExecutor;
     private static final RuntimeEnvironment instance = new RuntimeEnvironment();
 
+    private final LazilyInstantiate<SuperIndexSearcherFactory> lzSuperIndexSearcherFactory;
+    private final LazilyInstantiate<IndexSearcherFactory> lzIndexSearcherFactory;
+
     private final Map<Project, List<RepositoryInfo>> repository_map = new ConcurrentHashMap<>();
     private final Map<String, SearcherManager> searcherManagerMap = new ConcurrentHashMap<>();
 
@@ -155,10 +158,11 @@ private RuntimeEnvironment() {
         configuration = new Configuration();
         configLock = new CloseableReentrantReadWriteLock();
         watchDog = new WatchDogService();
-        lzIndexerParallelizer = LazilyInstantiate.using(() ->
-                new IndexerParallelizer(this));
+        lzIndexerParallelizer = LazilyInstantiate.using(() -> new IndexerParallelizer(this));
         lzSearchExecutor = LazilyInstantiate.using(this::newSearchExecutor);
         lzRevisionExecutor = LazilyInstantiate.using(this::newRevisionExecutor);
+        lzSuperIndexSearcherFactory = LazilyInstantiate.using(this::newSuperIndexSearcherFactory);
+        lzIndexSearcherFactory = LazilyInstantiate.using(this::newIndexSearcherFactory);
     }
 
     // Instance of authorization framework and its lock.
@@ -211,6 +215,22 @@ public void shutdownRevisionExecutor() throws InterruptedException {
         getRevisionExecutor().awaitTermination(getIndexerCommandTimeout(), TimeUnit.SECONDS);
     }
 
+    private SuperIndexSearcherFactory newSuperIndexSearcherFactory() {
+        return new SuperIndexSearcherFactory();
+    }
+
+    public SuperIndexSearcherFactory getSuperIndexSearcherFactory() {
+        return lzSuperIndexSearcherFactory.get();
+    }
+
+    private IndexSearcherFactory newIndexSearcherFactory() {
+        return new IndexSearcherFactory();
+    }
+
+    public IndexSearcherFactory getIndexSearcherFactory() {
+        return lzIndexSearcherFactory.get();
+    }
+
     /**
      * Get the one and only instance of the RuntimeEnvironment.
      *
@@ -1792,9 +1812,9 @@ public void maybeRefreshIndexSearchers() {
      * Get IndexSearcher for given project.
      * Each IndexSearcher is born from a SearcherManager object. There is one SearcherManager for every project.
      * This schema makes it possible to reuse IndexSearcher/IndexReader objects so the heavy lifting
-     * (esp. system calls) performed in FSDirectory and DirectoryReader happens only once for a project.
-     * The caller has to make sure that the IndexSearcher is returned back
-     * to the SearcherManager. This is done with returnIndexSearcher().
+     * (esp. system calls) performed in {@code FSDirectory} and {@code DirectoryReader} happens only once for a project.
+     * The caller has to make sure that the IndexSearcher is returned to the SearcherManager.
+     * This is done with {@code searcherManagerInstance.release(indexSearcherInstance);}
      * The return of the IndexSearcher should happen only after the search result data are read fully.
      *
      * @param projectName project
@@ -1803,24 +1823,24 @@ public void maybeRefreshIndexSearchers() {
      */
     @SuppressWarnings("java:S2095")
     public SuperIndexSearcher getIndexSearcher(String projectName) throws IOException {
-        SearcherManager mgr = searcherManagerMap.get(projectName);
-        SuperIndexSearcher searcher;
 
+        SearcherManager mgr = searcherManagerMap.get(projectName);
         if (mgr == null) {
             File indexDir = new File(getDataRootPath(), IndexDatabase.INDEX_DIR);
             Directory dir = FSDirectory.open(new File(indexDir, projectName).toPath());
-            mgr = new SearcherManager(dir, new ThreadpoolSearcherFactory());
+            mgr = new SearcherManager(dir, getSuperIndexSearcherFactory());
             searcherManagerMap.put(projectName, mgr);
         }
-        searcher = (SuperIndexSearcher) mgr.acquire();
+
+        SuperIndexSearcher searcher = (SuperIndexSearcher) mgr.acquire();
         searcher.setSearcherManager(mgr);
 
         return searcher;
     }
 
     /**
-     * After new configuration is put into place, the set of projects might
-     * change so we go through the SearcherManager objects and close those where
+     * After new configuration is put into place, the set of projects might change,
+     * so we go through the SearcherManager objects and close those where
      * the corresponding project is no longer present.
      */
     public void refreshSearcherManagerMap() {
@@ -1847,17 +1867,14 @@ public void refreshSearcherManagerMap() {
     }
 
     /**
-     * Return collection of IndexReader objects as MultiReader object
-     * for given list of projects.
-     * The caller is responsible for releasing the IndexSearcher objects
-     * so we add them to the map.
+     * Return collection of IndexReader objects as MultiReader object for given list of projects.
+     * The caller is responsible for releasing the IndexSearcher objects.
      *
      * @param projects list of projects
      * @param searcherList each SuperIndexSearcher produced will be put into this list
      * @return MultiReader for the projects
      */
-    public MultiReader getMultiReader(SortedSet<String> projects,
-        ArrayList<SuperIndexSearcher> searcherList) {
+    public MultiReader getMultiReader(SortedSet<String> projects, List<SuperIndexSearcher> searcherList) {
 
         IndexReader[] subreaders = new IndexReader[projects.size()];
         int ii = 0;

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/ThreadpoolSearcherFactory.java renamed to opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/SuperIndexSearcherFactory.java

@@ -18,7 +18,7 @@
  */
 
  /*
-  * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+  * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.
   */
 package org.opengrok.indexer.configuration;
 
@@ -31,12 +31,11 @@
  * to make sure the searcher threads are constrained to single thread pool.
  * @author vkotal
  */
-class ThreadpoolSearcherFactory extends SearcherFactory {
+class SuperIndexSearcherFactory extends SearcherFactory {
 
     @Override
     public SuperIndexSearcher newSearcher(IndexReader r, IndexReader prev) {
         // The previous IndexReader is not used here.
         return new SuperIndexSearcher(r, RuntimeEnvironment.getInstance().getSearchExecutor());
     }
-
 }

opengrok-indexer/src/main/java/org/opengrok/indexer/history/DirectoryHistoryReader.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
  */
 package org.opengrok.indexer.history;
 
@@ -93,16 +93,17 @@ public class DirectoryHistoryReader {
      * @throws IOException when index cannot be accessed
      */
     public DirectoryHistoryReader(String path) throws IOException {
-        IndexReader ireader = null;
+        IndexReader indexReader = null;
         try {
             // Prepare for index search.
             String srcRoot = RuntimeEnvironment.getInstance().getSourceRootPath();
-            ireader = IndexDatabase.getIndexReader(path);
-            if (ireader == null) {
-                throw new IOException("Could not locate index database");
+            indexReader = IndexDatabase.getIndexReader(path);
+            if (indexReader == null) {
+                throw new IOException(String.format("Could not locate index database for '%s'", path));
             }
             // The search results will be sorted by date.
-            IndexSearcher searcher = new IndexSearcher(ireader);
+            IndexSearcher searcher = RuntimeEnvironment.getInstance().
+                    getIndexSearcherFactory().newSearcher(indexReader);
             SortField sfield = new SortField(QueryBuilder.DATE, SortField.Type.STRING, true);
             Sort sort = new Sort(sfield);
             QueryParser qparser = new QueryParser(QueryBuilder.PATH, new CompatibleAnalyser());
@@ -130,19 +131,20 @@ public DirectoryHistoryReader(String path) throws IOException {
                     try {
                         cdate = DateTools.stringToDate(doc.get(QueryBuilder.DATE));
                     } catch (java.text.ParseException ex) {
-                        LOGGER.log(Level.WARNING, String.format("Could not get date for %s", path), ex);
+                        LOGGER.log(Level.WARNING, String.format("Could not get date for '%s'", path), ex);
                         cdate = new Date();
                     }
                     int ls = rpath.lastIndexOf('/');
                     if (ls != -1) {
                         String rparent = rpath.substring(0, ls);
                         String rbase = rpath.substring(ls + 1);
                         History hist = null;
+                        File f = new File(srcRoot + rparent, rbase);
                         try {
-                            File f = new File(srcRoot + rparent, rbase);
                             hist = HistoryGuru.getInstance().getHistory(f);
                         } catch (HistoryException e) {
-                            LOGGER.log(Level.WARNING, "An error occurred while getting history reader", e);
+                            LOGGER.log(Level.WARNING,
+                                    String.format("An error occurred while getting history reader for '%s'", f), e);
                         }
                         if (hist == null) {
                             put(cdate, "", "-", "", rpath);
@@ -164,11 +166,12 @@ public DirectoryHistoryReader(String path) throws IOException {
             // into history object.
             history = new History(entries);
         } finally {
-            if (ireader != null) {
+            if (indexReader != null) {
                 try {
-                    ireader.close();
+                    indexReader.close();
                 } catch (Exception ex) {
-                    LOGGER.log(Level.WARNING, "An error occurred while closing reader", ex);
+                    LOGGER.log(Level.WARNING,
+                            String.format("An error occurred while closing index reader for '%s'", path), ex);
                 }
             }
         }

opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexAnalysisSettingsAccessor.java

@@ -18,6 +18,7 @@
  */
 
 /*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2018, 2019, Chris Fraire <[email protected]>.
  */
 package org.opengrok.indexer.index;
@@ -37,6 +38,7 @@
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TopDocs;
 import org.opengrok.indexer.analysis.CompatibleAnalyser;
+import org.opengrok.indexer.configuration.RuntimeEnvironment;
 import org.opengrok.indexer.search.QueryBuilder;
 
 /**
@@ -73,9 +75,9 @@ public IndexAnalysisSettings3 read(IndexReader reader) throws IOException {
      * @return a defined instance, which is empty if none could be found
      * @throws IOException if I/O error occurs while searching Lucene
      */
-    public IndexAnalysisSettings3[] read(IndexReader reader, int n)
-            throws IOException {
-        IndexSearcher searcher = new IndexSearcher(reader);
+    public IndexAnalysisSettings3[] read(IndexReader reader, int n) throws IOException {
+
+        IndexSearcher searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(reader);
         Query q;
         try {
             q = new QueryParser(QueryBuilder.OBJUID, new CompatibleAnalyser()).

opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java

@@ -1852,10 +1852,10 @@ public int getNumFiles() throws IOException {
      * Get an indexReader for the Index database where a given file.
      *
      * @param path the file to get the database for
-     * @return The index database where the file should be located or null if it
-     * cannot be located.
+     * @return The index database where the file should be located or {@code null} if it cannot be located.
      */
     @SuppressWarnings("java:S2095")
+    @Nullable
     public static IndexReader getIndexReader(String path) {
         IndexReader ret = null;
 
@@ -1939,7 +1939,7 @@ private static Document getDocument(String path, IndexReader indexReader) throws
 
         Document doc;
         Query q = new QueryBuilder().setPath(path).build();
-        IndexSearcher searcher = new IndexSearcher(indexReader);
+        IndexSearcher searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(indexReader);
         Statistics stat = new Statistics();
         TopDocs top = searcher.search(q, 1);
         stat.report(LOGGER, Level.FINEST, "search via getDocument() done",

opengrok-indexer/src/main/java/org/opengrok/indexer/index/NumLinesLOCAccessor.java

@@ -18,6 +18,7 @@
  */
 
 /*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2020, Chris Fraire <[email protected]>.
  */
 package org.opengrok.indexer.index;
@@ -41,6 +42,7 @@
 import org.opengrok.indexer.analysis.AccumulatedNumLinesLOC;
 import org.opengrok.indexer.analysis.NullableNumLinesLOC;
 import org.opengrok.indexer.analysis.NumLinesLOC;
+import org.opengrok.indexer.configuration.RuntimeEnvironment;
 import org.opengrok.indexer.search.QueryBuilder;
 
 import java.io.File;
@@ -87,16 +89,15 @@ public void store(IndexWriter writer, IndexReader reader,
      * @return a value indicating whether any defined number-of-lines and
      * lines-of-code were found
      */
-    public boolean register(NumLinesLOCAggregator countsAggregator, IndexReader reader)
-            throws IOException {
+    public boolean register(NumLinesLOCAggregator countsAggregator, IndexReader reader) throws IOException {
 
         /*
          * Search for existing documents with any value of PATH. Those are
          * documents representing source code files, as opposed to source code
          * directories or other object data (e.g. IndexAnalysisSettings3), which
          * have no stored PATH.
          */
-        IndexSearcher searcher = new IndexSearcher(reader);
+        IndexSearcher searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(reader);
 
         Query query;
         try {
@@ -138,7 +139,7 @@ private void storeIterative(IndexWriter writer, IndexReader reader,
             List<AccumulatedNumLinesLOC> counts, boolean isAggregatingDeltas) throws IOException {
 
         // Search for existing documents with QueryBuilder.D.
-        IndexSearcher searcher = new IndexSearcher(reader);
+        IndexSearcher searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(reader);
 
         for (AccumulatedNumLinesLOC entry : counts) {
             Query query = new TermQuery(new Term(QueryBuilder.D, entry.getPath()));
@@ -205,7 +206,7 @@ private boolean processFileCounts(NumLinesLOCAggregator countsAggregator,
 
     private DSearchResult newDSearch(IndexReader reader, int n) throws IOException {
         // Search for existing documents with QueryBuilder.D.
-        IndexSearcher searcher = new IndexSearcher(reader);
+        IndexSearcher searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(reader);
         Query query;
         try {
             QueryParser parser = new QueryParser(QueryBuilder.D, new CompatibleAnalyser());