diff --git a/nbproject/project.properties b/nbproject/project.properties index 51288fb7dff..88d40f580b2 100644 --- a/nbproject/project.properties +++ b/nbproject/project.properties @@ -42,7 +42,7 @@ build.classes.dir=${build.dir}/classes dist.dir=dist manifest.file=manifest.mf javac.source=1.8 -run.jvmargs=-ea -enableassertions\:org.opensolaris.opengrok... +run.jvmargs=-ea -enableassertions:org.opensolaris.opengrok... manifest.custom.permissions= lucene-analyzers-common.jar=lucene-analyzers-common-${lucene.version}.jar run.test.classpath=\ diff --git a/src/org/opensolaris/opengrok/configuration/Configuration.java b/src/org/opensolaris/opengrok/configuration/Configuration.java index bdc657b3dd6..aa6ceba7e53 100644 --- a/src/org/opensolaris/opengrok/configuration/Configuration.java +++ b/src/org/opensolaris/opengrok/configuration/Configuration.java @@ -134,6 +134,7 @@ public final class Configuration { private final Map cmds; private int tabSize; private int command_timeout; // in seconds + private int indexRefreshPeriod; // in seconds private boolean scopesEnabled; private boolean foldingEnabled; @@ -225,6 +226,14 @@ public void setCommandTimeout(int timeout) { this.command_timeout = timeout; } + public int getIndexRefreshPeriod() { + return indexRefreshPeriod; + } + + public void setIndexRefreshPeriod(int seconds) { + this.indexRefreshPeriod = seconds; + } + /** * Creates a new instance of Configuration */ @@ -275,6 +284,7 @@ public Configuration() { setRevisionMessageCollapseThreshold(200); setPluginDirectory(null); setMaxSearchThreadCount(2 * Runtime.getRuntime().availableProcessors()); + setIndexRefreshPeriod(60); } public String getRepoCmd(String clazzName) { diff --git a/src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java b/src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java index aff081efe91..fb04006a485 100644 --- a/src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java +++ b/src/org/opensolaris/opengrok/configuration/RuntimeEnvironment.java @@ -18,8 +18,8 @@ */ /* - * Copyright (c) 2006, 2016, Oracle and/or its affiliates. All rights reserved. - */ + * Copyright (c) 2006, 2016, Oracle and/or its affiliates. All rights reserved. + */ package org.opensolaris.opengrok.configuration; import java.beans.XMLDecoder; @@ -67,11 +67,26 @@ import org.opensolaris.opengrok.logger.LoggerFactory; import org.opensolaris.opengrok.util.Executor; import org.opensolaris.opengrok.util.IOUtils; +import org.opensolaris.opengrok.configuration.ThreadpoolSearcherFactory; import static java.nio.file.FileVisitResult.CONTINUE; import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE; import static java.nio.file.StandardWatchEventKinds.ENTRY_DELETE; import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY; +import java.util.Collections; +import java.util.Iterator; +import java.util.SortedSet; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.MultiReader; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.SearcherFactory; +import org.apache.lucene.search.SearcherManager; +import org.apache.lucene.store.AlreadyClosedException; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; +import org.opensolaris.opengrok.index.IndexDatabase; /** @@ -91,6 +106,7 @@ public final class RuntimeEnvironment { private final Map> repository_map = new TreeMap<>(); private final Map> project_group_map = new TreeMap<>(); + private final Map searcherManagerMap = new ConcurrentHashMap<>(); /* Get thread pool used for top-level repository history generation. */ public static synchronized ExecutorService getHistoryExecutor() { @@ -232,6 +248,14 @@ public void setCommandTimeout(int timeout) { threadConfig.get().setCommandTimeout(timeout); } + public int getIndexRefreshPeriod() { + return threadConfig.get().getIndexRefreshPeriod(); + } + + public void setIndexRefreshPeriod(int seconds) { + threadConfig.get().setIndexRefreshPeriod(seconds); + } + /** * Get the path to the where the index database is stored * @@ -1184,6 +1208,8 @@ public void stopConfigurationListenerThread() { IOUtils.close(configServerSocket); } + private Thread configurationListenerThread; + /** * Start a thread to listen on a socket to receive new configurations to * use. @@ -1199,7 +1225,7 @@ public boolean startConfigurationListenerThread(SocketAddress endpoint) { configServerSocket.bind(endpoint); ret = true; final ServerSocket sock = configServerSocket; - Thread t = new Thread(new Runnable() { + configurationListenerThread = new Thread(new Runnable() { @Override public void run() { ByteArrayOutputStream bos = new ByteArrayOutputStream(1 << 13); @@ -1228,7 +1254,15 @@ public void run() { ((Configuration) obj).refreshDateForLastIndexRun(); setConfiguration((Configuration) obj); LOGGER.log(Level.INFO, "Configuration updated: {0}", - configuration.getSourceRoot()); + configuration.getSourceRoot()); + + // We are assuming that each update of configuration + // means reindex. If dedicated thread is introduced + // in the future solely for the purpose of getting + // the event of reindex, the 2 calls below should + // be moved there. + refreshSearcherManagerMap(); + maybeRefreshIndexSearchers(); } } catch (IOException e) { LOGGER.log(Level.SEVERE, "Error reading config file: ", e); @@ -1237,12 +1271,12 @@ public void run() { } } } - }, "conigurationListener"); - t.start(); + }, "configurationListener"); + configurationListenerThread.start(); } catch (UnknownHostException ex) { - LOGGER.log(Level.FINE, "Problem resolving sender: ", ex); + LOGGER.log(Level.WARNING, "Problem resolving sender: ", ex); } catch (IOException ex) { - LOGGER.log(Level.FINE, "I/O error when waiting for config: ", ex); + LOGGER.log(Level.WARNING, "I/O error when waiting for config: ", ex); } if (!ret && configServerSocket != null) { @@ -1343,4 +1377,200 @@ public void stopWatchDogService() { } } } + + private Thread indexReopenThread; + + public void maybeRefreshIndexSearchers() { + for (Map.Entry entry : searcherManagerMap.entrySet()) { + try { + entry.getValue().maybeRefresh(); + } catch (AlreadyClosedException ex) { + // This is a case of removed project. + // See refreshSearcherManagerMap() for details. + } catch (IOException ex) { + LOGGER.log(Level.SEVERE, "maybeRefresh failed", ex); + } + } + } + + /** + * Call maybeRefresh() on each SearcherManager object from dedicated thread + * periodically. + * If the corresponding index has changed in the meantime, it will be safely + * reopened, i.e. without impacting existing IndexSearcher/IndexReader + * objects, thus not disrupting searches in progress. + */ + public void startIndexReopenThread() { + indexReopenThread = new Thread(new Runnable() { + @Override + public void run() { + while (!Thread.currentThread().isInterrupted()) { + try { + maybeRefreshIndexSearchers(); + Thread.sleep(getIndexRefreshPeriod() * 1000); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + } + } + }, "indexReopenThread"); + + indexReopenThread.start(); + } + + public void stopIndexReopenThread() { + if (indexReopenThread != null) { + indexReopenThread.interrupt(); + try { + indexReopenThread.join(); + } catch (InterruptedException ex) { + LOGGER.log(Level.INFO, "Cannot join indexReopen thread: ", ex); + } + } + } + + /** + * 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(). + * The return of the IndexSearcher should happen only after the search + * result data are read fully. + * + * @param proj project + * @return SearcherManager for given project + */ + public IndexSearcher getIndexSearcher(String proj) throws IOException { + SearcherManager mgr = searcherManagerMap.get(proj); + IndexSearcher searcher = null; + if (mgr == null) { + File indexDir = new File(getDataRootPath(), IndexDatabase.INDEX_DIR); + + try { + Directory dir = FSDirectory.open(new File(indexDir, proj).toPath()); + mgr = new SearcherManager(dir, new ThreadpoolSearcherFactory()); + searcherManagerMap.put(proj, mgr); + searcher = mgr.acquire(); + } catch (IOException ex) { + LOGGER.log(Level.SEVERE, + "cannot construct IndexSearcher for project " + proj, ex); + } + } else { + searcher = mgr.acquire(); + } + + return searcher; + } + + /** + * Return IndexSearcher object back to corresponding SearcherManager. + * @param proj project name which belongs to the searcher + * @param searcher searcher object to release + */ + public void returnIndexSearcher(String proj, IndexSearcher searcher) { + SearcherManager mgr = searcherManagerMap.get(proj); + if (mgr != null) { + try { + mgr.release(searcher); + } catch (IOException ex) { + LOGGER.log(Level.SEVERE, "cannot release IndexSearcher for project " + proj, ex); + } + } else { + LOGGER.log(Level.SEVERE, "cannot find SearcherManager for project " + proj); + } + } + + /** + * 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. + */ + private void refreshSearcherManagerMap() { + for (Map.Entry entry : searcherManagerMap.entrySet()) { + try { + // If a project is gone, close the corresponding SearcherManager + // so that it cannot produce new IndexSearcher objects. + for (Project proj : getProjects()) { + if (entry.getKey().compareTo(proj.getDescription()) == 0) { + // XXX Ideally we would like to remove the entry from the map here. + // However, if some thread acquired an IndexSearcher and then config change happened + // and the corresponding searcherManager was removed from the map, + // returnIndexSearcher() will have no place to return the indexSearcher to. + // This would likely lead to leaks since the corresponding IndexReader + // will remain open. + // So, we cannot remove searcherManager from the map until all threads + // are done with it. We could handle this by inserting the pair into + // special to-be-removed list and then check reference count + // of corresponding IndexReader object in returnIndexSearcher(). + // If 0, the SearcherManager can be safely removed from the searcherManagerMap. + // For the time being, let the map to grow unbounded to keep things simple. + entry.getValue().close(); + } + } + } catch (IOException ex) { + LOGGER.log(Level.SEVERE, + "cannot close IndexReader for project" + entry.getKey(), ex); + } + } + } + + /** + * 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. + * + * @param projects list of projects + * @param map each IndexSearcher produced will be put into this map + * @return MultiReader for the projects + */ + public MultiReader getMultiReader(SortedSet projects, Map map) { + IndexReader[] subreaders = new IndexReader[projects.size()]; + int ii = 0; + + // TODO might need to rewrite to Project instead of + // String , need changes in projects.jspf too + for (String proj : projects) { + try { + IndexSearcher searcher = RuntimeEnvironment.getInstance().getIndexSearcher(proj); + subreaders[ii++] = searcher.getIndexReader(); + map.put(proj, searcher); + } catch (IOException ex) { + LOGGER.log(Level.SEVERE, + "cannot get IndexReader for project" + proj, ex); + } + } + MultiReader multiReader = null; + try { + multiReader = new MultiReader(subreaders, true); + } catch (IOException ex) { + LOGGER.log(Level.SEVERE, + "cannot construct MultiReader for set of projects", ex); + } + return multiReader; + } + + /** + * Helper method for the consumers of getMultiReader() to be called when + * destroying search context. This will make sure all indexSearcher + * objects are properly released. + * + * @param map map of project to indexSearcher + */ + public void freeIndexSearcherMap(Map map) { + List toRemove = new ArrayList<>(); + + for (Map.Entry entry : map.entrySet()) { + RuntimeEnvironment.getInstance().returnIndexSearcher(entry.getKey(), entry.getValue()); + toRemove.add(entry.getKey()); + } + + for (String key : toRemove) { + map.remove(key); + } + } } diff --git a/src/org/opensolaris/opengrok/configuration/ThreadpoolSearcherFactory.java b/src/org/opensolaris/opengrok/configuration/ThreadpoolSearcherFactory.java new file mode 100644 index 00000000000..1ef73f5330f --- /dev/null +++ b/src/org/opensolaris/opengrok/configuration/ThreadpoolSearcherFactory.java @@ -0,0 +1,44 @@ +/* + * 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) 2016, Oracle and/or its affiliates. All rights reserved. + */ +package org.opensolaris.opengrok.configuration; + +import java.io.IOException; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.SearcherFactory; + +/** + * Factory for producing IndexSearcher objects. + * This is used inside getIndexSearcher() to produce new SearcherManager objects + * to make sure the searcher threads are constrained to single thread pool. + * @author vkotal + */ +class ThreadpoolSearcherFactory extends SearcherFactory { + @Override + public IndexSearcher newSearcher(IndexReader r, IndexReader prev) throws IOException { + // The previous IndexReader is not used here. + IndexSearcher searcher = new IndexSearcher(r, + RuntimeEnvironment.getInstance().getSearchExecutor()); + return searcher; + } +} diff --git a/src/org/opensolaris/opengrok/search/Results.java b/src/org/opensolaris/opengrok/search/Results.java index 175aba5eeb9..1deb3352e0b 100644 --- a/src/org/opensolaris/opengrok/search/Results.java +++ b/src/org/opensolaris/opengrok/search/Results.java @@ -130,7 +130,7 @@ private static Reader getXrefReader( *
  • {@link SearchHelper#searcher}
  • {@link SearchHelper#hits}
    • *
  • {@link SearchHelper#historyContext} (ignored if {@code null})
    • *
  • {@link SearchHelper#sourceContext} (ignored if {@code null})
    • - *
  • {@link SearchHelper#summerizer} (if sourceContext is not + *
  • {@link SearchHelper#summarizer} (if sourceContext is not * {@code null})
  • {@link SearchHelper#compressed} (if sourceContext * is not {@code null})
  • {@link SearchHelper#sourceRoot} (if * sourceContext or historyContext is not {@code null})
    • @@ -207,15 +207,15 @@ public static void prettyPrint(Writer out, SearchHelper sh, int start, } else { scopes = new Scopes(); } - if (Genre.XREFABLE == genre && sh.summerizer != null) { + if (Genre.XREFABLE == genre && sh.summarizer != null) { String xtags = getTags(xrefDataDir, rpath, sh.compressed); // FIXME use Highlighter from lucene contrib here, // instead of summarizer, we'd also get rid of // apache lucene in whole source ... - out.write(sh.summerizer.getSummary(xtags).toString()); - } else if (Genre.HTML == genre && sh.summerizer != null) { + out.write(sh.summarizer.getSummary(xtags).toString()); + } else if (Genre.HTML == genre && sh.summarizer != null) { String htags = getTags(sh.sourceRoot, rpath, false); - out.write(sh.summerizer.getSummary(htags).toString()); + out.write(sh.summarizer.getSummary(htags).toString()); } else { FileReader r = genre == Genre.PLAIN ? new FileReader(new File(sh.sourceRoot, rpath)) diff --git a/src/org/opensolaris/opengrok/search/Search.java b/src/org/opensolaris/opengrok/search/Search.java index ae1a0f590bd..14f7496a879 100644 --- a/src/org/opensolaris/opengrok/search/Search.java +++ b/src/org/opensolaris/opengrok/search/Search.java @@ -49,8 +49,8 @@ final class Search { private SearchEngine engine; final List results = new ArrayList<>(); - int totalResults =0; - int nhits=0; + int totalResults = 0; + int nhits = 0; @SuppressWarnings({"PMD.SwitchStmtsShouldHaveDefault"}) protected boolean parseCmdLine(String[] argv) { @@ -119,6 +119,7 @@ protected boolean search() { engine.results(0, nhits, results); } totalResults = engine.totalHits; + engine.destroy(); return true; } diff --git a/src/org/opensolaris/opengrok/search/SearchEngine.java b/src/org/opensolaris/opengrok/search/SearchEngine.java index 1d2d10538db..56d43204c92 100644 --- a/src/org/opensolaris/opengrok/search/SearchEngine.java +++ b/src/org/opensolaris/opengrok/search/SearchEngine.java @@ -31,7 +31,13 @@ import java.io.InputStreamReader; import java.io.Reader; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.SortedSet; +import java.util.TreeMap; +import java.util.TreeSet; import java.util.logging.Level; import java.util.logging.Logger; import java.util.zip.GZIPInputStream; @@ -68,6 +74,7 @@ /** * This is an encapsulation of the details on how to search in the index * database. + * This is used for searching from the command line tool and also via the JSON interface. * * @author Trond Norbye 2005 * @author Lubos Kosco - upgrade to lucene 3.x, 4.x, 5.x @@ -84,7 +91,7 @@ public class SearchEngine { //increase the version - every change of below makes us incompatible with the //old index and we need to ask for reindex /** - * version of lucene index common for whole application + * version of Lucene index common for the whole application */ public static final Version LUCENE_VERSION = Version.LATEST; public static final String LUCENE_VERSION_HELP = LUCENE_VERSION.major + "_" + LUCENE_VERSION.minor + "_" + LUCENE_VERSION.bugfix; @@ -132,6 +139,7 @@ public class SearchEngine { private TopScoreDocCollector collector; private IndexSearcher searcher; boolean allCollected; + private final Map indexSearcherMap = new TreeMap<>(); /** * Creates a new instance of SearchEngine @@ -169,7 +177,7 @@ public boolean isValidQuery() { } /** - * + * Search one index. This is used if no projects are set up. * @param paging whether to use paging (if yes, first X pages will load * faster) * @param root which db to search @@ -194,27 +202,23 @@ private void searchSingleDatabase(File root, boolean paging) throws IOException } /** - * + * Perform search on multiple indexes in parallel. * @param paging whether to use paging (if yes, first X pages will load * faster) * @param root list of projects to search * @throws IOException */ private void searchMultiDatabase(List root, boolean paging) throws IOException { - IndexReader[] subreaders = new IndexReader[root.size()]; - File droot = new File(RuntimeEnvironment.getInstance().getDataRootFile(), IndexDatabase.INDEX_DIR); - int ii = 0; - for (Project project : root) { - IndexReader ireader = (DirectoryReader.open(FSDirectory.open(new File(droot, project.getPath()).toPath()))); - subreaders[ii++] = ireader; - } - MultiReader searchables = new MultiReader(subreaders, true); - if (Runtime.getRuntime().availableProcessors() > 1) { - searcher = new IndexSearcher(searchables, - RuntimeEnvironment.getInstance().getSearchExecutor()); - } else { - searcher = new IndexSearcher(searchables); + SortedSet projects = new TreeSet<>(); + for (Project p : root) { + projects.add(p.getDescription()); } + // We use MultiReader even for single project. This should + // not matter given that MultiReader is just a cheap wrapper + // around set of IndexReader objects. + MultiReader searchables = RuntimeEnvironment.getInstance(). + getMultiReader(projects, indexSearcherMap); + searcher = new IndexSearcher(searchables); collector = TopScoreDocCollector.create(hitsPerPage * cachePages); searcher.search(query, collector); totalHits = collector.getTotalHits(); @@ -242,7 +246,10 @@ public String getQuery() { * Before calling this function, * you must set the appropriate search criteria with the set-functions. Note * that this search will return the first cachePages of hitsPerPage, for - * more you need to call more + * more you need to call more. + * + * Call to search() must be eventually followed by call to destroy() + * so that IndexSearcher objects are properly freed. * * @return The number of hits * @see ProjectHelper#getAllProjects() @@ -260,7 +267,10 @@ public int search(HttpServletRequest req) { * Before calling this function, you must set the * appropriate search criteria with the set-functions. Note that this search * will return the first cachePages of hitsPerPage, for more you need to - * call more + * call more. + * + * Call to search() must be eventually followed by call to destroy() + * so that IndexSearcher objects are properly freed. * * @return The number of hits */ @@ -277,6 +287,9 @@ public int search() { * If @param projects is an empty list it tries to search in @code * searchSingleDatabase with root set to @param root * + * Call to search() must be eventually followed by call to destroy() + * so that IndexSearcher objects are properly freed. + * * @return The number of hits */ private int search(List projects, File root) { @@ -337,7 +350,7 @@ private int search(List projects, File root) { /** * get results , if no search was started before, no results are returned - * this method will requery if end end is more than first query from search, + * this method will requery if end is more than first query from search, * hence performance hit applies, if you want results in later pages than * number of cachePages also end has to be bigger than start ! * @@ -449,7 +462,10 @@ public void results(int start, int end, List ret) { Level.WARNING, SEARCH_EXCEPTION_MSG, e); } } + } + public void destroy() { + RuntimeEnvironment.getInstance().freeIndexSearcherMap(this.indexSearcherMap); } /** diff --git a/src/org/opensolaris/opengrok/web/JSONSearchServlet.java b/src/org/opensolaris/opengrok/web/JSONSearchServlet.java index 6996768545b..ad3245394a1 100644 --- a/src/org/opensolaris/opengrok/web/JSONSearchServlet.java +++ b/src/org/opensolaris/opengrok/web/JSONSearchServlet.java @@ -106,9 +106,12 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) result.put(PARAM_HIST, hist); } - if (valid) { - long start = System.currentTimeMillis(); + if (!valid) { + return; + } + try { + long start = System.currentTimeMillis(); int numResults = engine.search(req); int maxResults = MAX_RESULTS; String maxResultsParam = req.getParameter(PARAM_MAXRESULTS); @@ -141,7 +144,12 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) result.put(ATTRIBUTE_RESULT_COUNT, results.size()); result.put(ATTRIBUTE_RESULTS, resultsArray); + + + + resp.getWriter().write(result.toString()); + } finally { + engine.destroy(); } - resp.getWriter().write(result.toString()); } } diff --git a/src/org/opensolaris/opengrok/web/PageConfig.java b/src/org/opensolaris/opengrok/web/PageConfig.java index b5ec425742d..9b188c9ab5a 100644 --- a/src/org/opensolaris/opengrok/web/PageConfig.java +++ b/src/org/opensolaris/opengrok/web/PageConfig.java @@ -513,18 +513,8 @@ public QueryBuilder getQueryBuilder() { .setPath(req.getParameter(QueryBuilder.PATH)) .setHist(req.getParameter(QueryBuilder.HIST)) .setType(req.getParameter(QueryBuilder.TYPE)); - - // This is for backward compatibility with links created by OpenGrok - // 0.8.x and earlier. We used to concatenate the entire query into a - // single string and send it in the t parameter. If we get such a - // link, just add it to the freetext field, and we'll get the old - // behaviour. We can probably remove this code in the first feature - // release after 0.9. - String t = req.getParameter("t"); - if (t != null) { - queryBuilder.setFreetext(t); - } } + return queryBuilder; } diff --git a/src/org/opensolaris/opengrok/web/SearchHelper.java b/src/org/opensolaris/opengrok/web/SearchHelper.java index 2f8572105e4..e041b042553 100644 --- a/src/org/opensolaris/opengrok/web/SearchHelper.java +++ b/src/org/opensolaris/opengrok/web/SearchHelper.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved. * Portions copyright (c) 2011 Jens Elkner. */ package org.opensolaris.opengrok.web; @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Set; import java.util.SortedSet; +import java.util.TreeMap; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -44,6 +45,7 @@ import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; import org.apache.lucene.search.ScoreDoc; +import org.apache.lucene.search.SearcherManager; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; import org.apache.lucene.search.TermQuery; @@ -144,6 +146,16 @@ public class SearchHelper { * {@link #prepareExec(SortedSet)}. */ public IndexSearcher searcher; + /** + * If performing multi-project search, the indexSearcher objects will be + * tracked by the indexSearcherMap so that they can be properly released + * once the results are read. + */ + private final Map indexSearcherMap = new TreeMap<>(); + /** + * close IndexReader associated with searches on destroy() + */ + private Boolean closeOnDestroy; /** * list of docs which result from the executing the query */ @@ -158,7 +170,7 @@ public class SearchHelper { */ public Query query; /** - * the lucene sort instruction based on {@link #order} created via + * the Lucene sort instruction based on {@link #order} created via * {@link #prepareExec(SortedSet)}. */ protected Sort sort; @@ -178,7 +190,7 @@ public class SearchHelper { /** * result summarizer usually created via {@link #prepareSummary()}. */ - public Summarizer summerizer = null; + public Summarizer summarizer = null; /** * history context usually created via {@link #prepareSummary()}. */ @@ -221,8 +233,8 @@ public static Set> getFileTypeDescriptions() { *
  • {@link #projects}
  • {@link #errorMsg} if an error occurs
    • * * - * @param projects project to use query. If empty, a none-project opengrok - * setup is assumed (i.e. DATA_ROOT/index will be used instead of possible + * @param projects project to use query. If empty, a no-project setup + * is assumed (i.e. DATA_ROOT/index will be used instead of possible * multiple DATA_ROOT/$project/index). * @return this instance */ @@ -240,34 +252,24 @@ public SearchHelper prepareExec(SortedSet projects) { } this.projects = projects; if (projects.isEmpty()) { - //no project setup + // no project setup FSDirectory dir = FSDirectory.open(indexDir.toPath()); searcher = new IndexSearcher(DirectoryReader.open(dir)); - } else if (projects.size() == 1) { - // just 1 project selected - FSDirectory dir - = FSDirectory.open(new File(indexDir, projects.first()).toPath()); - searcher = new IndexSearcher(DirectoryReader.open(dir)); + closeOnDestroy = true; } else { - //more projects - IndexReader[] subreaders = new IndexReader[projects.size()]; - int ii = 0; - //TODO might need to rewrite to Project instead of - // String , need changes in projects.jspf too - for (String proj : projects) { - FSDirectory dir = FSDirectory.open(new File(indexDir, proj).toPath()); - subreaders[ii++] = DirectoryReader.open(dir); - } - MultiReader searchables = new MultiReader(subreaders, true); - searcher = parallel - ? new IndexSearcher(searchables, - RuntimeEnvironment.getInstance().getSearchExecutor()) - : new IndexSearcher(searchables); + // We use MultiReader even for single project. This should + // not matter given that MultiReader is just a cheap wrapper + // around set of IndexReader objects. + closeOnDestroy = false; + searcher = new IndexSearcher(RuntimeEnvironment.getInstance(). + getMultiReader(projects, indexSearcherMap)); } + // TODO check if below is somehow reusing sessions so we don't // requery again and again, I guess 2min timeout sessions could be // usefull, since you click on the next page within 2mins, if not, // then wait ;) + // Most probably they are not reused. SearcherLifetimeManager might help here. switch (order) { case LASTMODIFIED: sort = new Sort(new SortField(QueryBuilder.DATE, SortField.Type.STRING, true)); @@ -362,7 +364,8 @@ private void getSuggestion(Term term, IndexReader ir, String[] toks = TABSPACE.split(term.text(), 0); for (String tok : toks) { //TODO below seems to be case insensitive ... for refs/defs this is bad - SuggestWord[] words = checker.suggestSimilar(new Term(term.field(), tok), SPELLCHECK_SUGGEST_WORD_COUNT, ir, SuggestMode.SUGGEST_ALWAYS); + SuggestWord[] words = checker.suggestSimilar(new Term(term.field(), tok), + SPELLCHECK_SUGGEST_WORD_COUNT, ir, SuggestMode.SUGGEST_ALWAYS); for (SuggestWord w : words) { result.add(w.string); } @@ -404,8 +407,19 @@ public List getSuggestions() { for (String proj : name) { Suggestion s = new Suggestion(proj); try { - dir = FSDirectory.open(new File(indexDir, proj).toPath()); - ir = DirectoryReader.open(dir); + if (!closeOnDestroy) { + // Likely, the IndexSearcher has already been created by prepareExec() + // so just reuse it. + IndexSearcher searcher = indexSearcherMap.get(proj); + if (searcher == null) { + searcher = RuntimeEnvironment.getInstance().getIndexSearcher(proj); + indexSearcherMap.put(proj, searcher); + } + ir = searcher.getIndexReader(); + } else { + dir = FSDirectory.open(new File(indexDir, proj).toPath()); + ir = DirectoryReader.open(dir); + } if (builder.getFreetext() != null && !builder.getFreetext().isEmpty()) { t = new Term(QueryBuilder.FULL, builder.getFreetext()); @@ -435,7 +449,7 @@ public List getSuggestions() { LOGGER.log(Level.WARNING, "Got exception while getting " + "spelling suggestions: ", e); } finally { - if (ir != null) { + if (ir != null && closeOnDestroy) { try { ir.close(); } catch (IOException ex) { @@ -456,7 +470,7 @@ public List getSuggestions() { * Parameters which should be populated/set at this time:
      *
    • {@link #query}
    • {@link #builder}
    Populates/sets: * Otherwise the following fields are set (includes {@code null}):
      - *
    • {@link #sourceContext}
    • {@link #summerizer}
      • + *
    • {@link #sourceContext}
    • {@link #summarizer}
      • *
    • {@link #historyContext}
    * * @return this instance. @@ -467,9 +481,9 @@ public SearchHelper prepareSummary() { } try { sourceContext = new Context(query, builder.getQueries()); - summerizer = new Summarizer(query, new CompatibleAnalyser()); + summarizer = new Summarizer(query, new CompatibleAnalyser()); } catch (Exception e) { - LOGGER.log(Level.WARNING, "Summerizer: {0}", e.getMessage()); + LOGGER.log(Level.WARNING, "Summarizer: {0}", e.getMessage()); } try { historyContext = new HistoryContext(query); @@ -481,11 +495,13 @@ public SearchHelper prepareSummary() { /** * Free any resources associated with this helper (that includes closing the - * used {@link #searcher}). + * used {@link #searcher} in case of no-project setup). */ public void destroy() { - if (searcher != null) { + if (searcher != null && closeOnDestroy) { IOUtils.close(searcher.getIndexReader()); } + + RuntimeEnvironment.getInstance().freeIndexSearcherMap(this.indexSearcherMap); } -} +} \ No newline at end of file diff --git a/src/org/opensolaris/opengrok/web/WebappListener.java b/src/org/opensolaris/opengrok/web/WebappListener.java index 48a2ec6ca5b..af624f7a9f3 100644 --- a/src/org/opensolaris/opengrok/web/WebappListener.java +++ b/src/org/opensolaris/opengrok/web/WebappListener.java @@ -114,6 +114,8 @@ public void contextInitialized(final ServletContextEvent servletContextEvent) { if (pluginDirectory != null && watchDog != null && Boolean.parseBoolean(watchDog)) { RuntimeEnvironment.getInstance().startWatchDogService(new File(pluginDirectory)); } + + RuntimeEnvironment.getInstance().startIndexReopenThread(); } /** @@ -123,6 +125,7 @@ public void contextInitialized(final ServletContextEvent servletContextEvent) { public void contextDestroyed(final ServletContextEvent servletContextEvent) { RuntimeEnvironment.getInstance().stopConfigurationListenerThread(); RuntimeEnvironment.getInstance().stopWatchDogService(); + RuntimeEnvironment.getInstance().stopIndexReopenThread(); } /** @@ -131,6 +134,10 @@ public void contextDestroyed(final ServletContextEvent servletContextEvent) { @Override public void requestDestroyed(ServletRequestEvent e) { PageConfig.cleanup(e.getServletRequest()); + SearchHelper sh = (SearchHelper) e.getServletRequest().getAttribute("SearchHelper"); + if (sh != null) { + sh.destroy(); + } } /** diff --git a/test/org/opensolaris/opengrok/search/SearchEngineTest.java b/test/org/opensolaris/opengrok/search/SearchEngineTest.java index 9e3ffa74c3f..61c0521d09e 100644 --- a/test/org/opensolaris/opengrok/search/SearchEngineTest.java +++ b/test/org/opensolaris/opengrok/search/SearchEngineTest.java @@ -161,6 +161,7 @@ public void testSearch() { } List hits = new ArrayList(); + SearchEngine instance = new SearchEngine(); instance.setHistory("\"Add lint make target and fix lint warnings\""); int noHits = instance.search(); @@ -168,6 +169,7 @@ public void testSearch() { instance.results(0, noHits, hits); assertEquals(noHits, hits.size()); } + instance.destroy(); instance = new SearchEngine(); instance.setSymbol("printf"); @@ -180,10 +182,10 @@ public void testSearch() { assertEquals("main.c", hit.getFilename()); assertEquals(1, 1); } - instance.setFile("main.c OR Makefile"); noHits = instance.search(); assertEquals(8, noHits); + instance.destroy(); instance = new SearchEngine(); instance.setFreetext("arguments"); @@ -198,6 +200,7 @@ public void testSearch() { } } assertEquals(8, noHits); + instance.destroy(); instance = new SearchEngine(); instance.setDefinition("main"); @@ -212,6 +215,7 @@ public void testSearch() { } } assertEquals(8, noHits); + instance.destroy(); // wildcards and case sensitivity of definition search instance = new SearchEngine(); @@ -224,6 +228,7 @@ public void testSearch() { instance.setDefinition("MaI*"); // should not match Main instance.search(); assertEquals(0, instance.search()); + instance.destroy(); // wildcards and case sensitivity of symbol search instance = new SearchEngine(); @@ -234,29 +239,36 @@ public void testSearch() { instance.setSymbol("MaI*"); // should not match Main instance.search(); assertEquals(0, instance.search()); + instance.destroy(); // wildcards and case insensitivity of freetext search instance = new SearchEngine(); instance.setFreetext("MaI*"); // should match both Main and main instance.setFile("\"Main.java\" OR \"main.c\""); assertEquals(10, instance.search()); + instance.destroy(); // file name search is case insensitive instance = new SearchEngine(); instance.setFile("JaVa"); // should match java assertEquals(8, instance.search()); + instance.destroy(); //test eol and eof instance = new SearchEngine(); instance.setFreetext("makeW"); - assertEquals(1, instance.search()); + assertEquals(1, instance.search()); + instance.destroy(); + instance = new SearchEngine(); instance.setFreetext("WeirdEOL"); assertEquals(1, instance.search()); + instance.destroy(); //test bcel jar parser instance = new SearchEngine(); instance.setFreetext("InstConstraintVisitor"); - assertEquals(1, instance.search()); + assertEquals(1, instance.search()); + instance.destroy(); } } diff --git a/test/org/opensolaris/opengrok/search/SearchTest.java b/test/org/opensolaris/opengrok/search/SearchTest.java index 107fe07b45b..4c8144e0632 100644 --- a/test/org/opensolaris/opengrok/search/SearchTest.java +++ b/test/org/opensolaris/opengrok/search/SearchTest.java @@ -42,7 +42,7 @@ import org.opensolaris.opengrok.util.TestRepository; /** - * Basic testing of the Search class + * Basic testing of the Search class, i.e. the command line utility. * * @author Trond Norbye */ @@ -139,7 +139,9 @@ public void testSearch() { if (skip) { return; } + Search instance = new Search(); + assertFalse(instance.search()); assertTrue(instance.parseCmdLine(new String[]{"-p", "Makefile"})); assertTrue(instance.search()); diff --git a/test/org/opensolaris/opengrok/util/TestRepository.java b/test/org/opensolaris/opengrok/util/TestRepository.java index de7ce7ea66f..3611b35c78b 100644 --- a/test/org/opensolaris/opengrok/util/TestRepository.java +++ b/test/org/opensolaris/opengrok/util/TestRepository.java @@ -29,6 +29,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; import org.opensolaris.opengrok.configuration.RuntimeEnvironment; @@ -88,13 +89,19 @@ public String getDataRoot() { return dataRoot.getAbsolutePath(); } - private final static String dummyS = "dummy"; + private final static String dummyS = "dummy.txt"; - public void addDummyFile(String project) throws IOException { + public File addDummyFile(String project) throws IOException { File dummy = new File(getSourceRoot() + File.separator + project + File.separator + dummyS); if (!dummy.exists()) { dummy.createNewFile(); } + return dummy; + } + + public void addDummyFile(String project, String contents) throws IOException { + File dummy = addDummyFile(project); + Files.write(dummy.toPath(), contents.getBytes()); } public void removeDummyFile(String project) { diff --git a/test/org/opensolaris/opengrok/web/SearchHelperTest.java b/test/org/opensolaris/opengrok/web/SearchHelperTest.java index e74068ae2d3..0d19293e95c 100644 --- a/test/org/opensolaris/opengrok/web/SearchHelperTest.java +++ b/test/org/opensolaris/opengrok/web/SearchHelperTest.java @@ -18,18 +18,159 @@ */ /* - * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved. */ package org.opensolaris.opengrok.web; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.SortedSet; +import java.util.TreeSet; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import static org.junit.Assert.assertTrue; +import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; +import org.opensolaris.opengrok.configuration.RuntimeEnvironment; +import org.opensolaris.opengrok.index.Indexer; +import org.opensolaris.opengrok.index.IndexerTest; +import org.opensolaris.opengrok.search.QueryBuilder; +import org.opensolaris.opengrok.util.TestRepository; /** * Unit tests for the {@code SearchHelper} class. */ public class SearchHelperTest { + TestRepository repository; + private final String ctagsProperty = "org.opensolaris.opengrok.analysis.Ctags"; + RuntimeEnvironment env; + @BeforeClass + public static void setUpClass() throws Exception { + } + + @AfterClass + public static void tearDownClass() throws Exception { + } + + @Before + public void setUp() throws IOException { + repository = new TestRepository(); + repository.create(IndexerTest.class.getResourceAsStream("source.zip")); + + env = RuntimeEnvironment.getInstance(); + env.setSourceRoot(repository.getSourceRoot()); + env.setDataRoot(repository.getDataRoot()); + env.setVerbose(true); + } + + @After + public void tearDown() { + repository.destroy(); + } + + private void reindex() throws Exception { + System.out.println("Generating index by using the class methods"); + + Indexer.getInstance().prepareIndexer(env, true, true, "/c", null, + false, false, false, null, null, new ArrayList<>(), false); + Indexer.getInstance().doIndexerExecution(true, 1, null, null); + } + + private SearchHelper getSearchHelper(String searchTerm) { + SearchHelper sh = new SearchHelper(); + + sh.dataRoot = env.getDataRootFile(); // throws Exception if none-existent + sh.order = SortOrder.RELEVANCY; + sh.builder = new QueryBuilder().setFreetext(searchTerm); + Assert.assertNotSame(0, sh.builder.getSize()); + sh.start = 0; + sh.maxItems = env.getHitsPerPage(); + sh.contextPath = env.getUrlPrefix(); + sh.parallel = Runtime.getRuntime().availableProcessors() > 1; + sh.isCrossRefSearch = false; + sh.compressed = env.isCompressXref(); + sh.desc = null; + sh.sourceRoot = env.getSourceRootFile(); + sh.lastEditedDisplayMode = false; + + return sh; + } + + @Test + public void testSearchAfterReindex() { + SortedSet projects = new TreeSet<>(); + SearchHelper searchHelper; + + env.setCtags(System.getProperty(ctagsProperty, "ctags")); + if (!env.validateExuberantCtags()) { + System.out.println("Skipping test. Could not find a ctags I could use in path."); + return; + } + + try { + reindex(); + } catch (Exception ex) { + Assert.fail("failed to reindex: " + ex); + } + + // Search for existing term in single project. + projects.add("/c"); + searchHelper = this.getSearchHelper("foobar") + .prepareExec(projects).executeQuery().prepareSummary(); + Assert.assertNull(searchHelper.errorMsg); + System.out.println("single project search returned " + + Integer.toString(searchHelper.totalHits) + " hits"); + Assert.assertEquals(4, searchHelper.totalHits); + searchHelper.destroy(); + + // Search for existing term in multiple projects. + projects.add("/document"); + searchHelper = this.getSearchHelper("foobar") + .prepareExec(projects).executeQuery().prepareSummary(); + Assert.assertNull(searchHelper.errorMsg); + System.out.println("multi-project search returned " + + Integer.toString(searchHelper.totalHits) + " hits"); + Assert.assertEquals(5, searchHelper.totalHits); + searchHelper.destroy(); + + // Search for non-existing term. + searchHelper = this.getSearchHelper("CannotExistAnywhereForSure") + .prepareExec(projects).executeQuery().prepareSummary(); + Assert.assertNull(searchHelper.errorMsg); + System.out.println("multi-project search for non-existing term returned " + + Integer.toString(searchHelper.totalHits) + " hits"); + Assert.assertEquals(0, searchHelper.totalHits); + searchHelper.destroy(); + + // Add a change to the repository, reindex, try to reopen the indexes + // and repeat the search. + try { + repository.addDummyFile("c", "foobar"); + } catch (IOException ex) { + Assert.fail("failed to create and write a new file: " + ex); + } + try { + reindex(); + } catch (Exception ex) { + Assert.fail("failed to reindex: " + ex); + } + env.maybeRefreshIndexSearchers(); + searchHelper = this.getSearchHelper("foobar") + .prepareExec(projects).executeQuery().prepareSummary(); + Assert.assertNull(searchHelper.errorMsg); + System.out.println("multi-project search after reindex returned " + + Integer.toString(searchHelper.totalHits) + " hits"); + Assert.assertEquals(6, searchHelper.totalHits); + searchHelper.destroy(); + repository.removeDummyFile("c"); + } + /** * Test that calling destroy() on an uninitialized instance does not * fail. Used to fail with a NullPointerException. See bug #19232. diff --git a/web/search.jsp b/web/search.jsp index 72018e6245c..a3fb26f2b4a 100644 --- a/web/search.jsp +++ b/web/search.jsp @@ -68,8 +68,9 @@ include file="projects.jspf" long starttime = System.currentTimeMillis(); - SearchHelper searchHelper = cfg.prepareSearch() - .prepareExec(cfg.getRequestedProjects()).executeQuery().prepareSummary(); + SearchHelper searchHelper = cfg.prepareSearch(); + request.setAttribute("SearchHelper", searchHelper); + searchHelper.prepareExec(cfg.getRequestedProjects()).executeQuery().prepareSummary(); if (searchHelper.redirect != null) { response.sendRedirect(searchHelper.redirect); }