From 157b7ad281ae58a2560780a4405193c2e75fc625 Mon Sep 17 00:00:00 2001 From: John Niang Date: Thu, 29 Aug 2024 11:05:24 +0800 Subject: [PATCH] Fix the problem of LockObtainFailedException while performing a rolling update (#6543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug /area core /milestone 2.19.0 #### What this PR does / why we need it: This PR refactors LuceneSearchEngine to let IndexWriter and SearcherManager load lazily to prevent LockObtainFailedException from performing a rolling update. #### Which issue(s) this PR fixes: Fixes #6541 #### Special notes for your reviewer: 1. Use MySQL or PostgreSQL as database for Halo 2. Start an instance of Halo 3. Try to initialize Halo and search posts 4. Change the `server.port` and start another instance of Halo 5. Check the status of another instance #### Does this PR introduce a user-facing change? ```release-note 修复滚动更新时无法启动新的 Halo 实例的问题 ``` --- .../app/search/lucene/LuceneSearchEngine.java | 124 +++++++++++----- .../search/lucene/LuceneSearchEngineTest.java | 137 ++++++------------ 2 files changed, 129 insertions(+), 132 deletions(-) diff --git a/application/src/main/java/run/halo/app/search/lucene/LuceneSearchEngine.java b/application/src/main/java/run/halo/app/search/lucene/LuceneSearchEngine.java index b8e54fde7..2f4e2d62f 100644 --- a/application/src/main/java/run/halo/app/search/lucene/LuceneSearchEngine.java +++ b/application/src/main/java/run/halo/app/search/lucene/LuceneSearchEngine.java @@ -34,6 +34,7 @@ import org.apache.lucene.document.StoredField; import org.apache.lucene.document.StringField; import org.apache.lucene.document.TextField; import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.IndexNotFoundException; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; import org.apache.lucene.index.Term; @@ -60,6 +61,7 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.lang.NonNull; import org.springframework.util.StopWatch; import org.springframework.util.StringUtils; +import reactor.core.Exceptions; import run.halo.app.search.HaloDocument; import run.halo.app.search.SearchEngine; import run.halo.app.search.SearchOption; @@ -78,9 +80,7 @@ public class LuceneSearchEngine implements SearchEngine, InitializingBean, Dispo private Analyzer analyzer; - private IndexWriter indexWriter; - - private SearcherManager searcherManager; + private volatile SearcherManager searcherManager; private Directory directory; @@ -103,12 +103,15 @@ public class LuceneSearchEngine implements SearchEngine, InitializingBean, Dispo docs.add(doc); }); var deleteQuery = new TermInSetQuery("id", terms); - try { - this.indexWriter.updateDocuments(deleteQuery, docs); - this.searcherManager.maybeRefreshBlocking(); - this.indexWriter.commit(); + + var writerConfig = new IndexWriterConfig(this.analyzer) + .setOpenMode(CREATE_OR_APPEND); + try (var indexWriter = new IndexWriter(this.directory, writerConfig)) { + indexWriter.updateDocuments(deleteQuery, docs); } catch (IOException e) { - throw new RuntimeException(e); + throw Exceptions.propagate(e); + } finally { + this.refreshSearcherManager(); } } @@ -117,29 +120,43 @@ public class LuceneSearchEngine implements SearchEngine, InitializingBean, Dispo var terms = new LinkedList(); haloDocIds.forEach(haloDocId -> terms.add(new BytesRef(haloDocId))); var deleteQuery = new TermInSetQuery("id", terms); - try { - this.indexWriter.deleteDocuments(deleteQuery); - this.searcherManager.maybeRefreshBlocking(); - this.indexWriter.commit(); + var writerConfig = new IndexWriterConfig(this.analyzer) + .setOpenMode(CREATE_OR_APPEND); + try (var indexWriter = new IndexWriter(this.directory, writerConfig)) { + indexWriter.deleteDocuments(deleteQuery); } catch (IOException e) { - throw new RuntimeException(e); + throw Exceptions.propagate(e); + } finally { + this.refreshSearcherManager(); } } @Override public void deleteAll() { - try { - this.indexWriter.deleteAll(); - this.searcherManager.maybeRefreshBlocking(); - this.indexWriter.commit(); + var writerConfig = new IndexWriterConfig(this.analyzer) + .setOpenMode(CREATE_OR_APPEND); + try (var indexWriter = new IndexWriter(this.directory, writerConfig)) { + indexWriter.deleteAll(); } catch (IOException e) { - throw new RuntimeException(e); + throw Exceptions.propagate(e); + } finally { + this.refreshSearcherManager(); } } @Override public SearchResult search(SearchOption option) { IndexSearcher searcher = null; + var sm = obtainSearcherManager(); + if (sm.isEmpty()) { + // indicate the index is empty + var emptyResult = new SearchResult(); + emptyResult.setKeyword(option.getKeyword()); + emptyResult.setLimit(option.getLimit()); + emptyResult.setTotal(0L); + emptyResult.setHits(List.of()); + return emptyResult; + } try { searcher = searcherManager.acquire(); var queryParser = new StandardQueryParser(analyzer); @@ -270,28 +287,55 @@ public class LuceneSearchEngine implements SearchEngine, InitializingBean, Dispo @Override public void afterPropertiesSet() throws Exception { - try { - this.analyzer = CustomAnalyzer.builder() - .withTokenizer(StandardTokenizerFactory.class) - .addCharFilter(HTMLStripCharFilterFactory.NAME) - .addCharFilter(CJKWidthCharFilterFactory.NAME) - .addTokenFilter(LowerCaseFilterFactory.NAME) - .addTokenFilter(CJKWidthFilterFactory.NAME) - .addTokenFilter(CJKBigramFilterFactory.NAME) - .build(); - this.directory = FSDirectory.open(this.indexRootDir); - var writerConfig = new IndexWriterConfig(this.analyzer) - .setOpenMode(CREATE_OR_APPEND); - this.indexWriter = new IndexWriter(this.directory, writerConfig); - this.searcherManager = new SearcherManager(this.indexWriter, null); - log.info("Initialized lucene search engine"); - } catch (IOException e) { - throw new RuntimeException(e); + this.analyzer = CustomAnalyzer.builder() + .withTokenizer(StandardTokenizerFactory.class) + .addCharFilter(HTMLStripCharFilterFactory.NAME) + .addCharFilter(CJKWidthCharFilterFactory.NAME) + .addTokenFilter(LowerCaseFilterFactory.NAME) + .addTokenFilter(CJKWidthFilterFactory.NAME) + .addTokenFilter(CJKBigramFilterFactory.NAME) + .build(); + this.directory = FSDirectory.open(this.indexRootDir); + log.info("Initialized lucene search engine"); + } + + Optional obtainSearcherManager() { + if (this.searcherManager != null) { + return Optional.of(this.searcherManager); + } + synchronized (this) { + // double check + if (this.searcherManager != null) { + return Optional.of(this.searcherManager); + } + try { + this.searcherManager = new SearcherManager(this.directory, null); + return Optional.of(this.searcherManager); + } catch (IndexNotFoundException e) { + log.warn("Index not ready for creating searcher manager"); + } catch (IOException e) { + log.error("Failed to create searcher manager", e); + } + return Optional.empty(); } } - void setIndexWriter(IndexWriter indexWriter) { - this.indexWriter = indexWriter; + private void refreshSearcherManager() { + this.obtainSearcherManager().ifPresent(sm -> { + try { + sm.maybeRefreshBlocking(); + } catch (IOException e) { + log.warn("Failed to refresh searcher", e); + } + }); + } + + Directory getDirectory() { + return directory; + } + + Analyzer getAnalyzer() { + return analyzer; } void setDirectory(Directory directory) { @@ -323,13 +367,13 @@ public class LuceneSearchEngine implements SearchEngine, InitializingBean, Dispo if (this.searcherManager != null) { closers.add(this.searcherManager); } - if (this.indexWriter != null) { - closers.add(this.indexWriter); - } if (this.directory != null) { closers.add(this.directory); } IOUtils.close(closers); + this.analyzer = null; + this.searcherManager = null; + this.directory = null; log.info("Destroyed lucene search engine"); } diff --git a/application/src/test/java/run/halo/app/search/lucene/LuceneSearchEngineTest.java b/application/src/test/java/run/halo/app/search/lucene/LuceneSearchEngineTest.java index 4e73d00dc..a06f0026b 100644 --- a/application/src/test/java/run/halo/app/search/lucene/LuceneSearchEngineTest.java +++ b/application/src/test/java/run/halo/app/search/lucene/LuceneSearchEngineTest.java @@ -1,41 +1,21 @@ package run.halo.app.search.lucene; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.assertArg; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import java.io.IOException; import java.nio.file.Path; import java.time.Instant; import java.util.List; import java.util.Map; -import org.apache.lucene.analysis.Analyzer; -import org.apache.lucene.analysis.standard.StandardAnalyzer; -import org.apache.lucene.document.Document; -import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.index.StoredFields; -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.TopFieldDocs; -import org.apache.lucene.search.TotalHits; -import org.apache.lucene.store.Directory; -import org.assertj.core.util.Streams; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.store.AlreadyClosedException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; -import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import run.halo.app.search.HaloDocument; import run.halo.app.search.SearchOption; @@ -43,18 +23,6 @@ import run.halo.app.search.SearchOption; @ExtendWith(MockitoExtension.class) class LuceneSearchEngineTest { - @Mock - IndexWriter indexWriter; - - @Mock - SearcherManager searcherManager; - - @Mock - Directory directory; - - @Mock - Analyzer analyzer; - LuceneSearchEngine searchEngine; @TempDir @@ -62,91 +30,76 @@ class LuceneSearchEngineTest { @BeforeEach void setUp() throws Exception { - var searchEngine = new LuceneSearchEngine(tempDir); - searchEngine.setIndexWriter(indexWriter); - searchEngine.setDirectory(directory); - searchEngine.setSearcherManager(searcherManager); - searchEngine.setAnalyzer(analyzer); - this.searchEngine = searchEngine; + this.searchEngine = new LuceneSearchEngine(tempDir); + this.searchEngine.afterPropertiesSet(); } + @AfterEach + void cleanUp() throws Exception { + this.searchEngine.destroy(); + } @Test void shouldAddOrUpdateDocument() throws IOException { var haloDoc = createFakeHaloDoc(); searchEngine.addOrUpdate(List.of(haloDoc)); - verify(this.indexWriter).updateDocuments(any(Query.class), assertArg(docs -> { - var docList = Streams.stream(docs).toList(); - assertEquals(1, docList.size()); - var doc = docList.get(0); - assertInstanceOf(Document.class, doc); - var document = (Document) doc; - assertEquals("fake-id", document.get("id")); - })); - verify(this.searcherManager).maybeRefreshBlocking(); - verify(this.indexWriter).commit(); + // validate the index + var reader = DirectoryReader.open(searchEngine.getDirectory()); + assertEquals(1, reader.getDocCount("id")); } @Test void shouldDeleteDocument() throws IOException { + var haloDoc = createFakeHaloDoc(); + searchEngine.addOrUpdate(List.of(haloDoc)); + + var reader = DirectoryReader.open(searchEngine.getDirectory()); + assertEquals(1, reader.getDocCount("id")); + this.searchEngine.deleteDocument(List.of("fake-id")); - verify(this.indexWriter).deleteDocuments(any(Query.class)); - verify(this.searcherManager).maybeRefreshBlocking(); - verify(this.indexWriter).commit(); + + reader = DirectoryReader.open(searchEngine.getDirectory()); + assertEquals(0, reader.getDocCount("id")); } @Test void shouldDeleteAll() throws IOException { + var haloDoc = createFakeHaloDoc(); + searchEngine.addOrUpdate(List.of(haloDoc)); + + var reader = DirectoryReader.open(searchEngine.getDirectory()); + assertEquals(1, reader.getDocCount("id")); + this.searchEngine.deleteAll(); - verify(this.indexWriter).deleteAll(); - verify(this.searcherManager).maybeRefreshBlocking(); - verify(this.indexWriter).commit(); + reader = DirectoryReader.open(searchEngine.getDirectory()); + assertEquals(0, reader.getDocCount("id")); } @Test void shouldDestroy() throws Exception { + var directory = this.searchEngine.getDirectory(); this.searchEngine.destroy(); - verify(this.analyzer).close(); - verify(this.searcherManager).close(); - verify(this.indexWriter).close(); - verify(this.directory).close(); + assertThrows(AlreadyClosedException.class, () -> DirectoryReader.open(directory)); } @Test - void shouldAlwaysDestroyAllEvenErrorOccurred() throws Exception { - var analyzerCloseError = new IOException("analyzer close error"); - doThrow(analyzerCloseError).when(this.analyzer).close(); - - var directoryCloseError = new IOException("directory close error"); - doThrow(directoryCloseError).when(this.directory).close(); - var e = assertThrows(IOException.class, () -> this.searchEngine.destroy()); - assertEquals(analyzerCloseError, e); - assertEquals(directoryCloseError, e.getSuppressed()[0]); - verify(this.analyzer).close(); - verify(this.searcherManager).close(); - verify(this.indexWriter).close(); - verify(this.directory).close(); + void shouldSearchNothingIfIndexNotFound() { + var option = new SearchOption(); + option.setKeyword("fake"); + option.setLimit(123); + option.setHighlightPreTag(""); + option.setHighlightPostTag(""); + var result = this.searchEngine.search(option); + assertEquals(0, result.getTotal()); + assertEquals("fake", result.getKeyword()); + assertEquals(123, result.getLimit()); + assertEquals(0, result.getHits().size()); } @Test - void shouldSearch() throws IOException { - var searcher = mock(IndexSearcher.class); - when(this.searcherManager.acquire()).thenReturn(searcher); - this.searchEngine.setAnalyzer(new StandardAnalyzer()); - - var totalHits = new TotalHits(1234, TotalHits.Relation.EQUAL_TO); - var scoreDoc = new ScoreDoc(1, 1.0f); - - var topFieldDocs = new TopFieldDocs(totalHits, new ScoreDoc[] {scoreDoc}, null); - when(searcher.search(any(Query.class), eq(123), any(Sort.class))) - .thenReturn(topFieldDocs); - var storedFields = mock(StoredFields.class); - - var haloDoc = createFakeHaloDoc(); - var doc = this.searchEngine.getHaloDocumentConverter().convert(haloDoc); - when(storedFields.document(1)).thenReturn(doc); - when(searcher.storedFields()).thenReturn(storedFields); + void shouldSearch() { + this.searchEngine.addOrUpdate(List.of(createFakeHaloDoc())); var option = new SearchOption(); option.setKeyword("fake"); @@ -154,7 +107,7 @@ class LuceneSearchEngineTest { option.setHighlightPreTag(""); option.setHighlightPostTag(""); var result = this.searchEngine.search(option); - assertEquals(1234, result.getTotal()); + assertEquals(1, result.getTotal()); assertEquals("fake", result.getKeyword()); assertEquals(123, result.getLimit()); assertEquals(1, result.getHits().size());