From db269b095602149f19671705da1ede5ae1147a84 Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 10:38:29 -0500 Subject: [PATCH 1/9] Add a test of concurrent updates Setting up the executor as a ForkJoinPool and trying to do concurrent updates can cause a deadlock. --- .../lucene/LuceneIndexMaintenanceTest.java | 72 +++++++++++++++++++ .../lucene/LuceneIndexTestDataModel.java | 51 +++++++++++-- 2 files changed, 117 insertions(+), 6 deletions(-) diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java index c3090660b9..490c3f33c9 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java @@ -75,6 +75,7 @@ import java.io.IOException; import java.time.Instant; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; import java.util.IdentityHashMap; @@ -86,6 +87,8 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.ForkJoinWorkerThread; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -781,6 +784,75 @@ public void ensureValid() throws IOException { } } + @Test + void concurrentUpdate() throws IOException { + AtomicInteger threadCounter = new AtomicInteger(); + this.dbExtension.getDatabaseFactory().setExecutor(new ForkJoinPool(3, + pool -> { + final ForkJoinWorkerThread thread = ForkJoinPool.defaultForkJoinWorkerThreadFactory.newThread(pool); + thread.setName("ConcurrentUpdatePool-" + threadCounter.getAndIncrement()); + return thread; + }, + null, false)); + final long seed = 320947L; + final boolean isGrouped = true; + final boolean isSynthetic = true; + final boolean primaryKeySegmentIndexEnabled = true; + final int partitionHighWatermark = 100_000; + final LuceneIndexTestDataModel dataModel = new LuceneIndexTestDataModel.Builder(seed, this::getStoreBuilder, pathManager) + .setIsGrouped(isGrouped) + .setIsSynthetic(isSynthetic) + .setPrimaryKeySegmentIndexEnabled(primaryKeySegmentIndexEnabled) + .setPartitionHighWatermark(partitionHighWatermark) + .build(); + + final int repartitionCount = 10; + final int loopCount = 30; + + final RecordLayerPropertyStorage contextProps = RecordLayerPropertyStorage.newBuilder() + .addProp(LuceneRecordContextProperties.LUCENE_REPARTITION_DOCUMENT_COUNT, repartitionCount) + .addProp(LuceneRecordContextProperties.LUCENE_MAX_DOCUMENTS_TO_MOVE_DURING_REPARTITIONING, dataModel.nextInt(1000) + repartitionCount) + .addProp(LuceneRecordContextProperties.LUCENE_MERGE_SEGMENTS_PER_TIER, (double)dataModel.nextInt(10) + 2) // it must be at least 2.0 + .build(); + for (int i = 0; i < loopCount; i++) { + LOGGER.info(KeyValueLogMessage.of("concurrentUpdate loop", + "iteration", i, + "groupCount", dataModel.groupingKeyToPrimaryKeyToPartitionKey.size(), + "docCount", dataModel.groupingKeyToPrimaryKeyToPartitionKey.values().stream().mapToInt(Map::size).sum(), + "docMinPerGroup", dataModel.groupingKeyToPrimaryKeyToPartitionKey.values().stream().mapToInt(Map::size).min(), + "docMaxPerGroup", dataModel.groupingKeyToPrimaryKeyToPartitionKey.values().stream().mapToInt(Map::size).max())); + + long start = 234098; + try (FDBRecordContext context = openContext(contextProps)) { + dataModel.saveRecords(10, start, context, 1); + commit(context); + } + explicitMergeIndex(dataModel.index, contextProps, dataModel.schemaSetup); + } + + + try (FDBRecordContext context = openContext(contextProps)) { + FDBRecordStore recordStore = Objects.requireNonNull(dataModel.schemaSetup.apply(context)); + recordStore.getIndexDeferredMaintenanceControl().setAutoMergeDuringCommit(false); + assertThat(dataModel.updateableRecords, Matchers.aMapWithSize(Matchers.greaterThan(30))); + LOGGER.info("concurrentUpdate: Starting updates"); + RecordCursor.fromList(new ArrayList<>(dataModel.updateableRecords.entrySet())) + .mapPipelined(entry -> { + return dataModel.updateRecord(recordStore, entry.getKey(), entry.getValue()); + }, 10) + .asList().join(); + commit(context); + } + + + final LuceneIndexTestValidator luceneIndexTestValidator = new LuceneIndexTestValidator(() -> openContext(contextProps), context -> Objects.requireNonNull(dataModel.schemaSetup.apply(context))); + luceneIndexTestValidator.validate(dataModel.index, dataModel.groupingKeyToPrimaryKeyToPartitionKey, isSynthetic ? "child_str_value:forth" : "text_value:about"); + + if (isGrouped) { + validateDeleteWhere(isSynthetic, dataModel.groupingKeyToPrimaryKeyToPartitionKey, contextProps, dataModel.schemaSetup, dataModel.index); + } + } + static Stream concurrentStoreTest() { return Stream.concat( Stream.of( diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestDataModel.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestDataModel.java index 58e12ba3cb..8724c8f913 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestDataModel.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestDataModel.java @@ -34,6 +34,7 @@ import com.apple.foundationdb.record.test.TestKeySpace; import com.apple.foundationdb.record.test.TestKeySpacePathManagerExtension; import com.apple.foundationdb.tuple.Tuple; +import com.google.protobuf.Message; import javax.annotation.Nonnull; import java.util.HashMap; @@ -41,6 +42,7 @@ import java.util.Objects; import java.util.Random; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.function.Function; /** @@ -64,6 +66,7 @@ public class LuceneIndexTestDataModel { *

*/ final Map> groupingKeyToPrimaryKeyToPartitionKey; + Map> updateableRecords; private LuceneIndexTestDataModel(final Builder builder) { random = builder.random; @@ -92,6 +95,7 @@ private LuceneIndexTestDataModel(final Builder builder) { return store; }; groupingKeyToPrimaryKeyToPartitionKey = new HashMap<>(); + updateableRecords = new HashMap<>(); } @Override @@ -121,7 +125,7 @@ void saveRecords(int count, long start, FDBRecordContext context, final int grou FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); for (int j = 0; j < count; j++) { LuceneIndexTestDataModel.saveRecord(isGrouped, isSynthetic, random, groupingKeyToPrimaryKeyToPartitionKey, - textGenerator, start, recordStore, group); + textGenerator, start, recordStore, group, updateableRecords); } } @@ -129,10 +133,17 @@ static void saveRecord(final boolean isGrouped, final boolean isSynthetic, final final Map> groupingKeyToPrimaryKeyToPartitionKey, final RandomTextGenerator textGenerator, final long start, final FDBRecordStore recordStore, final int group) { + saveRecord(isGrouped, isSynthetic, random, groupingKeyToPrimaryKeyToPartitionKey, textGenerator, start, recordStore, group, new HashMap<>()); + } + + static void saveRecord(final boolean isGrouped, final boolean isSynthetic, final Random random, + final Map> groupingKeyToPrimaryKeyToPartitionKey, + final RandomTextGenerator textGenerator, final long start, final FDBRecordStore recordStore, + final int group, final Map> updateableRecords) { final Tuple groupTuple = isGrouped ? Tuple.from(group) : Tuple.from(); final int countInGroup = groupingKeyToPrimaryKeyToPartitionKey.computeIfAbsent(groupTuple, key -> new HashMap<>()).size(); long timestamp = start + countInGroup + random.nextInt(20) - 5; - final Tuple primaryKey = saveRecord(recordStore, isSynthetic, group, countInGroup, timestamp, textGenerator, random); + final Tuple primaryKey = saveRecord(recordStore, isSynthetic, group, countInGroup, timestamp, textGenerator, random, updateableRecords); groupingKeyToPrimaryKeyToPartitionKey.computeIfAbsent(groupTuple, key -> new HashMap<>()) .put(primaryKey, Tuple.from(timestamp).addAll(primaryKey)); } @@ -144,7 +155,8 @@ static Tuple saveRecord(final FDBRecordStore recordStore, final int countInGroup, final long timestamp, final RandomTextGenerator textGenerator, - final Random random) { + final Random random, + final Map> updateableRecords) { var parent = TestRecordsGroupedParentChildProto.MyParentRecord.newBuilder() .setGroup(group) .setRecNo(1001L + countInGroup) @@ -155,6 +167,8 @@ static Tuple saveRecord(final FDBRecordStore recordStore, .setChildRecNo(1000L - countInGroup) .build(); Tuple primaryKey; + final Tuple parentPrimaryKey = recordStore.saveRecord(parent).getPrimaryKey(); + updateableRecords.put(parentPrimaryKey, existingRecord -> updateParentRecord(existingRecord, random)); if (isSynthetic) { var child = TestRecordsGroupedParentChildProto.MyChildRecord.newBuilder() .setGroup(group) @@ -165,15 +179,40 @@ static Tuple saveRecord(final FDBRecordStore recordStore, final Tuple syntheticRecordTypeKey = recordStore.getRecordMetaData() .getSyntheticRecordType("JoinChildren") .getRecordTypeKeyTuple(); + final Tuple childPrimaryKey = recordStore.saveRecord(child).getPrimaryKey(); + updateableRecords.put(childPrimaryKey, existingRecord -> updateChildRecord(existingRecord, random)); primaryKey = Tuple.from(syntheticRecordTypeKey.getItems().get(0), - recordStore.saveRecord(parent).getPrimaryKey().getItems(), - recordStore.saveRecord(child).getPrimaryKey().getItems()); + parentPrimaryKey.getItems(), + childPrimaryKey.getItems()); } else { - primaryKey = recordStore.saveRecord(parent).getPrimaryKey(); + primaryKey = parentPrimaryKey; } return primaryKey; } + + private static Message updateParentRecord(Message existingRecord, final Random random) { + final var builder = TestRecordsGroupedParentChildProto.MyParentRecord.newBuilder(); + builder.mergeFrom(existingRecord); + builder.setIntValue(random.nextInt()); + return builder.build(); + } + + private static Message updateChildRecord(final Message existingRecord, final Random random) { + final var builder = TestRecordsGroupedParentChildProto.MyChildRecord.newBuilder(); + builder.mergeFrom(existingRecord); + builder.setOtherValue(random.nextInt()); + return builder.build(); + } + + public CompletableFuture updateRecord(final FDBRecordStore recordStore, + Tuple primaryKey, + Function updateMessage) { + return recordStore.loadRecordAsync(primaryKey).thenAccept(existingRecord -> { + recordStore.saveRecord(updateMessage.apply(existingRecord.getRecord())); + }); + } + @Nonnull static Index addIndex(final boolean isSynthetic, final KeyExpression rootExpression, final Map options, final RecordMetaDataBuilder metaDataBuilder) { From 84fa7c9523487644d271a729df0ad10c2372bc1f Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 10:53:34 -0500 Subject: [PATCH 2/9] AnalyzerChooser no longer depends on text The only implementation that actually referenced the text was in tests. Those tests need to be adjusted, but this makes it so that the type aligns with the behavior, and hopefully gets us on a path to an index having a fixed analyzer (at least for the lifetime of the transaction), which can simplify the FDBDirectoryWrapper --- .../record/lucene/AnalyzerChooser.java | 11 ++--------- .../lucene/EmailCjkSynonymAnalyzerFactory.java | 4 ++-- .../LuceneAnalyzerCombinationProvider.java | 4 ++-- .../record/lucene/LuceneAnalyzerFactory.java | 2 +- .../lucene/LuceneAnalyzerRegistryImpl.java | 4 ++-- .../LuceneAutoCompleteAnalyzerFactory.java | 2 +- .../lucene/exact/ExactTokenAnalyzerFactory.java | 5 ++--- .../record/lucene/ngram/NgramAnalyzer.java | 2 +- .../record/lucene/synonym/SynonymAnalyzer.java | 6 +++--- .../lucene/LuceneDocumentFromRecordTest.java | 4 ++-- .../record/lucene/LuceneIndexTestUtils.java | 17 +++++++++-------- 11 files changed, 27 insertions(+), 34 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/AnalyzerChooser.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/AnalyzerChooser.java index 7ce9348e1c..7699b81eba 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/AnalyzerChooser.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/AnalyzerChooser.java @@ -23,18 +23,11 @@ import org.apache.lucene.analysis.Analyzer; import javax.annotation.Nonnull; -import java.util.Collections; -import java.util.List; /** - * Choose an {@link Analyzer} given texts. + * Choose an {@link Analyzer}. */ public interface AnalyzerChooser { @Nonnull - default LuceneAnalyzerWrapper chooseAnalyzer(@Nonnull String text) { - return chooseAnalyzer(Collections.singletonList(text)); - } - - @Nonnull - LuceneAnalyzerWrapper chooseAnalyzer(@Nonnull List texts); + LuceneAnalyzerWrapper chooseAnalyzer(); } diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/EmailCjkSynonymAnalyzerFactory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/EmailCjkSynonymAnalyzerFactory.java index e02dbad568..a6b7687b75 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/EmailCjkSynonymAnalyzerFactory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/EmailCjkSynonymAnalyzerFactory.java @@ -65,7 +65,7 @@ public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { final String minLengthString = Optional.ofNullable(index.getOption(IndexOptions.TEXT_TOKEN_MIN_SIZE)).orElse(DEFAULT_MINIMUM_TOKEN_LENGTH); final String maxLengthString = Optional.ofNullable(index.getOption(IndexOptions.TEXT_TOKEN_MAX_SIZE)).orElse(Integer.toString(UAX29URLEmailAnalyzer.DEFAULT_MAX_TOKEN_LENGTH)); - return t -> new LuceneAnalyzerWrapper(UNIQUE_IDENTIFIER, + return () -> new LuceneAnalyzerWrapper(UNIQUE_IDENTIFIER, new EmailCjkSynonymAnalyzer(MINIMAL_STOP_WORDS, 1, Integer.parseInt(minLengthString), Integer.parseInt(maxLengthString), true, false, null)); } catch (NumberFormatException ex) { @@ -81,7 +81,7 @@ public AnalyzerChooser getQueryAnalyzerChooser(@Nonnull Index index, @Nonnull An final String minLengthString = Optional.ofNullable(index.getOption(IndexOptions.TEXT_TOKEN_MIN_SIZE)).orElse(DEFAULT_MINIMUM_TOKEN_LENGTH); final String maxLengthString = Optional.ofNullable(index.getOption(IndexOptions.TEXT_TOKEN_MAX_SIZE)).orElse(DEFAULT_MAXIMUM_TOKEN_LENGTH); final String synonymConfigName = index.getOption(LuceneIndexOptions.TEXT_SYNONYM_SET_NAME_OPTION); - return t -> new LuceneAnalyzerWrapper(UNIQUE_IDENTIFIER, + return () -> new LuceneAnalyzerWrapper(UNIQUE_IDENTIFIER, new EmailCjkSynonymAnalyzer(MINIMAL_STOP_WORDS, 1, Integer.parseInt(minLengthString), Integer.parseInt(maxLengthString), true, synonymConfigName != null, synonymConfigName != null ? SynonymMapRegistryImpl.instance().getSynonymMap(synonymConfigName) : null)); } catch (NumberFormatException ex) { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java index 179e56bbe0..82f98800e0 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java @@ -73,11 +73,11 @@ public LuceneAnalyzerWrapper provideQueryAnalyzer(@Nonnull List texts) { private static LuceneAnalyzerWrapper buildAnalyzerWrapper(@Nonnull List texts, @Nonnull AnalyzerChooser defaultAnalyzerChooser, @Nullable Map customizedAnalyzerChooserPerField) { - final LuceneAnalyzerWrapper defaultAnalyzerWrapper = defaultAnalyzerChooser.chooseAnalyzer(texts); + final LuceneAnalyzerWrapper defaultAnalyzerWrapper = defaultAnalyzerChooser.chooseAnalyzer(); if (customizedAnalyzerChooserPerField != null) { // The order of keys matters because the identifier for each map needs to be consistent SortedMap analyzerWrapperMap = new TreeMap<>(customizedAnalyzerChooserPerField.entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().chooseAnalyzer(texts)))); + .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().chooseAnalyzer()))); PerFieldAnalyzerWrapper analyzerWrapper = new PerFieldAnalyzerWrapper(defaultAnalyzerWrapper.getAnalyzer(), analyzerWrapperMap.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getAnalyzer()))); diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerFactory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerFactory.java index bc47f0158c..b9054760e3 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerFactory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerFactory.java @@ -75,6 +75,6 @@ public interface LuceneAnalyzerFactory { */ @Nonnull default AnalyzerChooser getQueryAnalyzerChooser(@Nonnull Index index, @Nonnull AnalyzerChooser indexAnalyzerChooser) { - return t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(); + return () -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(); } } diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryImpl.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryImpl.java index a52d1cbbcc..d2fe956b71 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryImpl.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryImpl.java @@ -148,8 +148,8 @@ private static boolean isEligibleForNoOpAnalyzer(@Nonnull final LuceneIndexExpre private Pair getAnalyzerChooser(@Nonnull Index index, @Nullable String analyzerName, @Nonnull LuceneAnalyzerType type) { final Map registryForType = Objects.requireNonNullElse(registry.get(type), Collections.emptyMap()); if (analyzerName == null || !registryForType.containsKey(analyzerName)) { - return Pair.of(t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(), - t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper()); + return Pair.of(() -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(), + () -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper()); } else { LuceneAnalyzerFactory analyzerFactory = registryForType.get(analyzerName); if (analyzerFactory == null) { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteAnalyzerFactory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteAnalyzerFactory.java index 7972e14235..9526ba63c6 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteAnalyzerFactory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteAnalyzerFactory.java @@ -48,6 +48,6 @@ public LuceneAnalyzerType getType() { @Nonnull @Override public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { - return t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(); + return () -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(); } } diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/exact/ExactTokenAnalyzerFactory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/exact/ExactTokenAnalyzerFactory.java index 9fca9dfa6f..7f21a76bcb 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/exact/ExactTokenAnalyzerFactory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/exact/ExactTokenAnalyzerFactory.java @@ -28,7 +28,6 @@ import com.google.auto.service.AutoService; import javax.annotation.Nonnull; -import java.util.List; /** * Constructs a new instance of {@link ExactTokenAnalyzer}. @@ -59,7 +58,7 @@ public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index ignored) { return new AnalyzerChooser() { @Nonnull @Override - public LuceneAnalyzerWrapper chooseAnalyzer(@Nonnull List ignored) { + public LuceneAnalyzerWrapper chooseAnalyzer() { return new LuceneAnalyzerWrapper(UNIQUE_NAME, new ExactTokenAnalyzer()); } }; @@ -71,7 +70,7 @@ public AnalyzerChooser getQueryAnalyzerChooser(@Nonnull Index ignored, @Nonnull return new AnalyzerChooser() { @Nonnull @Override - public LuceneAnalyzerWrapper chooseAnalyzer(@Nonnull List ignored) { + public LuceneAnalyzerWrapper chooseAnalyzer() { return new LuceneAnalyzerWrapper(UNIQUE_NAME, new ExactTokenAnalyzer()); } }; diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/ngram/NgramAnalyzer.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/ngram/NgramAnalyzer.java index 49e981341d..fac5356a81 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/ngram/NgramAnalyzer.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/ngram/NgramAnalyzer.java @@ -108,7 +108,7 @@ public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { final String minLengthString = Optional.ofNullable(index.getOption(IndexOptions.TEXT_TOKEN_MIN_SIZE)).orElse(DEFAULT_MINIMUM_NGRAM_TOKEN_LENGTH); final String maxLengthString = Optional.ofNullable(index.getOption(IndexOptions.TEXT_TOKEN_MAX_SIZE)).orElse(DEFAULT_MAXIMUM_NGRAM_TOKEN_LENGTH); final String edgesOnly = Optional.ofNullable(index.getOption(LuceneIndexOptions.NGRAM_TOKEN_EDGES_ONLY)).orElse(DEFAULT_NGRAM_WITH_EDGES_ONLY); - return t -> new LuceneAnalyzerWrapper(ANALYZER_FACTORY_NAME, + return () -> new LuceneAnalyzerWrapper(ANALYZER_FACTORY_NAME, new NgramAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, Integer.parseInt(minLengthString), Integer.parseInt(maxLengthString), Boolean.parseBoolean(edgesOnly))); } catch (NumberFormatException ex) { throw new RecordCoreArgumentException("Invalid index option for token size", ex); diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/synonym/SynonymAnalyzer.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/synonym/SynonymAnalyzer.java index 813c06763e..cb85feabfe 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/synonym/SynonymAnalyzer.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/synonym/SynonymAnalyzer.java @@ -129,7 +129,7 @@ public LuceneAnalyzerType getType() { @Nonnull @Override public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { - return t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(); + return () -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(); } @SuppressWarnings("deprecation") @@ -138,7 +138,7 @@ public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { public AnalyzerChooser getQueryAnalyzerChooser(@Nonnull Index index, @Nonnull AnalyzerChooser indexAnalyzerChooser) { final String name = Objects.requireNonNullElse(index.getOption(LuceneIndexOptions.TEXT_SYNONYM_SET_NAME_OPTION), EnglishSynonymMapConfig.ExpandedEnglishSynonymMapConfig.CONFIG_NAME); - return t -> new LuceneAnalyzerWrapper(ANALYZER_FACTORY_NAME, + return () -> new LuceneAnalyzerWrapper(ANALYZER_FACTORY_NAME, new SynonymAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, name)); } } @@ -169,7 +169,7 @@ public LuceneAnalyzerType getType() { public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { final String name = Objects.requireNonNullElse(index.getOption(LuceneIndexOptions.TEXT_SYNONYM_SET_NAME_OPTION), EnglishSynonymMapConfig.AuthoritativeOnlyEnglishSynonymMapConfig.CONFIG_NAME); - return t -> new LuceneAnalyzerWrapper(ANALYZER_FACTORY_NAME, + return () -> new LuceneAnalyzerWrapper(ANALYZER_FACTORY_NAME, new SynonymAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, name)); } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneDocumentFromRecordTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneDocumentFromRecordTest.java index 58b79cc8df..58fab8b42c 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneDocumentFromRecordTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneDocumentFromRecordTest.java @@ -55,8 +55,8 @@ */ class LuceneDocumentFromRecordTest { - private LuceneAnalyzerCombinationProvider analyzerProvider = new LuceneAnalyzerCombinationProvider(t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(), - t -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(), + private LuceneAnalyzerCombinationProvider analyzerProvider = new LuceneAnalyzerCombinationProvider(() -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(), + () -> LuceneAnalyzerWrapper.getStandardAnalyzerWrapper(), null, null); @Test diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestUtils.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestUtils.java index 8ab6da5447..0d6210bca8 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestUtils.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestUtils.java @@ -845,14 +845,15 @@ public AnalyzerChooser getIndexAnalyzerChooser(@Nonnull Index index) { private static class TestAnalyzerChooser implements AnalyzerChooser { @Override @Nonnull - public LuceneAnalyzerWrapper chooseAnalyzer(@Nonnull List texts) { - if (texts.stream().anyMatch(t -> t.contains("synonym"))) { - return new LuceneAnalyzerWrapper("TEST_SYNONYM", - new SynonymAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, EnglishSynonymMapConfig.ExpandedEnglishSynonymMapConfig.CONFIG_NAME)); - } else { - return new LuceneAnalyzerWrapper("TEST_NGRAM", - new NgramAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, 3, 30, false)); - } + public LuceneAnalyzerWrapper chooseAnalyzer() { + // TODO used to be: +// if (texts.stream().anyMatch(t -> t.contains("synonym"))) { +// return new LuceneAnalyzerWrapper("TEST_SYNONYM", +// new SynonymAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, EnglishSynonymMapConfig.ExpandedEnglishSynonymMapConfig.CONFIG_NAME)); +// } else { + // Need to figure out how tests were structured to better test this + return new LuceneAnalyzerWrapper("TEST_NGRAM", + new NgramAnalyzer(EnglishAnalyzer.ENGLISH_STOP_WORDS_SET, 3, 30, false)); } } From e64d4e3cfa4728f4933211d93f15d82ae38468be Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 11:38:16 -0500 Subject: [PATCH 3/9] LuceneAnalyzerCombinationProvider no longer depends on text --- .../LuceneAnalyzerCombinationProvider.java | 21 +++++-------------- .../lucene/LuceneAutoCompleteQueryClause.java | 7 +++---- .../record/lucene/LuceneIndexMaintainer.java | 8 +++---- .../LuceneQueryFieldComparisonClause.java | 2 +- .../LuceneQueryMultiFieldSearchClause.java | 2 +- .../lucene/LuceneQuerySearchClause.java | 2 +- .../lucene/highlight/LuceneHighlighting.java | 2 +- .../lucene/LuceneAnalyzerRegistryTest.java | 4 ++-- .../lucene/LuceneIndexMaintenanceTest.java | 2 +- 9 files changed, 19 insertions(+), 31 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java index 82f98800e0..c094841d48 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java @@ -24,8 +24,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.util.Collections; -import java.util.List; import java.util.Map; import java.util.SortedMap; import java.util.TreeMap; @@ -53,25 +51,16 @@ public LuceneAnalyzerCombinationProvider(@Nonnull AnalyzerChooser defaultIndexAn this.queryAnalyzerChooserPerFieldOverride = queryAnalyzerChooserPerFieldOverride; } - public LuceneAnalyzerWrapper provideIndexAnalyzer(@Nonnull String text) { - return provideIndexAnalyzer(Collections.singletonList(text)); + public LuceneAnalyzerWrapper provideIndexAnalyzer() { + return buildAnalyzerWrapper(defaultIndexAnalyzerChooser, indexAnalyzerChooserPerFieldOverride); } - public LuceneAnalyzerWrapper provideIndexAnalyzer(@Nonnull List texts) { - return buildAnalyzerWrapper(texts, defaultIndexAnalyzerChooser, indexAnalyzerChooserPerFieldOverride); - } - - public LuceneAnalyzerWrapper provideQueryAnalyzer(@Nonnull String text) { - return provideQueryAnalyzer(Collections.singletonList(text)); - } - - public LuceneAnalyzerWrapper provideQueryAnalyzer(@Nonnull List texts) { - return buildAnalyzerWrapper(texts, defaultQueryAnalyzerChooser, queryAnalyzerChooserPerFieldOverride); + public LuceneAnalyzerWrapper provideQueryAnalyzer() { + return buildAnalyzerWrapper(defaultQueryAnalyzerChooser, queryAnalyzerChooserPerFieldOverride); } @SuppressWarnings("PMD.CloseResource") - private static LuceneAnalyzerWrapper buildAnalyzerWrapper(@Nonnull List texts, - @Nonnull AnalyzerChooser defaultAnalyzerChooser, + private static LuceneAnalyzerWrapper buildAnalyzerWrapper(@Nonnull AnalyzerChooser defaultAnalyzerChooser, @Nullable Map customizedAnalyzerChooserPerField) { final LuceneAnalyzerWrapper defaultAnalyzerWrapper = defaultAnalyzerChooser.chooseAnalyzer(); if (customizedAnalyzerChooserPerField != null) { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteQueryClause.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteQueryClause.java index 12e283b699..cbed1e06a3 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteQueryClause.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAutoCompleteQueryClause.java @@ -122,12 +122,11 @@ public BoundQuery bind(@Nonnull FDBRecordStoreBase store, @Nonnull Index inde final Map pointsConfigMap = LuceneIndexExpressions.constructPointConfigMap(store, index); LuceneQueryParserFactory parserFactory = LuceneQueryParserFactoryProvider.instance().getParserFactory(); final QueryParser parser = parserFactory.createMultiFieldQueryParser(fields.toArray(new String[0]), - analyzerSelector.provideIndexAnalyzer(searchKey).getAnalyzer(), pointsConfigMap); + analyzerSelector.provideIndexAnalyzer().getAnalyzer(), pointsConfigMap); - final var finalQuery = phraseQueryNeeded - ? buildQueryForPhraseMatching(parser, fields, searchKey) - : buildQueryForTermsMatching(analyzerSelector.provideIndexAnalyzer(searchKey).getAnalyzer(), fields, searchKey); + final Query finalQuery; + finalQuery = phraseQueryNeeded ? buildQueryForPhraseMatching(parser, fields, searchKey) : buildQueryForTermsMatching(analyzerSelector.provideIndexAnalyzer().getAnalyzer(), fields, searchKey); if (LOGGER.isDebugEnabled()) { LOGGER.debug(KeyValueLogMessage.build("query for auto-complete") .addKeyAndValue(LogMessageKeys.INDEX_NAME, index.getName()) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java index 13d3a32216..abe6e49522 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java @@ -255,7 +255,7 @@ private void writeDocument(@Nonnull List .filter(f -> f.getType().equals(LuceneIndexExpressions.DocumentFieldType.TEXT)) .map(f -> (String) f.getValue()).collect(Collectors.toList()); Document document = new Document(); - final IndexWriter newWriter = directoryManager.getIndexWriter(groupingKey, partitionId, indexAnalyzerSelector.provideIndexAnalyzer(texts)); + final IndexWriter newWriter = directoryManager.getIndexWriter(groupingKey, partitionId, indexAnalyzerSelector.provideIndexAnalyzer()); BytesRef ref = new BytesRef(keySerializer.asPackedByteArray(primaryKey)); // use packed Tuple for the Stored and Sorted fields @@ -297,7 +297,7 @@ private Map> getIndex @SuppressWarnings({"PMD.CloseResource", "java:S2095"}) int deleteDocument(Tuple groupingKey, Integer partitionId, Tuple primaryKey) throws IOException { final long startTime = System.nanoTime(); - final IndexWriter indexWriter = directoryManager.getIndexWriter(groupingKey, partitionId, indexAnalyzerSelector.provideIndexAnalyzer("")); + final IndexWriter indexWriter = directoryManager.getIndexWriter(groupingKey, partitionId, indexAnalyzerSelector.provideIndexAnalyzer()); @Nullable final LucenePrimaryKeySegmentIndex segmentIndex = directoryManager.getDirectory(groupingKey, partitionId).getPrimaryKeySegmentIndex(); if (segmentIndex != null) { @@ -358,7 +358,7 @@ public CompletableFuture mergeIndex() { return rebalancePartitions() .thenCompose(ignored -> { state.store.getIndexDeferredMaintenanceControl().setLastStep(IndexDeferredMaintenanceControl.LastStep.MERGE); - return directoryManager.mergeIndex(partitioner, indexAnalyzerSelector.provideIndexAnalyzer("")); + return directoryManager.mergeIndex(partitioner, indexAnalyzerSelector.provideIndexAnalyzer()); }); } @@ -366,7 +366,7 @@ public CompletableFuture mergeIndex() { public void mergeIndexForTesting(@Nonnull final Tuple groupingKey, @Nullable final Integer partitionId, @Nonnull final AgilityContext agilityContext) throws IOException { - directoryManager.mergeIndexWithContext(indexAnalyzerSelector.provideIndexAnalyzer(""), + directoryManager.mergeIndexWithContext(indexAnalyzerSelector.provideIndexAnalyzer(), groupingKey, partitionId, agilityContext); } diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryFieldComparisonClause.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryFieldComparisonClause.java index 804b4e66f7..d6242883a4 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryFieldComparisonClause.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryFieldComparisonClause.java @@ -279,7 +279,7 @@ public BoundQuery bind(@Nonnull FDBRecordStoreBase store, @Nonnull Index inde try { final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(index, store.getRecordMetaData()); final LuceneAnalyzerCombinationProvider analyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); - final QueryParser parser = new QueryParser(field, analyzerSelector.provideQueryAnalyzer((String) comparand).getAnalyzer()); + final QueryParser parser = new QueryParser(field, analyzerSelector.provideQueryAnalyzer().getAnalyzer()); return toBoundQuery(parser.parse("\"" + comparand + "\"")); } catch (Exception ex) { throw new RecordCoreArgumentException("Unable to parse phrase for query", ex); diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryMultiFieldSearchClause.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryMultiFieldSearchClause.java index feebdd56d4..32471c15df 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryMultiFieldSearchClause.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQueryMultiFieldSearchClause.java @@ -72,7 +72,7 @@ public BoundQuery bind(@Nonnull FDBRecordStoreBase store, @Nonnull Index inde final Map pointsConfigMap = LuceneIndexExpressions.constructPointConfigMap(store, index); LuceneQueryParserFactory parserFactory = LuceneQueryParserFactoryProvider.instance().getParserFactory(); final QueryParser parser = parserFactory.createMultiFieldQueryParser(fieldNames, - analyzerSelector.provideQueryAnalyzer(searchString).getAnalyzer(), pointsConfigMap); + analyzerSelector.provideQueryAnalyzer().getAnalyzer(), pointsConfigMap); try { return toBoundQuery(parser.parse(searchString)); } catch (final Exception ioe) { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQuerySearchClause.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQuerySearchClause.java index b144630148..8dedfa700b 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQuerySearchClause.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneQuerySearchClause.java @@ -82,7 +82,7 @@ public BoundQuery bind(@Nonnull FDBRecordStoreBase store, @Nonnull Index inde final Map pointsConfigMap = LuceneIndexExpressions.constructPointConfigMap(store, index); LuceneQueryParserFactory parserFactory = LuceneQueryParserFactoryProvider.instance().getParserFactory(); - final QueryParser parser = parserFactory.createQueryParser(defaultField, analyzerSelector.provideQueryAnalyzer(searchString).getAnalyzer(), pointsConfigMap); + final QueryParser parser = parserFactory.createQueryParser(defaultField, analyzerSelector.provideQueryAnalyzer().getAnalyzer(), pointsConfigMap); try { return toBoundQuery(parser.parse(searchString)); } catch (Exception ioe) { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/highlight/LuceneHighlighting.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/highlight/LuceneHighlighting.java index a3e4c1367a..5f7319005c 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/highlight/LuceneHighlighting.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/highlight/LuceneHighlighting.java @@ -170,7 +170,7 @@ private static Object highlight(final @Nonnull LuceneAnalyzerCombinationProvider final String fieldName, final String value, final String termName) { - final LuceneAnalyzerWrapper queryAnalyzer = analyzerSelector.provideIndexAnalyzer(value); + final LuceneAnalyzerWrapper queryAnalyzer = analyzerSelector.provideIndexAnalyzer(); UnifiedHighlighter highlighter = makeHighlighter(fieldName, queryAnalyzer.getAnalyzer(), luceneQueryHighlightParameters.getSnippedSize()); try { return highlighter.highlightWithoutSearcher(termName, luceneQueryHighlightParameters.getQuery(), value, luceneQueryHighlightParameters.getMaxMatchCount()); diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryTest.java index 01156902a3..c372431821 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerRegistryTest.java @@ -47,9 +47,9 @@ void searchForAutoCompleteWithSynonymEnabledOnFullText() { LuceneIndexOptions.TEXT_SYNONYM_SET_NAME_OPTION, EnglishSynonymMapConfig.ExpandedEnglishSynonymMapConfig.CONFIG_NAME)); // Assert the synonym analyzer is used for query analyzer for full-text search Assertions.assertEquals(SynonymAnalyzer.QueryOnlySynonymAnalyzerFactory.ANALYZER_FACTORY_NAME, - LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(index, LuceneAnalyzerType.FULL_TEXT, Map.of()).provideQueryAnalyzer("").getUniqueIdentifier()); + LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(index, LuceneAnalyzerType.FULL_TEXT, Map.of()).provideQueryAnalyzer().getUniqueIdentifier()); // Assert the standard analyzer is used for query analyzer for auto-complete suggestions Assertions.assertEquals(LuceneAnalyzerWrapper.STANDARD_ANALYZER_NAME, - LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(index, LuceneAnalyzerType.AUTO_COMPLETE, Map.of()).provideQueryAnalyzer("").getUniqueIdentifier()); + LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(index, LuceneAnalyzerType.AUTO_COMPLETE, Map.of()).provideQueryAnalyzer().getUniqueIdentifier()); } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java index 490c3f33c9..061d693e96 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java @@ -736,7 +736,7 @@ void mergeLosesLockTest(int failurePercentage) throws IOException { final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(state.index, state.store.getRecordMetaData()); LuceneAnalyzerCombinationProvider indexAnalyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); - assertThrows(IOException.class, () -> fdbDirectoryWrapper.mergeIndex(indexAnalyzerSelector.provideIndexAnalyzer(""), new Exception()), "invalid lock"); + assertThrows(IOException.class, () -> fdbDirectoryWrapper.mergeIndex(indexAnalyzerSelector.provideIndexAnalyzer(), new Exception()), "invalid lock"); commit(context); } } From 31a99bd37e0fa0c52635925c12d7739936f3fd14 Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 11:57:46 -0500 Subject: [PATCH 4/9] LuceneAnalyzerCombinationProvider can store analyzers in fields Since there is no longer a text parameter, it can generater the wrappers once. This may result in some wasted CPU generating unused analyzers (e.g. when not querying or not writing), but it should be compensated by the fact that it consistently creates 2 wrappers, rather than creating a wrapper for every record update, or every query. --- .../LuceneAnalyzerCombinationProvider.java | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java index c094841d48..00f530470a 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java @@ -35,28 +35,24 @@ * The default analyzer chooser is used for all fields of one Lucene index except the fields which has overrides in the analyzer chooser per field mapping. */ public class LuceneAnalyzerCombinationProvider { - public static final String DELINEATOR_BETWEEN_KEY_AND_VALUE = ":"; - public static final String DELINEATOR_BETWEEN_KEY_VALUE_PAIRS = ","; - private AnalyzerChooser defaultIndexAnalyzerChooser; - private AnalyzerChooser defaultQueryAnalyzerChooser; - private Map indexAnalyzerChooserPerFieldOverride; - private Map queryAnalyzerChooserPerFieldOverride; + private final LuceneAnalyzerWrapper indexAnalyzerWrapper; + private final LuceneAnalyzerWrapper queryAnalyzerWrapper; - public LuceneAnalyzerCombinationProvider(@Nonnull AnalyzerChooser defaultIndexAnalyzerChooser, @Nonnull AnalyzerChooser defaultQueryAnalyzerChooser, - @Nullable Map indexAnalyzerChooserPerFieldOverride, @Nullable Map queryAnalyzerChooserPerFieldOverride) { - this.defaultIndexAnalyzerChooser = defaultIndexAnalyzerChooser; - this.defaultQueryAnalyzerChooser = defaultQueryAnalyzerChooser; - this.indexAnalyzerChooserPerFieldOverride = indexAnalyzerChooserPerFieldOverride; - this.queryAnalyzerChooserPerFieldOverride = queryAnalyzerChooserPerFieldOverride; + public LuceneAnalyzerCombinationProvider(@Nonnull AnalyzerChooser defaultIndexAnalyzerChooser, + @Nonnull AnalyzerChooser defaultQueryAnalyzerChooser, + @Nullable Map indexAnalyzerChooserPerFieldOverride, + @Nullable Map queryAnalyzerChooserPerFieldOverride) { + indexAnalyzerWrapper = buildAnalyzerWrapper(defaultIndexAnalyzerChooser, indexAnalyzerChooserPerFieldOverride); + queryAnalyzerWrapper = buildAnalyzerWrapper(defaultQueryAnalyzerChooser, queryAnalyzerChooserPerFieldOverride); } public LuceneAnalyzerWrapper provideIndexAnalyzer() { - return buildAnalyzerWrapper(defaultIndexAnalyzerChooser, indexAnalyzerChooserPerFieldOverride); + return indexAnalyzerWrapper; } public LuceneAnalyzerWrapper provideQueryAnalyzer() { - return buildAnalyzerWrapper(defaultQueryAnalyzerChooser, queryAnalyzerChooserPerFieldOverride); + return queryAnalyzerWrapper; } @SuppressWarnings("PMD.CloseResource") From 64dd9c1e6265ea95da3ce1d6606cac2680653ffc Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 12:18:53 -0500 Subject: [PATCH 5/9] Initialize Analyzer used by IndexWriter in FDBDirectoryManager Only initialize this once, and reuse. Part of the process of making FDBDirectoryWrapper not have to worry about changing analyzers --- .../LuceneAnalyzerCombinationProvider.java | 5 +++ .../record/lucene/LuceneIndexMaintainer.java | 18 +++++----- .../lucene/directory/FDBDirectoryManager.java | 36 +++++++++++-------- .../record/lucene/package-info.java | 2 +- .../lucene/LuceneIndexTestValidator.java | 2 +- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java index 00f530470a..f1dca5d27f 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneAnalyzerCombinationProvider.java @@ -36,7 +36,9 @@ */ public class LuceneAnalyzerCombinationProvider { + @Nonnull private final LuceneAnalyzerWrapper indexAnalyzerWrapper; + @Nonnull private final LuceneAnalyzerWrapper queryAnalyzerWrapper; public LuceneAnalyzerCombinationProvider(@Nonnull AnalyzerChooser defaultIndexAnalyzerChooser, @@ -47,14 +49,17 @@ public LuceneAnalyzerCombinationProvider(@Nonnull AnalyzerChooser defaultIndexAn queryAnalyzerWrapper = buildAnalyzerWrapper(defaultQueryAnalyzerChooser, queryAnalyzerChooserPerFieldOverride); } + @Nonnull public LuceneAnalyzerWrapper provideIndexAnalyzer() { return indexAnalyzerWrapper; } + @Nonnull public LuceneAnalyzerWrapper provideQueryAnalyzer() { return queryAnalyzerWrapper; } + @Nonnull @SuppressWarnings("PMD.CloseResource") private static LuceneAnalyzerWrapper buildAnalyzerWrapper(@Nonnull AnalyzerChooser defaultAnalyzerChooser, @Nullable Map customizedAnalyzerChooserPerField) { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java index abe6e49522..a48f900926 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintainer.java @@ -112,7 +112,7 @@ public class LuceneIndexMaintainer extends StandardIndexMaintainer { private static final Logger LOG = LoggerFactory.getLogger(LuceneIndexMaintainer.class); private final FDBDirectoryManager directoryManager; - private final LuceneAnalyzerCombinationProvider indexAnalyzerSelector; + private final LuceneAnalyzerCombinationProvider queryAnalyzerSelector; private final LuceneAnalyzerCombinationProvider autoCompleteAnalyzerSelector; public static final String PRIMARY_KEY_FIELD_NAME = "_p"; protected static final String PRIMARY_KEY_SEARCH_NAME = "_s"; @@ -128,7 +128,7 @@ public LuceneIndexMaintainer(@Nonnull final IndexMaintainerState state, @Nonnull this.executor = executor; this.directoryManager = FDBDirectoryManager.getManager(state); final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(state.index, state.store.getRecordMetaData()); - this.indexAnalyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); + this.queryAnalyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); this.autoCompleteAnalyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.AUTO_COMPLETE, fieldInfos); String formatString = state.index.getOption(LuceneIndexOptions.PRIMARY_KEY_SERIALIZATION_FORMAT); keySerializer = LuceneIndexKeySerializer.fromStringFormat(formatString); @@ -175,7 +175,7 @@ public RecordCursor scan(@Nonnull final IndexScanBounds scanBounds, state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_INDEX_CURSOR_PAGE_SIZE), scanProperties, state, scanQuery.getQuery(), scanQuery.getSort(), continuation, scanQuery.getGroupKey(), partitionInfo, scanQuery.getLuceneQueryHighlightParameters(), scanQuery.getTermMap(), - scanQuery.getStoredFields(), scanQuery.getStoredFieldTypes(), indexAnalyzerSelector, autoCompleteAnalyzerSelector); + scanQuery.getStoredFields(), scanQuery.getStoredFieldTypes(), queryAnalyzerSelector, autoCompleteAnalyzerSelector); } if (scanType.equals(LuceneScanTypes.BY_LUCENE_SPELL_CHECK)) { @@ -184,7 +184,8 @@ public RecordCursor scan(@Nonnull final IndexScanBounds scanBounds, } LuceneScanSpellCheck scanSpellcheck = (LuceneScanSpellCheck)scanBounds; return new LuceneSpellCheckRecordCursor(scanSpellcheck.getFields(), scanSpellcheck.getWord(), - executor, scanProperties, state, scanSpellcheck.getGroupKey(), partitioner.selectQueryPartitionId(scanSpellcheck.getGroupKey())); + executor, scanProperties, state, scanSpellcheck.getGroupKey(), + partitioner.selectQueryPartitionId(scanSpellcheck.getGroupKey())); } throw new RecordCoreException("unsupported scan type for Lucene index: " + scanType); @@ -255,7 +256,7 @@ private void writeDocument(@Nonnull List .filter(f -> f.getType().equals(LuceneIndexExpressions.DocumentFieldType.TEXT)) .map(f -> (String) f.getValue()).collect(Collectors.toList()); Document document = new Document(); - final IndexWriter newWriter = directoryManager.getIndexWriter(groupingKey, partitionId, indexAnalyzerSelector.provideIndexAnalyzer()); + final IndexWriter newWriter = directoryManager.getIndexWriter(groupingKey, partitionId); BytesRef ref = new BytesRef(keySerializer.asPackedByteArray(primaryKey)); // use packed Tuple for the Stored and Sorted fields @@ -297,7 +298,7 @@ private Map> getIndex @SuppressWarnings({"PMD.CloseResource", "java:S2095"}) int deleteDocument(Tuple groupingKey, Integer partitionId, Tuple primaryKey) throws IOException { final long startTime = System.nanoTime(); - final IndexWriter indexWriter = directoryManager.getIndexWriter(groupingKey, partitionId, indexAnalyzerSelector.provideIndexAnalyzer()); + final IndexWriter indexWriter = directoryManager.getIndexWriter(groupingKey, partitionId); @Nullable final LucenePrimaryKeySegmentIndex segmentIndex = directoryManager.getDirectory(groupingKey, partitionId).getPrimaryKeySegmentIndex(); if (segmentIndex != null) { @@ -358,7 +359,7 @@ public CompletableFuture mergeIndex() { return rebalancePartitions() .thenCompose(ignored -> { state.store.getIndexDeferredMaintenanceControl().setLastStep(IndexDeferredMaintenanceControl.LastStep.MERGE); - return directoryManager.mergeIndex(partitioner, indexAnalyzerSelector.provideIndexAnalyzer()); + return directoryManager.mergeIndex(partitioner); }); } @@ -366,8 +367,7 @@ public CompletableFuture mergeIndex() { public void mergeIndexForTesting(@Nonnull final Tuple groupingKey, @Nullable final Integer partitionId, @Nonnull final AgilityContext agilityContext) throws IOException { - directoryManager.mergeIndexWithContext(indexAnalyzerSelector.provideIndexAnalyzer(), - groupingKey, partitionId, agilityContext); + directoryManager.mergeIndexWithContext(groupingKey, partitionId, agilityContext); } @Nonnull diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java index 1d64116f74..6a67735bd2 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java @@ -30,8 +30,11 @@ import com.apple.foundationdb.record.ScanProperties; import com.apple.foundationdb.record.cursors.ChainedCursor; import com.apple.foundationdb.record.logging.KeyValueLogMessage; +import com.apple.foundationdb.record.lucene.LuceneAnalyzerRegistryImpl; +import com.apple.foundationdb.record.lucene.LuceneAnalyzerType; import com.apple.foundationdb.record.lucene.LuceneAnalyzerWrapper; import com.apple.foundationdb.record.lucene.LuceneExceptions; +import com.apple.foundationdb.record.lucene.LuceneIndexExpressions; import com.apple.foundationdb.record.lucene.LuceneIndexTypes; import com.apple.foundationdb.record.lucene.LuceneLogMessageKeys; import com.apple.foundationdb.record.lucene.LucenePartitionInfoProto; @@ -80,6 +83,7 @@ public class FDBDirectoryManager implements AutoCloseable { private final Map createdDirectories; private final int mergeDirectoryCount; private final Exception exceptionAtCreation; + private final LuceneAnalyzerWrapper writerAnalyzer; protected FDBDirectoryManager(@Nonnull IndexMaintainerState state) { this.state = state; @@ -90,6 +94,9 @@ protected FDBDirectoryManager(@Nonnull IndexMaintainerState state) { } else { this.exceptionAtCreation = null; } + final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(state.index, state.store.getRecordMetaData()); + final var analyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); + this.writerAnalyzer = analyzerSelector.provideIndexAnalyzer(); } @Override @@ -102,7 +109,7 @@ public synchronized void close() throws IOException { } @SuppressWarnings("PMD.CloseResource") - public CompletableFuture mergeIndex(@Nonnull LucenePartitioner partitioner, LuceneAnalyzerWrapper analyzerWrapper) { + public CompletableFuture mergeIndex(@Nonnull LucenePartitioner partitioner) { // This function will iterate the grouping keys and explicitly merge each final ScanProperties scanProperties = ScanProperties.FORWARD_SCAN.with( @@ -119,7 +126,7 @@ public CompletableFuture mergeIndex(@Nonnull LucenePartitioner partitioner if (! (rootExpression instanceof GroupingKeyExpression)) { // Here: empty grouping keys tuple - return mergeIndex(analyzerWrapper, TupleHelpers.EMPTY, partitioner, agilityContext) + return mergeIndex(TupleHelpers.EMPTY, partitioner, agilityContext) .whenComplete((ignore, ex) -> closeOrAbortAgilityContext(agilityContext, ex)); } // Here: iterate the grouping keys and merge each @@ -141,19 +148,19 @@ public CompletableFuture mergeIndex(@Nonnull LucenePartitioner partitioner // It may make sense in the future to make these concurrent, but there is enough complexity that it is // better to avoid the concurrent merges. // This also reduces the amount of load that a single store can cause on a system. - .forEachAsync(groupingKey -> mergeIndex(analyzerWrapper, groupingKey, partitioner, agilityContext), + .forEachAsync(groupingKey -> mergeIndex(groupingKey, partitioner, agilityContext), 1) .whenComplete((ignore, ex) -> closeOrAbortAgilityContext(agilityContext, ex)); } - private CompletableFuture mergeIndex(LuceneAnalyzerWrapper analyzerWrapper, Tuple groupingKey, + private CompletableFuture mergeIndex(Tuple groupingKey, @Nonnull LucenePartitioner partitioner, final AgilityContext agileContext) { // Note: We always flush before calls to `mergeIndexNow` because we won't come back to get the next partition // or group until after the merge which could be many seconds later, in which case the current transaction would // no longer be valid. It may make sense to have AgilityContext.Agile commit periodically regardless of activity if (!partitioner.isPartitioningEnabled()) { agileContext.flush(); - mergeIndexNow(analyzerWrapper, groupingKey, null); + mergeIndexNow(groupingKey, null); return AsyncUtil.DONE; } else { // Here: iterate the partition ids and merge each @@ -165,16 +172,16 @@ private CompletableFuture mergeIndex(LuceneAnalyzerWrapper analyzerWrapper return false; } agileContext.flush(); - mergeIndexNow(analyzerWrapper, groupingKey, partitionId); + mergeIndexNow(groupingKey, partitionId); return true; })); } } - private void mergeIndexNow(LuceneAnalyzerWrapper analyzerWrapper, Tuple groupingKey, @Nullable final Integer partitionId) { + private void mergeIndexNow(Tuple groupingKey, @Nullable final Integer partitionId) { final AgilityContext agilityContext = getAgilityContext(true, true); try { - mergeIndexWithContext(analyzerWrapper, groupingKey, partitionId, agilityContext); + mergeIndexWithContext(groupingKey, partitionId, agilityContext); } finally { // IndexWriter may release the file lock in a finally block in its own code, so if there is an error in its // code, we need to commit. We could optimize this a bit, and have it only flush if it has committed anything @@ -183,13 +190,12 @@ private void mergeIndexNow(LuceneAnalyzerWrapper analyzerWrapper, Tuple grouping } } - public void mergeIndexWithContext(@Nonnull final LuceneAnalyzerWrapper analyzerWrapper, - @Nonnull final Tuple groupingKey, - @Nullable final Integer partitionId, - @Nonnull final AgilityContext agilityContext) { + public void mergeIndexWithContext(@Nonnull final Tuple groupingKey, + @Nullable final Integer partitionId, + @Nonnull final AgilityContext agilityContext) { try (FDBDirectoryWrapper directoryWrapper = createDirectoryWrapper(groupingKey, partitionId, agilityContext)) { try { - directoryWrapper.mergeIndex(analyzerWrapper, exceptionAtCreation); + directoryWrapper.mergeIndex(this.writerAnalyzer, exceptionAtCreation); if (LOGGER.isDebugEnabled()) { LOGGER.debug(KeyValueLogMessage.of("Lucene merge success", LuceneLogMessageKeys.GROUP, groupingKey, @@ -349,8 +355,8 @@ public IndexReader getIndexReader(@Nullable Tuple groupingKey, @Nullable Integer } @Nonnull - public IndexWriter getIndexWriter(@Nullable Tuple groupingKey, @Nullable Integer partitionId, @Nonnull LuceneAnalyzerWrapper analyzerWrapper) throws IOException { - return getDirectoryWrapper(groupingKey, partitionId).getWriter(analyzerWrapper, exceptionAtCreation); + public IndexWriter getIndexWriter(@Nullable Tuple groupingKey, @Nullable Integer partitionId) throws IOException { + return getDirectoryWrapper(groupingKey, partitionId).getWriter(this.writerAnalyzer, exceptionAtCreation); } public DirectoryReader getDirectoryReader(@Nullable Tuple groupingKey, @Nullable Integer partititonId) throws IOException { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/package-info.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/package-info.java index d718a4abcd..5534e770c8 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/package-info.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/package-info.java @@ -24,7 +24,7 @@ *

* Lucene indexes are backed by FDB, using {@link com.apple.foundationdb.record.lucene.directory.FDBDirectory} to implement a virtual file system holding the inverted index files. * This is not fundamental, though. This maintainer used standard {@link org.apache.lucene.index.IndexWriter} and {@link org.apache.lucene.index.IndexReader}, gotten with - * {@link com.apple.foundationdb.record.lucene.directory.FDBDirectoryManager#getIndexWriter(com.apple.foundationdb.tuple.Tuple, java.lang.Integer, com.apple.foundationdb.record.lucene.LuceneAnalyzerWrapper)} getIndexWriter}, + * {@link #getIndexWriter(com.apple.foundationdb.tuple.Tuple, Integer)} getIndexWriter}, * for interfacing to Lucene. *

* diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java index ccf8a961b9..29b8dc285b 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java @@ -303,7 +303,7 @@ public static void validatePrimaryKeySegmentIndex(@Nonnull FDBRecordStore record .collect(Collectors.toList()), message); } - directoryManager.getIndexWriter(groupingKey, partitionId, LuceneAnalyzerWrapper.getStandardAnalyzerWrapper()); + directoryManager.getIndexWriter(groupingKey, partitionId); final DirectoryReader directoryReader = directoryManager.getDirectoryReader(groupingKey, partitionId); for (final Tuple primaryKey : expectedPrimaryKeys) { assertNotNull(primaryKeySegmentIndex.findDocument(directoryReader, primaryKey), From 8643b4bf1bcd58ecafbc1b5c405eb888b708ceda Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 12:33:05 -0500 Subject: [PATCH 6/9] Make FDBDirectoryWrapper.writer lazy, but final Since the analyzer is passed into the constructor, there's no reason for the writer to change. --- .../lucene/directory/FDBDirectoryManager.java | 9 ++++--- .../lucene/directory/FDBDirectoryWrapper.java | 26 ++++++++++--------- .../lucene/LuceneIndexMaintenanceTest.java | 10 ++++--- .../directory/MockedFDBDirectoryManager.java | 8 ++++-- .../directory/MockedFDBDirectoryWrapper.java | 7 +++-- 5 files changed, 36 insertions(+), 24 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java index 6a67735bd2..8134fca77e 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java @@ -83,7 +83,8 @@ public class FDBDirectoryManager implements AutoCloseable { private final Map createdDirectories; private final int mergeDirectoryCount; private final Exception exceptionAtCreation; - private final LuceneAnalyzerWrapper writerAnalyzer; + @Nonnull + protected final LuceneAnalyzerWrapper writerAnalyzer; protected FDBDirectoryManager(@Nonnull IndexMaintainerState state) { this.state = state; @@ -195,7 +196,7 @@ public void mergeIndexWithContext(@Nonnull final Tuple groupingKey, @Nonnull final AgilityContext agilityContext) { try (FDBDirectoryWrapper directoryWrapper = createDirectoryWrapper(groupingKey, partitionId, agilityContext)) { try { - directoryWrapper.mergeIndex(this.writerAnalyzer, exceptionAtCreation); + directoryWrapper.mergeIndex(exceptionAtCreation); if (LOGGER.isDebugEnabled()) { LOGGER.debug(KeyValueLogMessage.of("Lucene merge success", LuceneLogMessageKeys.GROUP, groupingKey, @@ -301,7 +302,7 @@ private FDBDirectoryWrapper createDirectoryWrapper(@Nullable Tuple groupingKey, } protected @Nonnull FDBDirectoryWrapper createNewDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize) { - return new FDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize); + return new FDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, writerAnalyzer); } private int getBlockCacheMaximumSize() { @@ -356,7 +357,7 @@ public IndexReader getIndexReader(@Nullable Tuple groupingKey, @Nullable Integer @Nonnull public IndexWriter getIndexWriter(@Nullable Tuple groupingKey, @Nullable Integer partitionId) throws IOException { - return getDirectoryWrapper(groupingKey, partitionId).getWriter(this.writerAnalyzer, exceptionAtCreation); + return getDirectoryWrapper(groupingKey, partitionId).getWriter(exceptionAtCreation); } public DirectoryReader getDirectoryReader(@Nullable Tuple groupingKey, @Nullable Integer partititonId) throws IOException { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java index cce0634ac2..75073616d0 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java @@ -70,14 +70,15 @@ public class FDBDirectoryWrapper implements AutoCloseable { private final int mergeDirectoryCount; private final AgilityContext agilityContext; private final Tuple key; + private final LuceneAnalyzerWrapper writerAnalyzer; @SuppressWarnings({"squid:S3077"}) // object is thread safe, so use of volatile to control instance creation is correct private volatile IndexWriter writer; @SuppressWarnings({"squid:S3077"}) // object is thread safe, so use of volatile to control instance creation is correct - private volatile String writerAnalyzerId; - @SuppressWarnings({"squid:S3077"}) // object is thread safe, so use of volatile to control instance creation is correct private volatile DirectoryReader writerReader; - FDBDirectoryWrapper(IndexMaintainerState state, Tuple key, int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize) { + FDBDirectoryWrapper(IndexMaintainerState state, Tuple key, int mergeDirectoryCount, + final AgilityContext agilityContext, final int blockCacheMaximumSize, + final LuceneAnalyzerWrapper writerAnalyzer) { final Subspace subspace = state.indexSubspace.subspace(key); final FDBDirectorySharedCacheManager sharedCacheManager = FDBDirectorySharedCacheManager.forContext(state.context); final Tuple sharedCacheKey = sharedCacheManager == null ? null : @@ -87,15 +88,18 @@ public class FDBDirectoryWrapper implements AutoCloseable { this.directory = createFDBDirectory(subspace, state.index.getOptions(), sharedCacheManager, sharedCacheKey, USE_COMPOUND_FILE, agilityContext, blockCacheMaximumSize); this.agilityContext = agilityContext; this.mergeDirectoryCount = mergeDirectoryCount; + this.writerAnalyzer = writerAnalyzer; } @VisibleForTesting - public FDBDirectoryWrapper(IndexMaintainerState state, FDBDirectory directory, Tuple key, int mergeDirectoryCount, final AgilityContext agilityContext) { + public FDBDirectoryWrapper(IndexMaintainerState state, FDBDirectory directory, Tuple key, int mergeDirectoryCount, + final AgilityContext agilityContext, final LuceneAnalyzerWrapper writerAnalyzer) { this.state = state; this.key = key; this.directory = directory; this.agilityContext = agilityContext; this.mergeDirectoryCount = mergeDirectoryCount; + this.writerAnalyzer = writerAnalyzer; } public FDBDirectory getDirectory() { @@ -268,16 +272,16 @@ private MergeScheduler getMergeScheduler(@Nonnull IndexMaintainerState state, @Nonnull @SuppressWarnings("PMD.CloseResource") - public IndexWriter getWriter(@Nonnull LuceneAnalyzerWrapper analyzerWrapper, @Nullable final Exception exceptionAtCreation) throws IOException { - if (writer == null || !writerAnalyzerId.equals(analyzerWrapper.getUniqueIdentifier())) { + public IndexWriter getWriter(@Nullable final Exception exceptionAtCreation) throws IOException { + if (writer == null) { synchronized (this) { - if (writer == null || !writerAnalyzerId.equals(analyzerWrapper.getUniqueIdentifier())) { + if (writer == null) { final IndexDeferredMaintenanceControl mergeControl = state.store.getIndexDeferredMaintenanceControl(); TieredMergePolicy tieredMergePolicy = new FDBTieredMergePolicy(mergeControl, agilityContext, state.indexSubspace, key, exceptionAtCreation) .setMaxMergedSegmentMB(state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_MERGE_MAX_SIZE)) .setSegmentsPerTier(state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_MERGE_SEGMENTS_PER_TIER)); tieredMergePolicy.setNoCFSRatio(1.00); - IndexWriterConfig indexWriterConfig = new IndexWriterConfig(analyzerWrapper.getAnalyzer()) + IndexWriterConfig indexWriterConfig = new IndexWriterConfig(writerAnalyzer.getAnalyzer()) .setUseCompoundFile(USE_COMPOUND_FILE) .setMergePolicy(tieredMergePolicy) .setMergeScheduler(getMergeScheduler(state, mergeDirectoryCount, agilityContext, key)) @@ -288,7 +292,6 @@ public IndexWriter getWriter(@Nonnull LuceneAnalyzerWrapper analyzerWrapper, @Nu writer.close(); } writer = new IndexWriter(directory, indexWriterConfig); - writerAnalyzerId = analyzerWrapper.getUniqueIdentifier(); if (writerReader != null) { writerReader.close(); @@ -308,12 +311,11 @@ public IndexWriter getWriter(@Nonnull LuceneAnalyzerWrapper analyzerWrapper, @Nu public synchronized void close() throws IOException { IOUtils.close(writer, writerReader, directory); writer = null; - writerAnalyzerId = null; writerReader = null; } - public void mergeIndex(@Nonnull LuceneAnalyzerWrapper analyzerWrapper, final Exception exceptionAtCreation) throws IOException { - getWriter(analyzerWrapper, exceptionAtCreation).maybeMerge(); + public void mergeIndex(final Exception exceptionAtCreation) throws IOException { + getWriter(exceptionAtCreation).maybeMerge(); } protected @Nonnull FDBDirectory createFDBDirectory(final Subspace subspace, diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java index 061d693e96..7926a1a43c 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java @@ -729,14 +729,16 @@ void mergeLosesLockTest(int failurePercentage) throws IOException { Tuple directoryKey = Tuple.from(1, LucenePartitioner.PARTITION_DATA_SUBSPACE, 0); IndexMaintainerState state = new IndexMaintainerState(recordStore, index, IndexMaintenanceFilter.NORMAL); + final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(state.index, state.store.getRecordMetaData()); + LuceneAnalyzerCombinationProvider indexAnalyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); // custom test directory that returns a lucene lock that's never valid (Lock.ensureValid() throws IOException) FDBDirectory fdbDirectory = new InvalidLockTestFDBDirectory(recordStore.indexSubspace(index).subspace(directoryKey), context, options, failurePercentage); - FDBDirectoryWrapper fdbDirectoryWrapper = new FDBDirectoryWrapper(state, fdbDirectory, directoryKey, 1, AgilityContext.agile(context, 1L, 1L)); + FDBDirectoryWrapper fdbDirectoryWrapper = new FDBDirectoryWrapper(state, fdbDirectory, directoryKey, + 1, AgilityContext.agile(context, 1L, 1L), + indexAnalyzerSelector.provideIndexAnalyzer()); - final var fieldInfos = LuceneIndexExpressions.getDocumentFieldDerivations(state.index, state.store.getRecordMetaData()); - LuceneAnalyzerCombinationProvider indexAnalyzerSelector = LuceneAnalyzerRegistryImpl.instance().getLuceneAnalyzerCombinationProvider(state.index, LuceneAnalyzerType.FULL_TEXT, fieldInfos); - assertThrows(IOException.class, () -> fdbDirectoryWrapper.mergeIndex(indexAnalyzerSelector.provideIndexAnalyzer(), new Exception()), "invalid lock"); + assertThrows(IOException.class, () -> fdbDirectoryWrapper.mergeIndex(new Exception()), "invalid lock"); commit(context); } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java index e234e49650..30c0f56c14 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java @@ -35,7 +35,11 @@ public MockedFDBDirectoryManager(@Nonnull final IndexMaintainerState state) { @Nonnull @Override - protected FDBDirectoryWrapper createNewDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize) { - return new MockedFDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize); + protected FDBDirectoryWrapper createNewDirectoryWrapper(final IndexMaintainerState state, final Tuple key, + final int mergeDirectoryCount, + final AgilityContext agilityContext, + final int blockCacheMaximumSize) { + return new MockedFDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, + writerAnalyzer); } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java index 15df4859e1..db82aa5477 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java @@ -20,6 +20,7 @@ package com.apple.foundationdb.record.lucene.directory; +import com.apple.foundationdb.record.lucene.LuceneAnalyzerWrapper; import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; import com.apple.foundationdb.subspace.Subspace; import com.apple.foundationdb.tuple.Tuple; @@ -31,8 +32,10 @@ * A Testing-focused {@link FDBDirectoryWrapper} that allows a mocked-FDBDirectory to be injected into the system. */ public class MockedFDBDirectoryWrapper extends FDBDirectoryWrapper { - MockedFDBDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize) { - super(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize); + MockedFDBDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, + final AgilityContext agilityContext, final int blockCacheMaximumSize, + final LuceneAnalyzerWrapper writerAnalyzer) { + super(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, writerAnalyzer); } @Nonnull From 1204321b921e1289bef690584fc1e867a6e29da6 Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Tue, 3 Dec 2024 16:41:52 -0500 Subject: [PATCH 7/9] Use LazyCloseable for FDBDirectoryWrapper.writer By using a LazyCloseable instead of synchronized blocks, it allows the ForkJoinPool to mark the thread as inactive, and will create new threads to process operations, thus avoiding the deadlock --- .../lucene/directory/FDBDirectoryManager.java | 5 +- .../lucene/directory/FDBDirectoryWrapper.java | 114 +++++++++--------- .../lucene/LuceneIndexMaintenanceTest.java | 2 +- .../directory/MockedFDBDirectoryManager.java | 2 +- .../directory/MockedFDBDirectoryWrapper.java | 19 ++- 5 files changed, 68 insertions(+), 74 deletions(-) diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java index 8134fca77e..a0de3a6997 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryManager.java @@ -82,7 +82,7 @@ public class FDBDirectoryManager implements AutoCloseable { @Nonnull private final Map createdDirectories; private final int mergeDirectoryCount; - private final Exception exceptionAtCreation; + protected final Exception exceptionAtCreation; @Nonnull protected final LuceneAnalyzerWrapper writerAnalyzer; @@ -302,7 +302,8 @@ private FDBDirectoryWrapper createDirectoryWrapper(@Nullable Tuple groupingKey, } protected @Nonnull FDBDirectoryWrapper createNewDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize) { - return new FDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, writerAnalyzer); + return new FDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, + writerAnalyzer, exceptionAtCreation); } private int getBlockCacheMaximumSize() { diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java index 75073616d0..bc04d1d43c 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryWrapper.java @@ -24,6 +24,7 @@ import com.apple.foundationdb.record.lucene.LuceneEvents; import com.apple.foundationdb.record.lucene.LuceneLoggerInfoStream; import com.apple.foundationdb.record.lucene.LuceneRecordContextProperties; +import com.apple.foundationdb.record.lucene.codec.LazyCloseable; import com.apple.foundationdb.record.lucene.codec.LuceneOptimizedCodec; import com.apple.foundationdb.record.provider.foundationdb.IndexDeferredMaintenanceControl; import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; @@ -48,7 +49,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.io.IOException; -import java.util.Map; import java.util.Objects; import java.util.concurrent.ThreadLocalRandom; @@ -70,36 +70,53 @@ public class FDBDirectoryWrapper implements AutoCloseable { private final int mergeDirectoryCount; private final AgilityContext agilityContext; private final Tuple key; - private final LuceneAnalyzerWrapper writerAnalyzer; - @SuppressWarnings({"squid:S3077"}) // object is thread safe, so use of volatile to control instance creation is correct - private volatile IndexWriter writer; + private volatile boolean useWriter; + // object is thread safe, so use of volatile to control instance creation is correct + private final LazyCloseable writerOpener; @SuppressWarnings({"squid:S3077"}) // object is thread safe, so use of volatile to control instance creation is correct private volatile DirectoryReader writerReader; FDBDirectoryWrapper(IndexMaintainerState state, Tuple key, int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize, - final LuceneAnalyzerWrapper writerAnalyzer) { - final Subspace subspace = state.indexSubspace.subspace(key); - final FDBDirectorySharedCacheManager sharedCacheManager = FDBDirectorySharedCacheManager.forContext(state.context); - final Tuple sharedCacheKey = sharedCacheManager == null ? null : - (sharedCacheManager.getSubspace() == null ? state.store.getSubspace() : sharedCacheManager.getSubspace()).unpack(subspace.pack()); - this.state = state; - this.key = key; - this.directory = createFDBDirectory(subspace, state.index.getOptions(), sharedCacheManager, sharedCacheKey, USE_COMPOUND_FILE, agilityContext, blockCacheMaximumSize); - this.agilityContext = agilityContext; - this.mergeDirectoryCount = mergeDirectoryCount; - this.writerAnalyzer = writerAnalyzer; + final LuceneAnalyzerWrapper writerAnalyzer, final Exception exceptionAtManagerCreation) { + this(state, createFDBDirectory(state, key, agilityContext, blockCacheMaximumSize), + key, mergeDirectoryCount, agilityContext, writerAnalyzer, exceptionAtManagerCreation); } @VisibleForTesting public FDBDirectoryWrapper(IndexMaintainerState state, FDBDirectory directory, Tuple key, int mergeDirectoryCount, - final AgilityContext agilityContext, final LuceneAnalyzerWrapper writerAnalyzer) { + final AgilityContext agilityContext, final LuceneAnalyzerWrapper writerAnalyzer, + final Exception exceptionAtManagerCreation) { this.state = state; this.key = key; this.directory = directory; this.agilityContext = agilityContext; this.mergeDirectoryCount = mergeDirectoryCount; - this.writerAnalyzer = writerAnalyzer; + useWriter = false; + writerOpener = LazyCloseable.supply(() -> { + useWriter = true; + return createIndexWriter(exceptionAtManagerCreation, writerAnalyzer); + }); + } + + @Nonnull + protected static FDBDirectory createFDBDirectory(final IndexMaintainerState state, + final Tuple key, + final AgilityContext agilityContext, + final int blockCacheMaximumSize) { + final Subspace subspace = state.indexSubspace.subspace(key); + final FDBDirectorySharedCacheManager sharedCacheManager = FDBDirectorySharedCacheManager.forContext(state.context); + final Tuple sharedCacheKey; + if (sharedCacheManager == null) { + sharedCacheKey = null; + } else { + if (sharedCacheManager.getSubspace() == null) { + sharedCacheKey = state.store.getSubspace().unpack(subspace.pack()); + } else { + sharedCacheKey = sharedCacheManager.getSubspace().unpack(subspace.pack()); + } + } + return new FDBDirectory(subspace, state.index.getOptions(), sharedCacheManager, sharedCacheKey, USE_COMPOUND_FILE, agilityContext, blockCacheMaximumSize); } public FDBDirectory getDirectory() { @@ -108,7 +125,7 @@ public FDBDirectory getDirectory() { @SuppressWarnings("PMD.CloseResource") public IndexReader getReader() throws IOException { - if (writer == null) { + if (!useWriter) { return StandardDirectoryReaderOptimization.open(directory, null, null, state.context.getExecutor(), state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_OPEN_PARALLELISM)); @@ -122,7 +139,7 @@ public DirectoryReader getWriterReader(boolean flush) throws IOException { if (flush || writerReader == null) { synchronized (this) { if (flush || writerReader == null) { - writerReader = DirectoryReader.open(Objects.requireNonNull(writer)); + writerReader = DirectoryReader.open(Objects.requireNonNull(writerOpener.get())); } } } @@ -273,57 +290,38 @@ private MergeScheduler getMergeScheduler(@Nonnull IndexMaintainerState state, @Nonnull @SuppressWarnings("PMD.CloseResource") public IndexWriter getWriter(@Nullable final Exception exceptionAtCreation) throws IOException { - if (writer == null) { - synchronized (this) { - if (writer == null) { - final IndexDeferredMaintenanceControl mergeControl = state.store.getIndexDeferredMaintenanceControl(); - TieredMergePolicy tieredMergePolicy = new FDBTieredMergePolicy(mergeControl, agilityContext, state.indexSubspace, key, exceptionAtCreation) - .setMaxMergedSegmentMB(state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_MERGE_MAX_SIZE)) - .setSegmentsPerTier(state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_MERGE_SEGMENTS_PER_TIER)); - tieredMergePolicy.setNoCFSRatio(1.00); - IndexWriterConfig indexWriterConfig = new IndexWriterConfig(writerAnalyzer.getAnalyzer()) - .setUseCompoundFile(USE_COMPOUND_FILE) - .setMergePolicy(tieredMergePolicy) - .setMergeScheduler(getMergeScheduler(state, mergeDirectoryCount, agilityContext, key)) - .setCodec(CODEC) - .setInfoStream(new LuceneLoggerInfoStream(LOGGER)); + return writerOpener.get(); + } - if (writer != null) { - writer.close(); - } - writer = new IndexWriter(directory, indexWriterConfig); + @Nonnull + private IndexWriter createIndexWriter(@Nullable final Exception exceptionAtCreation, + @Nonnull final LuceneAnalyzerWrapper writerAnalyzer) throws IOException { + final IndexDeferredMaintenanceControl mergeControl = state.store.getIndexDeferredMaintenanceControl(); + TieredMergePolicy tieredMergePolicy = new FDBTieredMergePolicy(mergeControl, agilityContext, state.indexSubspace, key, exceptionAtCreation) + .setMaxMergedSegmentMB(state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_MERGE_MAX_SIZE)) + .setSegmentsPerTier(state.context.getPropertyStorage().getPropertyValue(LuceneRecordContextProperties.LUCENE_MERGE_SEGMENTS_PER_TIER)); + tieredMergePolicy.setNoCFSRatio(1.00); + IndexWriterConfig indexWriterConfig = new IndexWriterConfig(writerAnalyzer.getAnalyzer()) + .setUseCompoundFile(USE_COMPOUND_FILE) + .setMergePolicy(tieredMergePolicy) + .setMergeScheduler(getMergeScheduler(state, mergeDirectoryCount, agilityContext, key)) + .setCodec(CODEC) + .setInfoStream(new LuceneLoggerInfoStream(LOGGER)); - if (writerReader != null) { - writerReader.close(); - writerReader = null; - } + // Merge is required when creating an index writer (do we have a better indicator for a required merge?) + mergeControl.setMergeRequiredIndexes(state.index); - // Merge is required when creating an index writer (do we have a better indicator for a required merge?) - mergeControl.setMergeRequiredIndexes(state.index); - } - } - } - return writer; + return new IndexWriter(directory, indexWriterConfig); } @Override @SuppressWarnings("PMD.CloseResource") public synchronized void close() throws IOException { - IOUtils.close(writer, writerReader, directory); - writer = null; + IOUtils.close(writerOpener, writerReader, directory); writerReader = null; } public void mergeIndex(final Exception exceptionAtCreation) throws IOException { getWriter(exceptionAtCreation).maybeMerge(); } - - protected @Nonnull FDBDirectory createFDBDirectory(final Subspace subspace, - final Map options, - final FDBDirectorySharedCacheManager sharedCacheManager, - final Tuple sharedCacheKey, - final boolean useCompoundFile, final AgilityContext agilityContext, - final int blockCacheMaximumSize) { - return new FDBDirectory(subspace, options, sharedCacheManager, sharedCacheKey, useCompoundFile, agilityContext, blockCacheMaximumSize); - } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java index 7926a1a43c..b1e190cbbc 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java @@ -735,7 +735,7 @@ void mergeLosesLockTest(int failurePercentage) throws IOException { FDBDirectory fdbDirectory = new InvalidLockTestFDBDirectory(recordStore.indexSubspace(index).subspace(directoryKey), context, options, failurePercentage); FDBDirectoryWrapper fdbDirectoryWrapper = new FDBDirectoryWrapper(state, fdbDirectory, directoryKey, 1, AgilityContext.agile(context, 1L, 1L), - indexAnalyzerSelector.provideIndexAnalyzer()); + indexAnalyzerSelector.provideIndexAnalyzer(), new Exception()); assertThrows(IOException.class, () -> fdbDirectoryWrapper.mergeIndex(new Exception()), "invalid lock"); diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java index 30c0f56c14..10370e2d83 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryManager.java @@ -40,6 +40,6 @@ protected FDBDirectoryWrapper createNewDirectoryWrapper(final IndexMaintainerSta final AgilityContext agilityContext, final int blockCacheMaximumSize) { return new MockedFDBDirectoryWrapper(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, - writerAnalyzer); + writerAnalyzer, exceptionAtCreation); } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java index db82aa5477..b7d1e952b6 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/MockedFDBDirectoryWrapper.java @@ -22,25 +22,20 @@ import com.apple.foundationdb.record.lucene.LuceneAnalyzerWrapper; import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; -import com.apple.foundationdb.subspace.Subspace; import com.apple.foundationdb.tuple.Tuple; -import javax.annotation.Nonnull; -import java.util.Map; - /** * A Testing-focused {@link FDBDirectoryWrapper} that allows a mocked-FDBDirectory to be injected into the system. */ public class MockedFDBDirectoryWrapper extends FDBDirectoryWrapper { MockedFDBDirectoryWrapper(final IndexMaintainerState state, final Tuple key, final int mergeDirectoryCount, final AgilityContext agilityContext, final int blockCacheMaximumSize, - final LuceneAnalyzerWrapper writerAnalyzer) { - super(state, key, mergeDirectoryCount, agilityContext, blockCacheMaximumSize, writerAnalyzer); - } - - @Nonnull - @Override - protected FDBDirectory createFDBDirectory(final Subspace subspace, final Map options, final FDBDirectorySharedCacheManager sharedCacheManager, final Tuple sharedCacheKey, final boolean useCompoundFile, final AgilityContext agilityContext, final int blockCacheMaximumSize) { - return new MockedFDBDirectory(subspace, options, sharedCacheManager, sharedCacheKey, useCompoundFile, agilityContext, blockCacheMaximumSize); + final LuceneAnalyzerWrapper writerAnalyzer, final Exception exceptionAtManagerCreation) { + super(state, + new MockedFDBDirectory(state.indexSubspace.subspace(key), state.index.getOptions(), + null, null, // TODO support shared cache + USE_COMPOUND_FILE, agilityContext, blockCacheMaximumSize), + key, mergeDirectoryCount, agilityContext, writerAnalyzer, + exceptionAtManagerCreation); } } From 648f78fc7eb76af5b489fe7ce7bcd9d1af6a4e3d Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Wed, 4 Dec 2024 16:44:16 -0500 Subject: [PATCH 8/9] LuceneIndexTestValidator: support for unpartitioned indexes --- .../lucene/LuceneIndexTestValidator.java | 141 +++++++++++------- 1 file changed, 91 insertions(+), 50 deletions(-) diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java index 29b8dc285b..0a420add9a 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexTestValidator.java @@ -59,6 +59,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -112,11 +113,8 @@ void validate(Index index, final Map> expectedDocumentI */ void validate(Index index, final Map> expectedDocumentInformation, final String universalSearch, final boolean allowDuplicatePrimaryKeys) throws IOException { - final int partitionHighWatermark = Integer.parseInt(index.getOption(LuceneIndexOptions.INDEX_PARTITION_HIGH_WATERMARK)); - String partitionLowWaterMarkStr = index.getOption(LuceneIndexOptions.INDEX_PARTITION_LOW_WATERMARK); - final int partitionLowWatermark = partitionLowWaterMarkStr == null ? - Math.max(LucenePartitioner.DEFAULT_PARTITION_LOW_WATERMARK, 1) : - Integer.parseInt(partitionLowWaterMarkStr); + final int partitionHighWatermark = getPartitionHighWatermark(index); + final int partitionLowWatermark = getPartitionLowWatermark(index); Map> missingDocuments = new HashMap<>(); expectedDocumentInformation.forEach((groupingKey, groupedIds) -> { @@ -132,56 +130,98 @@ void validate(Index index, final Map> expectedDocumentI .sorted(Map.Entry.comparingByValue()) .map(Map.Entry::getKey) .collect(Collectors.toList()); - List partitionInfos = getPartitionMeta(index, groupingKey); - partitionInfos.sort(Comparator.comparing(info -> Tuple.fromBytes(info.getFrom().toByteArray()))); - final String allCounts = partitionInfos.stream() - .map(info -> Tuple.fromBytes(info.getFrom().toByteArray()).toString() + info.getCount()) - .collect(Collectors.joining(",", "[", "]")); - Set usedPartitionIds = new HashSet<>(); - Tuple lastToTuple = null; - int visitedCount = 0; + if (partitionHighWatermark > 0) { + validatePartitionedGroup(index, universalSearch, allowDuplicatePrimaryKeys, groupingKey, partitionLowWatermark, partitionHighWatermark, records, missingDocuments); + } else { + validateUnpartitionedGroup(index, universalSearch, allowDuplicatePrimaryKeys, groupingKey, partitionLowWatermark, partitionHighWatermark, records, missingDocuments); + } + } + missingDocuments.entrySet().removeIf(entry -> entry.getValue().isEmpty()); + assertEquals(Map.of(), missingDocuments, "We should have found all documents in the index"); + } - try (FDBRecordContext context = contextProvider.get()) { - final FDBRecordStore recordStore = schemaSetup.apply(context); + private void validateUnpartitionedGroup(final Index index, final String universalSearch, final boolean allowDuplicatePrimaryKeys, final Tuple groupingKey, final int partitionLowWatermark, final int partitionHighWatermark, final List records, final Map> missingDocuments) throws IOException { + try (FDBRecordContext context = contextProvider.get()) { + final FDBRecordStore recordStore = schemaSetup.apply(context); + LOGGER.debug(KeyValueLogMessage.of("Visiting group", + "group", groupingKey, + "documentsInGroup", records.size())); + validateDocsInPartition(recordStore, index, null, groupingKey, Set.copyOf(records), universalSearch); + validatePrimaryKeySegmentIndex(recordStore, index, groupingKey, null, + Set.copyOf(records), allowDuplicatePrimaryKeys); + Set.copyOf(records).forEach(primaryKey -> missingDocuments.get(groupingKey).remove(primaryKey)); + } + } - for (int i = 0; i < partitionInfos.size(); i++) { - final LucenePartitionInfoProto.LucenePartitionInfo partitionInfo = partitionInfos.get(i); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Group: " + groupingKey + " PartitionInfo[" + partitionInfo.getId() + - "]: count:" + partitionInfo.getCount() + " " + - Tuple.fromBytes(partitionInfo.getFrom().toByteArray()) + "-> " + - Tuple.fromBytes(partitionInfo.getTo().toByteArray())); - } + private void validatePartitionedGroup(final Index index, final String universalSearch, final boolean allowDuplicatePrimaryKeys, final Tuple groupingKey, final int partitionLowWatermark, final int partitionHighWatermark, final List records, final Map> missingDocuments) throws IOException { + List partitionInfos = getPartitionMeta(index, groupingKey); + partitionInfos.sort(Comparator.comparing(info -> Tuple.fromBytes(info.getFrom().toByteArray()))); + final String allCounts = partitionInfos.stream() + .map(info -> Tuple.fromBytes(info.getFrom().toByteArray()).toString() + info.getCount()) + .collect(Collectors.joining(",", "[", "]")); + Set usedPartitionIds = new HashSet<>(); + Tuple lastToTuple = null; + int visitedCount = 0; - assertTrue(isParititionCountWithinBounds(partitionInfos, i, partitionLowWatermark, partitionHighWatermark), - "Group: " + groupingKey + " - " + allCounts + "\nlowWatermark: " + partitionLowWatermark + ", highWatermark: " + partitionHighWatermark + - "\nCurrent count: " + partitionInfo.getCount()); - assertTrue(usedPartitionIds.add(partitionInfo.getId()), () -> "Duplicate id: " + partitionInfo); - final Tuple fromTuple = Tuple.fromBytes(partitionInfo.getFrom().toByteArray()); - if (i > 0) { - assertThat(fromTuple, greaterThan(lastToTuple)); - } - lastToTuple = Tuple.fromBytes(partitionInfo.getTo().toByteArray()); - assertThat(fromTuple, lessThanOrEqualTo(lastToTuple)); + try (FDBRecordContext context = contextProvider.get()) { + final FDBRecordStore recordStore = schemaSetup.apply(context); + for (int i = 0; i < partitionInfos.size(); i++) { + final LucenePartitionInfoProto.LucenePartitionInfo partitionInfo = partitionInfos.get(i); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("Group: " + groupingKey + " PartitionInfo[" + partitionInfo.getId() + + "]: count:" + partitionInfo.getCount() + " " + + Tuple.fromBytes(partitionInfo.getFrom().toByteArray()) + "-> " + + Tuple.fromBytes(partitionInfo.getTo().toByteArray())); + } - LOGGER.debug(KeyValueLogMessage.of("Visited partition", - "group", groupingKey, - "documentsSoFar", visitedCount, - "documentsInGroup", records.size(), - "partitionInfo.count", partitionInfo.getCount())); - final Set expectedPrimaryKeys = Set.copyOf(records.subList(visitedCount, visitedCount + partitionInfo.getCount())); - validateDocsInPartition(recordStore, index, partitionInfo.getId(), groupingKey, - expectedPrimaryKeys, - universalSearch); - visitedCount += partitionInfo.getCount(); - validatePrimaryKeySegmentIndex(recordStore, index, groupingKey, partitionInfo.getId(), - expectedPrimaryKeys, allowDuplicatePrimaryKeys); - expectedPrimaryKeys.forEach(primaryKey -> missingDocuments.get(groupingKey).remove(primaryKey)); + assertTrue(isParititionCountWithinBounds(partitionInfos, i, partitionLowWatermark, partitionHighWatermark), + "Group: " + groupingKey + " - " + allCounts + "\nlowWatermark: " + partitionLowWatermark + ", highWatermark: " + partitionHighWatermark + + "\nCurrent count: " + partitionInfo.getCount()); + assertTrue(usedPartitionIds.add(partitionInfo.getId()), () -> "Duplicate id: " + partitionInfo); + final Tuple fromTuple = Tuple.fromBytes(partitionInfo.getFrom().toByteArray()); + if (i > 0) { + assertThat(fromTuple, greaterThan(lastToTuple)); } + lastToTuple = Tuple.fromBytes(partitionInfo.getTo().toByteArray()); + assertThat(fromTuple, lessThanOrEqualTo(lastToTuple)); + + LOGGER.debug(KeyValueLogMessage.of("Visited partition", + "group", groupingKey, + "documentsSoFar", visitedCount, + "documentsInGroup", records.size(), + "partitionInfo.count", partitionInfo.getCount())); + // if partitionInfo.getCount() is wrong, this can be very confusing, so a different assertion might be + // worthwhile + final Set expectedPrimaryKeys = Set.copyOf(records.subList(visitedCount, + Math.min(records.size() - 1, visitedCount + partitionInfo.getCount()))); + validateDocsInPartition(recordStore, index, partitionInfo.getId(), groupingKey, + expectedPrimaryKeys, + universalSearch); + visitedCount += partitionInfo.getCount(); + assertThat(records.size(), greaterThanOrEqualTo(visitedCount)); + validatePrimaryKeySegmentIndex(recordStore, index, groupingKey, partitionInfo.getId(), + expectedPrimaryKeys, allowDuplicatePrimaryKeys); + expectedPrimaryKeys.forEach(primaryKey -> missingDocuments.get(groupingKey).remove(primaryKey)); } } - missingDocuments.entrySet().removeIf(entry -> entry.getValue().isEmpty()); - assertEquals(Map.of(), missingDocuments, "We should have found all documents in the index"); + } + + private static int getPartitionLowWatermark(final Index index) { + String partitionLowWaterMarkStr = index.getOption(LuceneIndexOptions.INDEX_PARTITION_LOW_WATERMARK); + if (partitionLowWaterMarkStr == null) { + return Math.max(LucenePartitioner.DEFAULT_PARTITION_LOW_WATERMARK, 1); + } else { + return Integer.parseInt(partitionLowWaterMarkStr); + } + } + + private static int getPartitionHighWatermark(final Index index) { + final String option = index.getOption(LuceneIndexOptions.INDEX_PARTITION_HIGH_WATERMARK); + if (option == null) { + return -1; + } else { + return Integer.parseInt(option); + } } List getPartitionMeta(Index index, @@ -215,7 +255,8 @@ int getPartitionExtraCapacity(int count, int highWatermark) { return Math.max(0, highWatermark - count); } - public static void validateDocsInPartition(final FDBRecordStore recordStore, Index index, int partitionId, Tuple groupingKey, + public static void validateDocsInPartition(final FDBRecordStore recordStore, Index index, + @Nullable Integer partitionId, Tuple groupingKey, Set expectedPrimaryKeys, final String universalSearch) throws IOException { LuceneScanQuery scanQuery; if (groupingKey.isEmpty()) { @@ -251,7 +292,7 @@ public static void validateDocsInPartition(final FDBRecordStore recordStore, Ind } public static IndexReader getIndexReader(final FDBRecordStore recordStore, final Index index, - final Tuple groupingKey, final int partitionId) throws IOException { + final Tuple groupingKey, @Nullable final Integer partitionId) throws IOException { final FDBDirectoryManager manager = getDirectoryManager(recordStore, index); return manager.getIndexReader(groupingKey, partitionId); } From 4fbdc797a7af090b6d0fbbf9eb5e74e15fb59596 Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Wed, 4 Dec 2024 16:44:40 -0500 Subject: [PATCH 9/9] Get LuceneIndexMaintenanceTest.concurrentUpdate to pass I had to disable: - synthetic indexes, because updating the parent and child at the same time breaks the primaryKeySegmentIndex - partitioning because the partitionInfo (including counts) is not updated in a thread-safe way --- .../record/lucene/LuceneIndexMaintenanceTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java index b1e190cbbc..0444ee5cd4 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/LuceneIndexMaintenanceTest.java @@ -798,9 +798,12 @@ void concurrentUpdate() throws IOException { null, false)); final long seed = 320947L; final boolean isGrouped = true; - final boolean isSynthetic = true; + // updating the parent & child concurrently is not thread safe, we may want to fix the behavior, or say that is + // not supported, as it is the same as updating the same record concurrently, which I don't think is generally + // supported. + final boolean isSynthetic = false; final boolean primaryKeySegmentIndexEnabled = true; - final int partitionHighWatermark = 100_000; + final int partitionHighWatermark = -1; // LucenePartitioner is not thread safe, and the counts get broken final LuceneIndexTestDataModel dataModel = new LuceneIndexTestDataModel.Builder(seed, this::getStoreBuilder, pathManager) .setIsGrouped(isGrouped) .setIsSynthetic(isSynthetic)