Skip to content

Commit 80d5e7c

Browse files
authored
Remove synchronized blocks from FDBDirectoryWrapper (#3494)
ForkJoinPool doesn't play well with `synchronized` blocks, particularly, it can result in thread starvation / deadlocks. Threads waiting on `synchronized` blocks, still count as active, and thus count towards the parallelism of the pool. The tests here run in a very small pool, to make sure that this won't happen, but in actual production workloads, one would expect a much larger pool, but also a lot more concurrent activity. These tests were failing pretty reliably before with a pool size of 3, and now they seem to pass reliably. One change in here that may be surprising is the change around the `writerReader`. I looked at the usages, and there were two: 1. Cursors would call `getReader` which would always flush, and thus never use the field 2. `LuceneIndexMaintainer.deleteDocument` would never flush So it makes sense to have these be two independent calls, which allows `getWriterReader` to use a `LazyCloseable`. Resolves: #2989 Resolves: #3146
1 parent 618d881 commit 80d5e7c

File tree

8 files changed

+165
-115
lines changed

8 files changed

+165
-115
lines changed

fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ int deleteDocument(Tuple groupingKey, Integer partitionId, Tuple primaryKey) thr
299299
@Nullable final LucenePrimaryKeySegmentIndex segmentIndex = directoryManager.getDirectory(groupingKey, partitionId).getPrimaryKeySegmentIndex();
300300

301301
if (segmentIndex != null) {
302-
final DirectoryReader directoryReader = directoryManager.getDirectoryReader(groupingKey, partitionId);
302+
final DirectoryReader directoryReader = directoryManager.getWriterReader(groupingKey, partitionId);
303303
final LucenePrimaryKeySegmentIndex.DocumentIndexEntry documentIndexEntry = segmentIndex.findDocument(directoryReader, primaryKey);
304304
if (documentIndexEntry != null) {
305305
state.context.ensureActive().clear(documentIndexEntry.entryKey); // TODO: Only if valid?
@@ -702,14 +702,15 @@ private CompletableFuture<IndexOperationResult> getLuceneInfoForAllPartitions(fi
702702
@SuppressWarnings("PMD.CloseResource") // the indexReader and directory are closed by the directoryManager
703703
private CompletableFuture<LuceneMetadataInfo.LuceneInfo> getLuceneInfo(final Tuple groupingKey, final Integer partitionId) {
704704
try {
705-
final IndexReader indexReader = directoryManager.getIndexReader(groupingKey, partitionId);
706-
final FDBDirectory directory = getDirectory(groupingKey, partitionId);
707-
final CompletableFuture<Integer> fieldInfosFuture = directory.getFieldInfosCount();
708-
return directory.listAllAsync()
709-
.thenCombine(fieldInfosFuture, (fileList, fieldInfosCount) ->
710-
new LuceneMetadataInfo.LuceneInfo(
711-
indexReader.numDocs(),
712-
fileList, fieldInfosCount));
705+
try (IndexReader indexReader = directoryManager.getIndexReader(groupingKey, partitionId)) {
706+
final FDBDirectory directory = getDirectory(groupingKey, partitionId);
707+
final CompletableFuture<Integer> fieldInfosFuture = directory.getFieldInfosCount();
708+
return directory.listAllAsync()
709+
.thenCombine(fieldInfosFuture, (fileList, fieldInfosCount) ->
710+
new LuceneMetadataInfo.LuceneInfo(
711+
indexReader.numDocs(),
712+
fileList, fieldInfosCount));
713+
}
713714
} catch (IOException e) {
714715
return CompletableFuture.failedFuture(e);
715716
}

fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexScrubbingToolsMissing.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ private boolean isMissingIndexKey(FDBIndexableRecord<Message> rec, Integer parti
198198
throw LuceneExceptions.toRecordCoreException("failed getIndexWriter", e);
199199
}
200200
try {
201-
DirectoryReader directoryReader = directoryManager.getDirectoryReader(groupingKey, partitionId);
201+
DirectoryReader directoryReader = directoryManager.getWriterReader(groupingKey, partitionId);
202202
final LucenePrimaryKeySegmentIndex.DocumentIndexEntry documentIndexEntry = segmentIndex.findDocument(directoryReader, rec.getPrimaryKey());
203203
if (documentIndexEntry == null) {
204204
// Here: the document had not been found in the PK segment index

fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,22 +84,18 @@ public class FDBDirectoryManager implements AutoCloseable {
8484
@Nonnull
8585
private final Map<Tuple, FDBDirectoryWrapper> createdDirectories;
8686
private final int mergeDirectoryCount;
87-
@Nullable
88-
private final Exception exceptionAtCreation;
8987
@Nonnull
9088
protected final LuceneAnalyzerWrapper writerAnalyzer;
9189
@Nonnull
9290
private final LuceneAnalyzerCombinationProvider analyzerSelector;
91+
@Nullable
92+
protected final Exception exceptionAtCreation;
9393

9494
protected FDBDirectoryManager(@Nonnull IndexMaintainerState state) {
9595
this.state = state;
9696
this.createdDirectories = new ConcurrentHashMap<>();
9797
this.mergeDirectoryCount = getMergeDirectoryCount(state);
98-
if (FDBTieredMergePolicy.usesCreationStack()) {
99-
this.exceptionAtCreation = new Exception();
100-
} else {
101-
this.exceptionAtCreation = null;
102-
}
98+
this.exceptionAtCreation = FDBTieredMergePolicy.usesCreationStack() ? new Exception() : null;
10399
final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(state.index, state.store.getRecordMetaData());
104100
this.analyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos);
105101
this.writerAnalyzer = analyzerSelector.provideIndexAnalyzer();
@@ -206,7 +202,7 @@ public void mergeIndexWithContext(@Nonnull final Tuple groupingKey,
206202
@Nonnull final AgilityContext agilityContext) {
207203
try (FDBDirectoryWrapper directoryWrapper = createDirectoryWrapper(groupingKey, partitionId, agilityContext)) {
208204
try {
209-
directoryWrapper.mergeIndex(exceptionAtCreation);
205+
directoryWrapper.mergeIndex();
210206
if (LOGGER.isDebugEnabled()) {
211207
LOGGER.debug(KeyValueLogMessage.of("Lucene merge success",
212208
LuceneLogMessageKeys.GROUP, groupingKey,
@@ -312,7 +308,8 @@ private FDBDirectoryWrapper createDirectoryWrapper(@Nullable Tuple groupingKey,
312308
}
313309

314310
protected @Nonnull FDBDirectoryWrapper createNewDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize) {
315-
return new FDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, writerAnalyzer);
311+
return new FDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize,
312+
writerAnalyzer, exceptionAtCreation);
316313
}
317314

318315
private int getBlockCacheMaximumSize() {
@@ -367,11 +364,11 @@ public IndexReader getIndexReader(@Nullable Tuple groupingKey, @Nullable Integer
367364

368365
@Nonnull
369366
public IndexWriter getIndexWriter(@Nullable Tuple groupingKey, @Nullable Integer partitionId) throws IOException {
370-
return getDirectoryWrapper(groupingKey, partitionId).getWriter(exceptionAtCreation);
367+
return getDirectoryWrapper(groupingKey, partitionId).getWriter();
371368
}
372369

373-
public DirectoryReader getDirectoryReader(@Nullable Tuple groupingKey, @Nullable Integer partititonId) throws IOException {
374-
return getDirectoryWrapper(groupingKey, partititonId).getWriterReader(false);
370+
public DirectoryReader getWriterReader(@Nullable Tuple groupingKey, @Nullable Integer partititonId) throws IOException {
371+
return getDirectoryWrapper(groupingKey, partititonId).getWriterReader();
375372
}
376373

377374
@Nonnull

0 commit comments

Comments
 (0)