diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 8d1eb2106f8..746901262cb 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -108,6 +108,9 @@ Bug Fixes * LUCENE-7630: Fix (Edge)NGramTokenFilter to no longer drop payloads and preserve all attributes. (Nathan Gass via Uwe Schindler) +* LUCENE-7670: AnalyzingInfixSuggester should not immediately open an + IndexWriter over an already-built index. (Steve Rowe) + Improvements * LUCENE-7055: Added Weight#scorerSupplier, which allows to estimate the cost 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 81880d4e913..0ca81c75667 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 @@ -249,9 +249,7 @@ public class AnalyzingInfixSuggester extends Lookup implements Closeable { if (DirectoryReader.indexExists(dir)) { // Already built; open it: - writer = new IndexWriter(dir, - getIndexWriterConfig(getGramAnalyzer(), IndexWriterConfig.OpenMode.APPEND)); - searcherMgr = new SearcherManager(writer, null); + searcherMgr = new SearcherManager(dir, null); } } 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 fc5e2b7bd96..478358b5c92 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 @@ -1360,7 +1360,8 @@ public class AnalyzingInfixSuggesterTest extends LuceneTestCase { // * 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, + Path tempDir = createTempDir("analyzingInfixContext"); + final MyAnalyzingInfixSuggester suggester = new MyAnalyzingInfixSuggester(newFSDirectory(tempDir), a, a, 3, false, AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, true); suggester.build(new InputArrayIterator(sharedInputs)); assertNull(suggester.getIndexWriter()); @@ -1368,6 +1369,16 @@ public class AnalyzingInfixSuggesterTest extends LuceneTestCase { expectThrows(AlreadyClosedException.class, () -> suggester.getSearcherManager().maybeRefreshBlocking()); suggester.close(); + + // After instantiating from an already-built suggester dir: + // * The IndexWriter should be null + // * The SearcherManager should be non-null + final MyAnalyzingInfixSuggester suggester2 = new MyAnalyzingInfixSuggester(newFSDirectory(tempDir), a, a, 3, false, + AnalyzingInfixSuggester.DEFAULT_ALL_TERMS_REQUIRED, AnalyzingInfixSuggester.DEFAULT_HIGHLIGHT, true); + assertNull(suggester2.getIndexWriter()); + assertNotNull(suggester2.getSearcherManager()); + + suggester2.close(); a.close(); } diff --git a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java index 34cd30641a2..385b9c57250 100644 --- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java +++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java @@ -42,12 +42,20 @@ import org.apache.solr.update.AddUpdateCommand; import org.apache.solr.update.CommitUpdateCommand; import org.apache.solr.update.UpdateHandler; import org.apache.solr.util.ReadOnlyCoresLocator; +import org.junit.BeforeClass; import org.junit.Test; public class TestLazyCores extends SolrTestCaseJ4 { private File solrHomeDirectory; + @BeforeClass + public static void setupClass() throws Exception { + // Need to use a disk-based directory because there are tests that close a core after adding documents + // then expect to be able to re-open that core and execute a search + useFactory("solr.StandardDirectoryFactory"); + } + private static CoreDescriptor makeCoreDescriptor(CoreContainer cc, String coreName, String isTransient, String loadOnStartup) { return new CoreDescriptor(cc, coreName, cc.getCoreRootDirectory().resolve(coreName), CoreDescriptor.CORE_TRANSIENT, isTransient, @@ -721,4 +729,71 @@ public class TestLazyCores extends SolrTestCaseJ4 { } } + // Insure that when a core is aged out of the transient cache, any uncommitted docs are preserved. + // Note, this needs FS-based indexes to persist! + // Cores 2, 3, 6, 7, 8, 9 are transient + @Test + public void testNoCommit() throws Exception { + CoreContainer cc = init(); + String[] coreList = new String[]{ + "collection2", + "collection3", + "collection6", + "collection7", + "collection8", + "collection9" + }; + try { + // First, go through all the transient cores and add some docs. DO NOT COMMIT! + // The aged-out core should commit the docs when it gets closed. + List openCores = new ArrayList<>(); + for (String coreName : coreList) { + SolrCore core = cc.getCore(coreName); + openCores.add(core); + add10(core); + } + + // Just proving that some cores have been aged out. + checkNotInCores(cc, "collection2", "collection3"); + + // Close our get of all cores above. + for (SolrCore core : openCores) core.close(); + openCores.clear(); + + // We still should have 6, 7, 8, 9 loaded, their reference counts have NOT dropped to zero + checkInCores(cc, "collection6", "collection7", "collection8", "collection9"); + + for (String coreName : coreList) { + // The point of this test is to insure that when cores are aged out and re-opened + // that the docs are there, so insure that the core we're testing is gone, gone, gone. + checkNotInCores(cc, coreName); + + // Load the core up again. + SolrCore core = cc.getCore(coreName); + openCores.add(core); + + // Insure docs are still there. + check10(core); + } + for (SolrCore core : openCores) core.close(); + } finally { + cc.shutdown(); + } + } + + private void add10(SolrCore core) throws IOException { + for (int idx = 0; idx < 10; ++idx) { + addLazy(core, "id", "0" + idx); + } + SolrQueryRequest req = makeReq(core); + + } + + private void check10(SolrCore core) { + // Just get a couple of searches to work! + assertQ("test closing core without committing", + makeReq(core, "q", "*:*") + , "//result[@numFound='10']" + ); + } }