Skip to content

Commit

Permalink
Fix the problem of LockObtainFailedException while performing a rolli…
Browse files Browse the repository at this point in the history 
…ng update (halo-dev#6543)

#### 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 halo-dev#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 实例的问题
```
  • Loading branch information
@JohnNiang
JohnNiang authored Aug 29, 2024
1 parent f61f846 commit 157b7ad
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 132 deletions.
124 changes: 84 additions & 40 deletions application/src/main/java/run/halo/app/search/lucene/LuceneSearchEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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;
Expand All @@ -60,6 +61,7 @@
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;
Expand All @@ -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;

Expand All @@ -103,12 +103,15 @@ public void addOrUpdate(Iterable<HaloDocument> haloDocs) {
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();
}
}

Expand All @@ -117,29 +120,43 @@ public void deleteDocument(Iterable<String> haloDocIds) {
var terms = new LinkedList<BytesRef>();
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);
Expand Down Expand Up @@ -270,28 +287,55 @@ public SearchResult search(SearchOption option) {

@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<SearcherManager> 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) {
Expand Down Expand Up @@ -323,13 +367,13 @@ public void destroy() throws Exception {
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");
}

Expand Down
137 changes: 45 additions & 92 deletions application/src/test/java/run/halo/app/search/lucene/LuceneSearchEngineTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,160 +1,113 @@
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;

@ExtendWith(MockitoExtension.class)
class LuceneSearchEngineTest {

@Mock
IndexWriter indexWriter;

@Mock
SearcherManager searcherManager;

@Mock
Directory directory;

@Mock
Analyzer analyzer;

LuceneSearchEngine searchEngine;

@TempDir
Path tempDir;

@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("<fake-tag>");
option.setHighlightPostTag("</fake-tag>");
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");
option.setLimit(123);
option.setHighlightPreTag("<fake-tag>");
option.setHighlightPostTag("</fake-tag>");
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());
Expand Down

0 comments on commit 157b7ad

Please sign in to comment.