Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions dumpsearch/src/util/dump/SearchIndex.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
import org.apache.lucene.util.IOUtils;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
Expand Down Expand Up @@ -109,6 +110,9 @@ public static <T> SearchIndexBuilder<T> with( @NonNull Dump<T> dump, @NonNull St
private FacetsConfig _facetsConfig;
private DirectoryTaxonomyReader _taxoReader;
private int _version;
private Directory _writerDir;
private Directory _taxoWriterDir;
private Directory _taxoReaderDir;

private SearchIndex( @NonNull Dump<E> dump, @NonNull FieldAccessor idFieldAccessor, @NonNull BiConsumer<Document, E> documentBuilder,
@NonNull IndexWriterConfig config, @NonNull QueryParser queryParser, @NonNull FacetsConfig facetsConfig, int version ) {
Expand All @@ -130,18 +134,9 @@ private SearchIndex( @NonNull Dump<E> dump, @NonNull FieldAccessor idFieldAccess
public void close() throws IOException {
commit();

if ( _writer != null ) {
_writer.close();
}
if ( _taxoWriter != null ) {
_taxoWriter.close();
}
if ( _searcherManager != null ) {
_searcherManager.close();
}
if ( _taxoReader != null ) {
_taxoReader.close();
}
// readers must be closed before the writers they were opened on, and the Directory instances
// must be closed last, once nothing still has open files on them
IOUtils.close(_searcherManager, _taxoReader, _writer, _taxoWriter, _writerDir, _taxoWriterDir, _taxoReaderDir);

super.close();
}
Expand Down Expand Up @@ -379,13 +374,13 @@ protected void initLookupMap() {
@Override
protected void initLookupOutputStream() {
try {
Directory facetDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath());
_taxoWriter = new DirectoryTaxonomyWriter(facetDir);
_taxoWriterDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath());
_taxoWriter = new DirectoryTaxonomyWriter(_taxoWriterDir);
_taxoWriter.commit();

Directory dir = FSDirectory.open(getLookupFile().toPath());
_writerDir = FSDirectory.open(getLookupFile().toPath());
_config.setOpenMode(OpenMode.CREATE_OR_APPEND);
_writer = new IndexWriter(dir, _config);
_writer = new IndexWriter(_writerDir, _config);
_writer.commit();

}
Expand All @@ -408,8 +403,8 @@ protected void openSearcher() {
}

try {
Directory facetDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath());
_taxoReader = new DirectoryTaxonomyReader(facetDir);
_taxoReaderDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath());
_taxoReader = new DirectoryTaxonomyReader(_taxoReaderDir);
}
catch ( IOException e ) {
throw new RuntimeException("Failed to initialize dump facet index with lookup file " + getLookupFile() + "-facets", e);
Expand Down
28 changes: 28 additions & 0 deletions dumpsearch/test/util/dump/SearchIndexTest.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package util.dump;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static util.dump.SearchIndex.SortBuilder.Direction.ASC;
import static util.dump.SearchIndex.SortBuilder.Direction.DESC;
import static util.dump.SearchIndex.sort;
import static util.dump.SearchIndex.with;

import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
Expand All @@ -18,6 +20,8 @@
import org.apache.lucene.document.TextField;
import org.apache.lucene.facet.FacetField;
import org.apache.lucene.facet.FacetResult;
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.BytesRef;
import org.junit.After;
import org.junit.Before;
Expand Down Expand Up @@ -165,6 +169,30 @@ public void testFaceting() throws Exception {
}
}

/**
* Regression test for the leaked {@link Directory} handles that used to keep the Lucene index directory locked
* and writable after {@link SearchIndex#close()} returned, which surfaced as flaky {@code DirectoryNotEmptyException}s
* in callers deleting the underlying folder right after close (e.g. JUnit 5's {@code @TempDir}).
*/
@Test
public void testCloseReleasesUnderlyingLuceneDirectories() throws Exception {
File dumpFile = new File(_tmpdir, DUMP_FILENAME);
SearchIndex<Bean> index;

try (Dump<Bean> dump = new Dump<>(Bean.class, dumpFile)) {
index = with(dump, "_idLong", ( doc, o ) -> doc.add(new FacetField("facetField", o._data))).build();
dump.add(new Bean(1, "row"));
} // closes dump, which cascades to index.close()

for ( String fieldName : new String[]{ "_writerDir", "_taxoWriterDir", "_taxoReaderDir" } ) {
Field field = SearchIndex.class.getDeclaredField(fieldName);
field.setAccessible(true);
Directory dir = (Directory)field.get(index);
assertThat(dir).as(fieldName + " was never opened").isNotNull();
assertThatThrownBy(dir::listAll).as(fieldName + " was not closed by SearchIndex.close()").isInstanceOf(AlreadyClosedException.class);
}
}

@Test
public void testVersion() throws Exception {
File dumpFile = new File(_tmpdir, DUMP_FILENAME);
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.15.0</version>
<configuration>
<encoding>UTF-8</encoding>
<source>17</source>
<target>17</target>
<release>17</release>
</configuration>
</plugin>
<plugin>
Expand Down
Loading