You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by kr...@apache.org on 2016/11/18 16:42:33 UTC
[08/20] lucene-solr:jira/solr-8593: LUCENE-7564:
AnalyzingInfixSuggester should close its IndexWriter by default at the end of
build()
LUCENE-7564: AnalyzingInfixSuggester should close its IndexWriter by default at the end of build()
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/f9a0693b
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/f9a0693b
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/f9a0693b
Branch: refs/heads/jira/solr-8593
Commit: f9a0693bf98a4000b6568e7c63f3e303118470bd
Parents: e402a30
Author: Steve Rowe <sa...@apache.org>
Authored: Wed Nov 16 18:03:51 2016 -0500
Committer: Steve Rowe <sa...@apache.org>
Committed: Wed Nov 16 18:03:51 2016 -0500
----------------------------------------------------------------------
lucene/CHANGES.txt | 3 +
.../analyzing/AnalyzingInfixSuggester.java | 82 +++++++++++---
.../analyzing/AnalyzingInfixSuggesterTest.java | 111 +++++++++++++++++++
3 files changed, 181 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f9a0693b/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 28ec5f0..27d6cc9 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -87,6 +87,9 @@ Improvements
* LUCENE-7524: Added more detailed explanation of how IDF is computed in
ClassicSimilarity and BM25Similarity. (Adrien Grand)
+
+* LUCENE-7564: AnalyzingInfixSuggester should close its IndexWriter by default
+ at the end of build(). (Steve Rowe)
* LUCENE-7526: Enhanced UnifiedHighlighter's passage relevancy for queries with
wildcards and sometimes just terms. Added shouldPreferPassageRelevancyOverSpeed()
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f9a0693b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java
----------------------------------------------------------------------
diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java
index aa60237..b8c2dbd 100644
--- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java
+++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java
@@ -129,9 +129,10 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
private final boolean highlight;
private final boolean commitOnBuild;
+ private final boolean closeIndexWriterOnBuild;
/** Used for ongoing NRT additions/updates. */
- private IndexWriter writer;
+ protected IndexWriter writer;
/** {@link IndexSearcher} used for lookups. */
protected SearcherManager searcherMgr;
@@ -146,6 +147,9 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
/** Default higlighting option. */
public static final boolean DEFAULT_HIGHLIGHT = true;
+ /** Default option to close the IndexWriter once the index has been built. */
+ protected final static boolean DEFAULT_CLOSE_INDEXWRITER_ON_BUILD = true;
+
/** How we sort the postings and search results. */
private static final Sort SORT = new Sort(new SortField("weight", SortField.Type.LONG, true));
@@ -198,8 +202,34 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
*
*/
public AnalyzingInfixSuggester(Directory dir, Analyzer indexAnalyzer, Analyzer queryAnalyzer, int minPrefixChars,
- boolean commitOnBuild,
+ boolean commitOnBuild,
boolean allTermsRequired, boolean highlight) throws IOException {
+ this(dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild, allTermsRequired, highlight,
+ DEFAULT_CLOSE_INDEXWRITER_ON_BUILD);
+ }
+
+ /** Create a new instance, loading from a previously built
+ * AnalyzingInfixSuggester directory, if it exists. This directory must be
+ * private to the infix suggester (i.e., not an external
+ * Lucene index). Note that {@link #close}
+ * will also close the provided directory.
+ *
+ * @param minPrefixChars Minimum number of leading characters
+ * before PrefixQuery is used (default 4).
+ * Prefixes shorter than this are indexed as character
+ * ngrams (increasing index size but making lookups
+ * faster).
+ *
+ * @param commitOnBuild Call commit after the index has finished building. This would persist the
+ * suggester index to disk and future instances of this suggester can use this pre-built dictionary.
+ *
+ * @param allTermsRequired All terms in the suggest query must be matched.
+ * @param highlight Highlight suggest query in suggestions.
+ * @param closeIndexWriterOnBuild If true, the IndexWriter will be closed after the index has finished building.
+ */
+ public AnalyzingInfixSuggester(Directory dir, Analyzer indexAnalyzer, Analyzer queryAnalyzer, int minPrefixChars,
+ boolean commitOnBuild, boolean allTermsRequired,
+ boolean highlight, boolean closeIndexWriterOnBuild) throws IOException {
if (minPrefixChars < 0) {
throw new IllegalArgumentException("minPrefixChars must be >= 0; got: " + minPrefixChars);
@@ -212,6 +242,7 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
this.commitOnBuild = commitOnBuild;
this.allTermsRequired = allTermsRequired;
this.highlight = highlight;
+ this.closeIndexWriterOnBuild = closeIndexWriterOnBuild;
if (DirectoryReader.indexExists(dir)) {
// Already built; open it:
@@ -276,15 +307,22 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
}
//System.out.println("initial indexing time: " + ((System.nanoTime()-t0)/1000000) + " msec");
- if (commitOnBuild) {
+ if (commitOnBuild || closeIndexWriterOnBuild) {
commit();
}
searcherMgr = new SearcherManager(writer, null);
success = true;
} finally {
- if (success == false && writer != null) {
- writer.rollback();
- writer = null;
+ if (success) {
+ if (closeIndexWriterOnBuild) {
+ writer.close();
+ writer = null;
+ }
+ } else { // failure
+ if (writer != null) {
+ writer.rollback();
+ writer = null;
+ }
}
}
}
@@ -294,9 +332,13 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
* @see IndexWriter#commit */
public void commit() throws IOException {
if (writer == null) {
- throw new IllegalStateException("Cannot commit on an closed writer. Add documents first");
+ if (searcherMgr == null || closeIndexWriterOnBuild == false) {
+ throw new IllegalStateException("Cannot commit on an closed writer. Add documents first");
+ }
+ // else no-op: writer was committed and closed after the index was built, so commit is unnecessary
+ } else {
+ writer.commit();
}
- writer.commit();
}
private Analyzer getGramAnalyzer() {
@@ -321,13 +363,17 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
private synchronized void ensureOpen() throws IOException {
if (writer == null) {
- if (searcherMgr != null) {
- searcherMgr.close();
- searcherMgr = null;
+ if (DirectoryReader.indexExists(dir)) {
+ // Already built; open it:
+ writer = new IndexWriter(dir, getIndexWriterConfig(getGramAnalyzer(), IndexWriterConfig.OpenMode.APPEND));
+ } else {
+ writer = new IndexWriter(dir, getIndexWriterConfig(getGramAnalyzer(), IndexWriterConfig.OpenMode.CREATE));
}
- writer = new IndexWriter(dir,
- getIndexWriterConfig(getGramAnalyzer(), IndexWriterConfig.OpenMode.CREATE));
+ SearcherManager oldSearcherMgr = searcherMgr;
searcherMgr = new SearcherManager(writer, null);
+ if (oldSearcherMgr != null) {
+ oldSearcherMgr.close();
+ }
}
}
@@ -382,7 +428,11 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
if (searcherMgr == null) {
throw new IllegalStateException("suggester was not built");
}
- searcherMgr.maybeRefreshBlocking();
+ if (writer != null) {
+ searcherMgr.maybeRefreshBlocking();
+ }
+ // else no-op: writer was committed and closed after the index was built
+ // and before searchMgr was constructed, so refresh is unnecessary
}
/**
@@ -791,9 +841,11 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable {
}
if (writer != null) {
writer.close();
- dir.close();
writer = null;
}
+ if (dir != null) {
+ dir.close();
+ }
}
@Override
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f9a0693b/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggesterTest.java
----------------------------------------------------------------------
diff --git a/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggesterTest.java b/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggesterTest.java
index d98d052..fc5e2b7 100644
--- a/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggesterTest.java
+++ b/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggesterTest.java
@@ -35,11 +35,14 @@ import org.apache.lucene.analysis.StopFilter;
import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
import org.apache.lucene.analysis.tokenattributes.OffsetAttribute;
+import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.SearcherManager;
import org.apache.lucene.search.suggest.Input;
import org.apache.lucene.search.suggest.InputArrayIterator;
import org.apache.lucene.search.suggest.Lookup.LookupResult;
+import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.IOUtils;
@@ -1334,4 +1337,112 @@ public class AnalyzingInfixSuggesterTest extends LuceneTestCase {
suggester.close();
}
+
+ public void testCloseIndexWriterOnBuild() throws Exception {
+ class MyAnalyzingInfixSuggester extends AnalyzingInfixSuggester {
+ public MyAnalyzingInfixSuggester(Directory dir, Analyzer indexAnalyzer, Analyzer queryAnalyzer,
+ int minPrefixChars, boolean commitOnBuild, boolean allTermsRequired,
+ boolean highlight, boolean closeIndexWriterOnBuild) throws IOException {
+ super(dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild,
+ allTermsRequired, highlight, closeIndexWriterOnBuild);
+ }
+ public IndexWriter getIndexWriter() {
+ return writer;
+ }
+ public SearcherManager getSearcherManager() {
+ return searcherMgr;
+ }
+ }
+
+ // After build(), when closeIndexWriterOnBuild = true:
+ // * The IndexWriter should be null
+ // * The SearcherManager should be non-null
+ // * SearcherManager's IndexWriter reference should be closed
+ // (as evidenced by maybeRefreshBlocking() throwing AlreadyClosedException)
+ Analyzer a = new MockAnalyzer(random(), MockTokenizer.WHITESPACE, false);
+ MyAnalyzingInfixSuggester suggester = new MyAnalyzingInfixSuggester(newDirectory(), a, a, 3, false,
+ AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, true);
+ suggester.build(new InputArrayIterator(sharedInputs));
+ assertNull(suggester.getIndexWriter());
+ assertNotNull(suggester.getSearcherManager());
+ expectThrows(AlreadyClosedException.class, () -> suggester.getSearcherManager().maybeRefreshBlocking());
+
+ suggester.close();
+ a.close();
+ }
+
+ public void testCommitAfterBuild() throws Exception {
+ performOperationWithAllOptionCombinations(suggester -> {
+ suggester.build(new InputArrayIterator(sharedInputs));
+ suggester.commit();
+ });
+ }
+
+ public void testRefreshAfterBuild() throws Exception {
+ performOperationWithAllOptionCombinations(suggester -> {
+ suggester.build(new InputArrayIterator(sharedInputs));
+ suggester.refresh();
+ });
+ }
+
+ public void testDisallowCommitBeforeBuild() throws Exception {
+ performOperationWithAllOptionCombinations
+ (suggester -> expectThrows(IllegalStateException.class, suggester::commit));
+ }
+
+ public void testDisallowRefreshBeforeBuild() throws Exception {
+ performOperationWithAllOptionCombinations
+ (suggester -> expectThrows(IllegalStateException.class, suggester::refresh));
+ }
+
+ private Input sharedInputs[] = new Input[] {
+ new Input("lend me your ear", 8, new BytesRef("foobar")),
+ new Input("a penny saved is a penny earned", 10, new BytesRef("foobaz")),
+ };
+
+ private interface SuggesterOperation {
+ void operate(AnalyzingInfixSuggester suggester) throws Exception;
+ }
+
+ /**
+ * Perform the given operation on suggesters constructed with all combinations of options
+ * commitOnBuild and closeIndexWriterOnBuild, including defaults.
+ */
+ private void performOperationWithAllOptionCombinations(SuggesterOperation operation) throws Exception {
+ Analyzer a = new MockAnalyzer(random(), MockTokenizer.WHITESPACE, false);
+
+ AnalyzingInfixSuggester suggester = new AnalyzingInfixSuggester(newDirectory(), a);
+ operation.operate(suggester);
+ suggester.close();
+
+ suggester = new AnalyzingInfixSuggester(newDirectory(), a, a, 3, false);
+ operation.operate(suggester);
+ suggester.close();
+
+ suggester = new AnalyzingInfixSuggester(newDirectory(), a, a, 3, true);
+ operation.operate(suggester);
+ suggester.close();
+
+ suggester = new AnalyzingInfixSuggester(newDirectory(), a, a, 3, true,
+ AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, true);
+ operation.operate(suggester);
+ suggester.close();
+
+ suggester = new AnalyzingInfixSuggester(newDirectory(), a, a, 3, true,
+ AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, false);
+ operation.operate(suggester);
+ suggester.close();
+
+ suggester = new AnalyzingInfixSuggester(newDirectory(), a, a, 3, false,
+ AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, true);
+ operation.operate(suggester);
+ suggester.close();
+
+ suggester = new AnalyzingInfixSuggester(newDirectory(), a, a, 3, false,
+ AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, false);
+ operation.operate(suggester);
+ suggester.close();
+
+ a.close();
+ }
}