From 21ccba7c3db37461ef6319afa5344690d1f9131e Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 9 Jul 2026 11:49:47 -0400 Subject: [PATCH 1/5] initial implementation --- .../queue/PendingWritesQueue.java | 119 ++++-- .../queue/PendingWritesQueueEntry.java | 15 +- .../PendingWritesQueueLegacyFallbackTest.java | 195 ++++++++++ .../record/lucene/LuceneIndexMaintainer.java | 11 +- .../lucene/LuceneRecordContextProperties.java | 8 + .../record/lucene/directory/FDBDirectory.java | 47 ++- .../lucene/directory/FDBDirectoryManager.java | 4 +- .../lucene/directory/FDBDirectoryWrapper.java | 135 ++++++- .../directory/PendingWritesQueueHelper.java | 90 +++++ .../lucene/FDBLuceneQueuedDocQueryTest.java | 4 +- .../lucene/LuceneIndexMaintenanceTest.java | 4 +- .../lucene/directory/FDBDirectoryTest.java | 9 +- ...dingWriteQueueFormatCompatibilityTest.java | 342 ++++++++++++++++++ .../PendingWriteQueueIntegrationTest.java | 146 +++++++- .../PendingWriteQueueSizeIntegrationTest.java | 12 +- 15 files changed, 1065 insertions(+), 76 deletions(-) create mode 100644 fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java create mode 100644 fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueFormatCompatibilityTest.java diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java index a78696e9cc..51ca103f4f 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java @@ -53,6 +53,7 @@ import java.nio.ByteOrder; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.function.Function; /** * A persistent FDB-backed queue of pending entries, each carrying a typed Protobuf payload. @@ -126,9 +127,19 @@ public class PendingWritesQueue { private final long maxQueueSize; @Nonnull private final Class payloadClass; + /** + * Optional fallback used on read when an on-disk entry is not in the current envelope format + * (i.e. it has no {@code version >= 1}). It receives the raw stored value bytes and returns + * the payload {@code T}. This exists to read entries written by a predecessor queue + * implementation whose value layout differs from {@link PendingWritesQueueProto.PendingWriteItem} + * (for example, entries wrapped by a caller-specific serializer). {@code null} when there is + * no legacy format to support, in which case a non-current entry is a storage error. + */ + @Nullable + private final Function legacyDecoder; /** - * Construct a pending writes queue. + * Construct a pending writes queue with no legacy-format support. * * @param queueSubspace subspace under which queue entries live; the caller owns its layout * @param queueSizeSubspace subspace under which the atomic size counter lives; must not @@ -143,10 +154,35 @@ public PendingWritesQueue(@Nonnull Subspace queueSubspace, @Nonnull Subspace queueSizeSubspace, long maxQueueSize, @Nonnull Class payloadClass) { + this(queueSubspace, queueSizeSubspace, maxQueueSize, payloadClass, null); + } + + /** + * Construct a pending writes queue that can also read entries written by a predecessor + * implementation via a legacy decoder. + * + * @param queueSubspace subspace under which queue entries live; the caller owns its layout + * @param queueSizeSubspace subspace under which the atomic size counter lives; must not + * overlap with {@code queueSubspace} + * @param maxQueueSize maximum number of entries allowed in the queue; {@link #enqueue} + * rejects further entries when the counter reaches this value. Pass {@code 0} to disable + * the cap. + * @param payloadClass class of the payload message type, e.g. {@code MyPayload.class}; used + * to verify and unpack the payload on read + * @param legacyDecoder fallback that turns the raw stored bytes of a non-current-format entry + * into a payload {@code T}; {@code null} if there is no legacy format to read. The queue only + * ever writes the current format; the decoder is used on the read path. + */ + public PendingWritesQueue(@Nonnull Subspace queueSubspace, + @Nonnull Subspace queueSizeSubspace, + long maxQueueSize, + @Nonnull Class payloadClass, + @Nullable Function legacyDecoder) { this.queueSubspace = queueSubspace; this.queueSizeSubspace = queueSizeSubspace; this.maxQueueSize = maxQueueSize; this.payloadClass = payloadClass; + this.legacyDecoder = legacyDecoder; } /** @@ -331,38 +367,67 @@ private CompletableFuture capacityCheck(@Nonnull FDBRecordContext context) @Nonnull private PendingWritesQueueEntry toQueueEntry(@Nonnull Tuple keyTuple, @Nonnull byte[] valueBytes) { - PendingWritesQueueProto.PendingWriteItem item; - try { - item = PendingWritesQueueProto.PendingWriteItem.parseFrom(valueBytes); - } catch (InvalidProtocolBufferException ex) { - throw new RecordCoreStorageException("Failed to parse pending writes queue entry", ex) - .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); - } - if (item.getVersion() > CURRENT_VERSION) { - throw new RecordCoreStorageException("Pending writes queue entry version is newer than this reader supports") - .addLogInfo(LogMessageKeys.VERSION, CURRENT_VERSION) - .addLogInfo(LogMessageKeys.STORED_VERSION, item.getVersion()) - .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); + final PendingWritesQueueProto.PendingWriteItem item = tryParseEnvelope(valueBytes); + if (item != null && item.getVersion() > 0) { + // Current format: a versioned envelope carrying an Any payload. + if (item.getVersion() > CURRENT_VERSION) { + throw new RecordCoreStorageException("Pending writes queue entry version is newer than this reader supports") + .addLogInfo(LogMessageKeys.VERSION, CURRENT_VERSION) + .addLogInfo(LogMessageKeys.STORED_VERSION, item.getVersion()) + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); + } + final Any storedPayload = item.getPayload(); + // Any#is derives the expected type URL from the bound message class internally and + // compares it to the stored URL. If it doesn't match, we surface both URLs for + // diagnostics before unpacking would itself throw. + if (!storedPayload.is(payloadClass)) { + throw new RecordCoreStorageException("Pending writes queue entry payload type does not match the queue's bound type") + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) + .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()) + .addLogInfo(LogMessageKeys.ACTUAL_TYPE, storedPayload.getTypeUrl()); + } + final T payload; + try { + payload = storedPayload.unpack(payloadClass); + } catch (InvalidProtocolBufferException ex) { + throw new RecordCoreStorageException("Failed to unpack pending writes queue entry payload", ex) + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) + .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()); + } + return new PendingWritesQueueEntry<>(keyTuple, payload, storedPayload.getTypeUrl(), item.getEnqueueTimestamp()); } - Any storedPayload = item.getPayload(); - // Any#is derives the expected type URL from the bound message class internally and - // compares it to the stored URL. If it doesn't match, we surface both URLs for - // diagnostics before unpacking would itself throw. - if (!storedPayload.is(payloadClass)) { - throw new RecordCoreStorageException("Pending writes queue entry payload type does not match the queue's bound type") + + // Not the current format (parse failed, or the entry has no version). If a legacy decoder + // is configured, hand it the raw stored bytes; otherwise this is a storage error. + if (legacyDecoder == null) { + throw new RecordCoreStorageException("Failed to parse pending writes queue entry and no legacy decoder is configured") .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) - .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()) - .addLogInfo(LogMessageKeys.ACTUAL_TYPE, storedPayload.getTypeUrl()); + .addLogInfo(LogMessageKeys.STORED_VERSION, item == null ? -1 : item.getVersion()); } - T payload; + final T payload; try { - payload = storedPayload.unpack(payloadClass); + payload = legacyDecoder.apply(valueBytes); + } catch (RuntimeException ex) { + throw new RecordCoreStorageException("Failed to decode legacy pending writes queue entry", ex) + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); + } + // Legacy entries were not Any-wrapped and carry no envelope timestamp, so the payload + // type URL is empty and the enqueue timestamp is unavailable at this layer. + return new PendingWritesQueueEntry<>(keyTuple, payload, "", 0L); + } + + /** + * Parse the stored bytes as the current envelope, returning {@code null} if they are not a + * valid envelope (e.g. an entry written by a predecessor implementation in a different + * layout, which the {@link #legacyDecoder} handles instead). + */ + @Nullable + private static PendingWritesQueueProto.PendingWriteItem tryParseEnvelope(@Nonnull byte[] valueBytes) { + try { + return PendingWritesQueueProto.PendingWriteItem.parseFrom(valueBytes); } catch (InvalidProtocolBufferException ex) { - throw new RecordCoreStorageException("Failed to unpack pending writes queue entry payload", ex) - .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) - .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()); + return null; } - return new PendingWritesQueueEntry<>(keyTuple, payload, storedPayload.getTypeUrl(), item.getEnqueueTimestamp()); } private void mutateQueueSizeCounter(@Nonnull FDBRecordContext context, long delta) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java index bdb4970f73..90a4294dd4 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java @@ -50,8 +50,11 @@ public final class PendingWritesQueueEntry { @Nonnull T payload, @Nonnull String payloadTypeUrl, long enqueueTimestamp) { - // Sanity-check the key shape — the queue always writes (incarnation, versionstamp). - if (keyTuple.size() != 2) { + // Sanity-check the key shape. The queue itself always writes (incarnation, versionstamp) + // (size 2). Reads additionally tolerate a bare (versionstamp) key (size 1) so that entries + // written by a predecessor queue that did not prefix an incarnation can still be read back + // — see the legacy-decoder path on the read side. + if (keyTuple.size() != 1 && keyTuple.size() != 2) { throw new RecordCoreStorageException("Unexpected queue key shape") .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); } @@ -66,8 +69,14 @@ public Tuple getKeyTuple() { return keyTuple; } + /** + * Returns the incarnation prefix of this entry's key, or {@code 0} when the key carries no + * incarnation (a legacy, size-1 key). + * + * @return the incarnation, or {@code 0} for an incarnation-less legacy key + */ public int getIncarnation() { - return (int)keyTuple.getLong(0); + return keyTuple.size() == 2 ? (int)keyTuple.getLong(0) : 0; } public long getEnqueueTimestamp() { diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java new file mode 100644 index 0000000000..d40dd370e9 --- /dev/null +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java @@ -0,0 +1,195 @@ +/* + * PendingWritesQueueLegacyFallbackTest.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-2026 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.apple.foundationdb.record.provider.foundationdb.queue; + +import com.apple.foundationdb.MutationType; +import com.apple.foundationdb.record.PendingWritesQueueTestProto.OtherTestQueuePayload; +import com.apple.foundationdb.record.PendingWritesQueueTestProto.TestQueuePayload; +import com.apple.foundationdb.record.RecordCoreStorageException; +import com.apple.foundationdb.record.ScanProperties; +import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; +import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreTestBase; +import com.apple.foundationdb.record.provider.foundationdb.FDBRecordVersion; +import com.apple.foundationdb.record.provider.foundationdb.SplitHelper; +import com.apple.foundationdb.subspace.Subspace; +import com.apple.foundationdb.tuple.Tuple; +import com.google.protobuf.InvalidProtocolBufferException; +import com.apple.test.Tags; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.util.Arrays; +import java.util.List; +import java.util.function.Function; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Tests the {@link PendingWritesQueue} legacy-decoder fallback: a queue can read a mix of + * current-format entries (a versioned envelope with an {@code Any} payload, written by + * {@link PendingWritesQueue#enqueue}) and "legacy" entries written by a predecessor + * implementation in a different value layout. On read, an entry that is not the current format + * (parse fails, or it has no {@code version >= 1}) is handed to the legacy decoder. + * + *

Here the simulated legacy layout is a bare {@link OtherTestQueuePayload} (optionally wrapped + * with a one-byte prefix to stand in for a caller-specific serializer); the decoder maps it to a + * {@link TestQueuePayload}, so all entries surface as the queue's bound payload type.

+ */ +@Tag(Tags.RequiresFDB) +class PendingWritesQueueLegacyFallbackTest extends FDBRecordStoreTestBase { + + /** Legacy layout: a bare {@link OtherTestQueuePayload}. Decoder maps {@code text -> label}. */ + private static final Function BARE_LEGACY_DECODER = raw -> { + try { + return TestQueuePayload.newBuilder().setLabel(OtherTestQueuePayload.parseFrom(raw).getText()).build(); + } catch (InvalidProtocolBufferException e) { + throw new IllegalStateException(e); + } + }; + + /** Legacy layout: a one-byte {@code 0x00} prefix (a stand-in serializer) + {@link OtherTestQueuePayload}. */ + private static final Function PREFIXED_LEGACY_DECODER = raw -> { + try { + byte[] unwrapped = Arrays.copyOfRange(raw, 1, raw.length); + return TestQueuePayload.newBuilder().setLabel(OtherTestQueuePayload.parseFrom(unwrapped).getText()).build(); + } catch (InvalidProtocolBufferException e) { + throw new IllegalStateException(e); + } + }; + + /** + * A queue reads current-format and legacy (version-0, still-parseable) entries interleaved, + * in versionstamp order, surfacing each as the bound payload type. + */ + @Test + void readsMixOfNewAndLegacyEntries() { + PendingWritesQueue queue; + try (FDBRecordContext context = openContext()) { + queue = getQueue(context, BARE_LEGACY_DECODER); + queue.enqueue(context, payload("new-0"), 0).join(); + writeRawLegacyEntry(context, bareLegacy("legacy-1")); + queue.enqueue(context, payload("new-2"), 0).join(); + commit(context); + } + + List> entries = readAll(queue); + assertEquals(3, entries.size()); + assertEquals("new-0", entries.get(0).getPayload().getLabel()); + assertEquals("legacy-1", entries.get(1).getPayload().getLabel()); // via the legacy decoder + assertEquals("new-2", entries.get(2).getPayload().getLabel()); + } + + /** + * A legacy entry whose bytes do not even parse as the envelope (the stand-in serializer + * prefix is an invalid protobuf tag) still routes to the legacy decoder. + */ + @Test + void legacyEntryThatFailsEnvelopeParseUsesDecoder() { + PendingWritesQueue queue; + try (FDBRecordContext context = openContext()) { + queue = getQueue(context, PREFIXED_LEGACY_DECODER); + writeRawLegacyEntry(context, prefixedLegacy("wrapped-legacy")); + queue.enqueue(context, payload("new"), 0).join(); + commit(context); + } + + List> entries = readAll(queue); + assertEquals(2, entries.size()); + assertEquals("wrapped-legacy", entries.get(0).getPayload().getLabel()); + assertEquals("new", entries.get(1).getPayload().getLabel()); + } + + /** + * A non-current-format entry with no legacy decoder configured is a storage error. + */ + @Test + void nonCurrentEntryWithoutLegacyDecoderThrows() { + PendingWritesQueue queue; + try (FDBRecordContext context = openContext()) { + queue = getQueue(context, null); + writeRawLegacyEntry(context, bareLegacy("orphan")); + commit(context); + } + + try (FDBRecordContext context = openContext()) { + Assertions.assertThatThrownBy( + () -> queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null).asList().join()) + .hasCauseInstanceOf(RecordCoreStorageException.class); + } + } + + private void writeRawLegacyEntry(@Nonnull FDBRecordContext context, @Nonnull byte[] rawValue) { + Subspace queueSubspace = queueSubspaceFor(context); + FDBRecordVersion recordVersion = FDBRecordVersion.incomplete(context.claimLocalVersion()); + Tuple keyTuple = Tuple.from(0, recordVersion.toVersionstamp()); + SplitHelper.saveWithSplit(context, queueSubspace, keyTuple, rawValue, null, true, false, false, null, null); + // Keep the size counter consistent with the entry we wrote directly. + context.ensureActive().mutate(MutationType.ADD, counterSubspaceFor(context).pack(), + new byte[] {1, 0, 0, 0, 0, 0, 0, 0}); + } + + @Nonnull + private static byte[] bareLegacy(@Nonnull String text) { + return OtherTestQueuePayload.newBuilder().setText(text).build().toByteArray(); + } + + @Nonnull + private static byte[] prefixedLegacy(@Nonnull String text) { + byte[] bare = bareLegacy(text); + byte[] prefixed = new byte[bare.length + 1]; + prefixed[0] = 0x00; // an invalid protobuf tag, so the envelope parse fails + System.arraycopy(bare, 0, prefixed, 1, bare.length); + return prefixed; + } + + @Nonnull + private PendingWritesQueue getQueue(@Nonnull FDBRecordContext context, + @Nullable Function legacyDecoder) { + return new PendingWritesQueue<>(queueSubspaceFor(context), counterSubspaceFor(context), 0, + TestQueuePayload.class, legacyDecoder); + } + + @Nonnull + private Subspace queueSubspaceFor(@Nonnull FDBRecordContext context) { + return path.toSubspace(context).subspace(Tuple.from("queue")); + } + + @Nonnull + private Subspace counterSubspaceFor(@Nonnull FDBRecordContext context) { + return path.toSubspace(context).subspace(Tuple.from("counter")); + } + + @Nonnull + private List> readAll( + @Nonnull PendingWritesQueue queue) { + try (FDBRecordContext context = openContext()) { + return queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null).asList().join(); + } + } + + @Nonnull + private static TestQueuePayload payload(@Nonnull String label) { + return TestQueuePayload.newBuilder().setLabel(label).build(); + } +} 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 72dbffdf73..b057b3b847 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 @@ -39,7 +39,6 @@ import com.apple.foundationdb.record.lucene.directory.FDBDirectory; import com.apple.foundationdb.record.lucene.directory.FDBDirectoryManager; import com.apple.foundationdb.record.lucene.directory.FDBLuceneFileReference; -import com.apple.foundationdb.record.lucene.directory.PendingWriteQueue; import com.apple.foundationdb.record.lucene.idformat.LuceneIndexKeySerializer; import com.apple.foundationdb.record.metadata.Index; import com.apple.foundationdb.record.metadata.IndexAggregateFunction; @@ -178,8 +177,8 @@ public RecordCursor scan(@Nonnull final IndexScanBounds scanBounds, private void writeDocument(final FDBIndexableRecord newRecord, final Map.Entry> entry, final Integer partitionId) { if (shouldUseQueue(entry.getKey(), partitionId)) { - PendingWriteQueue queue = directoryManager.getPendingWriteQueue(entry.getKey(), partitionId); - queue.enqueueInsert(state.store.getContext(), newRecord.getPrimaryKey(), entry.getValue(), getIncarnationSafe(state.store)); + directoryManager.getDirectoryWrapper(entry.getKey(), partitionId).enqueuePendingInsert( + state.store.getContext(), newRecord.getPrimaryKey(), entry.getValue(), getIncarnationSafe(state.store)); // Require deferred merge (+ drain) in case there is a merge indicator without an active merge this.state.store.getIndexDeferredMaintenanceControl().setMergeRequiredIndexes(this.state.index); } else { @@ -210,8 +209,8 @@ public void writeDocumentBypassQueue(Tuple groupingKey, private int deleteDocument(Tuple groupingKey, @Nullable Integer partitionId, Tuple primaryKey) throws IOException { if (shouldUseQueue(groupingKey, partitionId)) { - PendingWriteQueue queue = directoryManager.getPendingWriteQueue(groupingKey, partitionId); - queue.enqueueDelete(state.store.getContext(), primaryKey, getIncarnationSafe(state.store)); + directoryManager.getDirectoryWrapper(groupingKey, partitionId).enqueuePendingDelete( + state.store.getContext(), primaryKey, getIncarnationSafe(state.store)); // Require deferred merge (+ drain) in case there is a merge indicator without an active merge this.state.store.getIndexDeferredMaintenanceControl().setMergeRequiredIndexes(this.state.index); return 0; // partition count will be adjusted during drain @@ -589,7 +588,7 @@ private CompletableFuture getLuceneInfo(final Tup directory.getFieldInfosCount(); final CompletableFuture queueSizeFuture = directoryManager.getPendingWriteQueue(groupingKey, partitionId) - .getQueueSize(state.context); + .getQueueSizeNoConflict(state.context); return filesFuture.thenCompose(fileList -> fieldInfosFuture.thenCompose(fieldInfosCount -> queueSizeFuture.thenApply(queueSize -> diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneRecordContextProperties.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneRecordContextProperties.java index 5826cae26d..305fdfded4 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneRecordContextProperties.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneRecordContextProperties.java @@ -142,4 +142,12 @@ public final class LuceneRecordContextProperties { * Max size of pending writes queue. */ public static final RecordLayerPropertyKey LUCENE_MAX_PENDING_QUEUE_SIZE = RecordLayerPropertyKey.integerPropertyKey("com.apple.foundationdb.record.lucene.max.pending.queue.size", PendingWriteQueue.DEFAULT_MAX_PENDING_QUEUE_SIZE); + /** + * Whether to write pending-write-queue entries in the new (versioned {@code Any}-payload) + * format instead of the legacy format. Readers always understand both formats; this only + * controls what is written. It must be turned on only after every instance in the deployment + * runs a version that can read the new format, so no older instance encounters an entry it + * cannot parse. Default: {@code false} (write the legacy format). + */ + public static final RecordLayerPropertyKey LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT = RecordLayerPropertyKey.booleanPropertyKey("com.apple.foundationdb.record.lucene.pending.writes.queue.write.new.format", false); } diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java index e9f1c3caeb..9f3b46ac8d 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java @@ -38,6 +38,7 @@ import com.apple.foundationdb.record.lucene.LuceneIndexOptions; import com.apple.foundationdb.record.lucene.LuceneIndexTypes; import com.apple.foundationdb.record.lucene.LuceneLogMessageKeys; +import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; import com.apple.foundationdb.record.lucene.LucenePrimaryKeySegmentIndex; import com.apple.foundationdb.record.lucene.LucenePrimaryKeySegmentIndexV1; import com.apple.foundationdb.record.lucene.LucenePrimaryKeySegmentIndexV2; @@ -47,6 +48,7 @@ import com.apple.foundationdb.record.lucene.codec.PrefetchableBufferedChecksumIndexInput; import com.apple.foundationdb.record.provider.common.StoreTimer; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; import com.apple.foundationdb.record.util.pair.ComparablePair; import com.apple.foundationdb.record.util.pair.NonnullPair; import com.apple.foundationdb.subspace.Subspace; @@ -1051,11 +1053,54 @@ public CompletableFuture clearOngoingMergeIndicatorIfQueueEmptyAsync() { })); } - public PendingWriteQueue createPendingWritesQueue() { + /** + * Create the pending-writes queue used for reading the queue (drain, replay, size, + * emptiness) and for writing new-format entries. This is the generic + * {@link PendingWritesQueue} carrying a {@link LucenePendingWriteQueueProto.PendingWriteItem} + * payload. It reads both the new (versioned {@code Any}-payload) format and the legacy + * (serializer-wrapped) format — the latter via a legacy decoder. + * + * @return the generic pending-writes queue bound to the Lucene payload type + */ + public PendingWritesQueue createPendingWritesQueue() { + return new PendingWritesQueue<>( + pendingWritesQueueSubspace, pendingQueueSizeSubspace, maxPendingQueueSize, + LucenePendingWriteQueueProto.PendingWriteItem.class, + rawBytes -> PendingWritesQueueHelper.decodeLegacyItem(serializer, rawBytes)); + } + + /** + * Create the legacy {@link PendingWriteQueue} used only to write legacy-format entries until + * the deployment has switched over to the new format. Reads do not through this queue. + * + * @return the legacy pending-writes queue + */ + public PendingWriteQueue createLegacyPendingWritesQueue() { final boolean allowIncarnation = getBooleanIndexOption(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, false); return new PendingWriteQueue(pendingWritesQueueSubspace, pendingQueueSizeSubspace, maxPendingWritesToReplay, maxPendingQueueSize, serializer, allowIncarnation); } + /** + * The maximum number of pending entries to replay into an {@link org.apache.lucene.index.IndexWriter} + * for a query. A value of {@code 0} means unlimited. + * + * @return the configured replay limit + */ + public int getMaxPendingWritesToReplay() { + return maxPendingWritesToReplay; + } + + /** + * Whether new pending-queue entries should be written in the new (versioned {@code Any}-payload) + * format. Gated by {@link LuceneRecordContextProperties#LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT}. + * Reads always understand both formats regardless of this flag. + * + * @return {@code true} to write the new format, {@code false} to write the legacy format + */ + public boolean shouldWritePendingQueueNewFormat() { + return Boolean.TRUE.equals(agilityContext.getPropertyValue(LuceneRecordContextProperties.LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT)); + } + public int getBlockCacheMaximumSize() { return blockCacheMaximumSize; } 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 8c70fa3f31..99e7ea98cf 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 @@ -40,6 +40,7 @@ import com.apple.foundationdb.record.lucene.LuceneLogMessageKeys; import com.apple.foundationdb.record.lucene.LucenePartitionInfoProto; import com.apple.foundationdb.record.lucene.LucenePartitioner; +import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; import com.apple.foundationdb.record.lucene.LuceneRecordContextProperties; import com.apple.foundationdb.record.metadata.expressions.GroupingKeyExpression; import com.apple.foundationdb.record.metadata.expressions.KeyExpression; @@ -49,6 +50,7 @@ import com.apple.foundationdb.record.provider.foundationdb.IndexDeferredMaintenanceControl; import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; import com.apple.foundationdb.record.provider.foundationdb.KeyValueCursor; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; import com.apple.foundationdb.subspace.Subspace; import com.apple.foundationdb.tuple.Tuple; import com.apple.foundationdb.tuple.TupleHelpers; @@ -382,7 +384,7 @@ public void invalidatePrefix(@Nonnull Tuple prefix) { } } - public PendingWriteQueue getPendingWriteQueue(@Nullable Tuple groupingKey, @Nullable Integer partitionId) { + public PendingWritesQueue getPendingWriteQueue(@Nullable Tuple groupingKey, @Nullable Integer partitionId) { return getDirectoryWrapper(groupingKey, partitionId).getPendingWriteQueue(); } 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 819e32bce8..02f8b772b5 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 @@ -22,23 +22,30 @@ import com.apple.foundationdb.annotation.API; import com.apple.foundationdb.async.AsyncUtil; +import com.apple.foundationdb.record.ExecuteProperties; import com.apple.foundationdb.record.RecordCoreException; +import com.apple.foundationdb.record.RecordCursor; import com.apple.foundationdb.record.RecordCursorResult; import com.apple.foundationdb.record.ScanProperties; import com.apple.foundationdb.record.logging.KeyValueLogMessage; import com.apple.foundationdb.record.logging.LogMessageKeys; import com.apple.foundationdb.record.lucene.LuceneAnalyzerWrapper; import com.apple.foundationdb.record.lucene.LuceneConcurrency; +import com.apple.foundationdb.record.lucene.LuceneDocumentFromRecord; import com.apple.foundationdb.record.lucene.LuceneEvents; import com.apple.foundationdb.record.lucene.LuceneIndexMaintainer; import com.apple.foundationdb.record.lucene.LuceneLoggerInfoStream; +import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; import com.apple.foundationdb.record.lucene.LuceneRecordContextProperties; import com.apple.foundationdb.record.lucene.codec.LazyCloseable; import com.apple.foundationdb.record.lucene.codec.LazyOpener; import com.apple.foundationdb.record.lucene.codec.LuceneOptimizedCodec; +import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStore; import com.apple.foundationdb.record.provider.foundationdb.IndexDeferredMaintenanceControl; import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueueEntry; import com.apple.foundationdb.record.provider.foundationdb.runners.throttled.CursorFactory; import com.apple.foundationdb.record.provider.foundationdb.runners.throttled.ItemHandler; import com.apple.foundationdb.record.provider.foundationdb.runners.throttled.ThrottledRetryingIterator; @@ -68,6 +75,7 @@ import javax.annotation.Nullable; import java.io.Closeable; import java.io.IOException; +import java.util.List; import java.util.Map; import java.util.Queue; import java.util.concurrent.CompletableFuture; @@ -140,7 +148,12 @@ public class FDBDirectoryWrapper implements AutoCloseable { /** * The instance of the pending writes queue. */ - private LazyOpener pendingWriteQueue; + private LazyOpener> pendingWriteQueue; + /** + * The instance of the legacy pending writes queue, used only to write legacy-format entries + * while the deployment has not yet switched to the new format. Reads never go through it. + */ + private LazyOpener legacyPendingWriteQueue; FDBDirectoryWrapper(@Nonnull final IndexMaintainerState state, @Nonnull final Tuple key, @@ -163,6 +176,7 @@ public class FDBDirectoryWrapper implements AutoCloseable { replayedQueueContext = LazyCloseable.supply(() -> createReadOnlyContext()); replayedQueueIndexWriter = LazyCloseable.supply(() -> createIndexWriterWithReplayedQueue()); pendingWriteQueue = LazyOpener.supply(() -> directory.createPendingWritesQueue()); + legacyPendingWriteQueue = LazyOpener.supply(() -> directory.createLegacyPendingWritesQueue()); } @VisibleForTesting @@ -188,6 +202,7 @@ public FDBDirectoryWrapper(@Nonnull final IndexMaintainerState state, replayedQueueContext = LazyCloseable.supply(() -> createReadOnlyContext()); replayedQueueIndexWriter = LazyCloseable.supply(() -> createIndexWriterWithReplayedQueue()); pendingWriteQueue = LazyOpener.supply(() -> directory.createPendingWritesQueue()); + legacyPendingWriteQueue = LazyOpener.supply(() -> directory.createLegacyPendingWritesQueue()); } @Nonnull @@ -293,7 +308,7 @@ public IndexReader getIndexReaderWithReplayedQueue() throws IOException { // In case the current directory has writes, prioritize them over the pending writes queue. return DirectoryReader.open(writer.get()); } else { - PendingWriteQueue queue = getPendingWriteQueue(); + PendingWritesQueue queue = getPendingWriteQueue(); // Use the regular context to find out if the queue has anything final Boolean queueIsEmpty = LuceneConcurrency.asyncToSync( LuceneEvents.Waits.WAIT_LUCENE_READ_PENDING_QUEUE, @@ -325,10 +340,36 @@ private IndexWriter createIndexWriterWithReplayedQueue() throws IOException { agilityContextReadOnly.asyncToSync( LuceneEvents.Waits.WAIT_LUCENE_REPLAY_QUEUE, agilityContextReadOnly.apply(aContext -> - getPendingWriteQueue().replayQueuedOperations(aContext, readOnlyIndexWriter, state.index))); + replayQueuedOperations(aContext, readOnlyIndexWriter))); return readOnlyIndexWriter; } + /** + * Replay all queued operations into the given {@link IndexWriter} so that they are visible to a + * query. Honors the configured replay limit, failing if the queue has more + * entries than {@link FDBDirectory#getMaxPendingWritesToReplay()} allows. + */ + @Nonnull + private CompletableFuture replayQueuedOperations(@Nonnull final FDBRecordContext context, + @Nonnull final IndexWriter indexWriter) { + final int maxEntriesToReplay = directory.getMaxPendingWritesToReplay(); + ScanProperties scanProperties = ScanProperties.FORWARD_SCAN; + if (maxEntriesToReplay > 0) { + scanProperties = ExecuteProperties.newBuilder() + .setReturnedRowLimit(maxEntriesToReplay) + .build() + .asScanProperties(false); + } + return getPendingWriteQueue().getQueueCursor(context, scanProperties, null) + .forEachResult(result -> + PendingWritesQueueHelper.replayItem(result.get().getPayload(), indexWriter, state.index)) + .thenAccept(lastResult -> { + if (lastResult.getNoNextReason() == RecordCursor.NoNextReason.RETURN_LIMIT_REACHED) { + throw new PendingWriteQueue.TooManyPendingWritesException("Too many entries to replay into IndexWriter", maxEntriesToReplay); + } + }); + } + @Nonnull private CloseableReadOnlyAgilityContext createReadOnlyContext() { AgilityContext readOnlyContext = AgilityContext.readOnlyNonAgile(agilityContext.getCallerContext(), null); @@ -358,10 +399,67 @@ public DirectoryReader getWriterReader(boolean refresh) throws IOException { * Return the {@link PendingWriteQueue} instance to use to read documents queued while the directory was locked. * @return the queue instance */ - public PendingWriteQueue getPendingWriteQueue() { + public PendingWritesQueue getPendingWriteQueue() { return pendingWriteQueue.getUnchecked(); } + /** + * Return the legacy {@link PendingWriteQueue} instance used to write legacy-format + * entries until the deployment switches to the new format. + * @return the legacy queue instance + */ + private PendingWriteQueue getLegacyPendingWriteQueue() { + return legacyPendingWriteQueue.getUnchecked(); + } + + /** + * Enqueue an INSERT into the pending-writes queue, choosing the on-disk format by + * {@link FDBDirectory#shouldWritePendingQueueNewFormat()}: the new format is written through + * the generic {@link PendingWritesQueue}; the legacy format through the legacy + * {@link PendingWriteQueue}. + * + * @param context the record context to enqueue within + * @param primaryKey the record's primary key + * @param fields the document fields to index + * @param incarnation the incarnation prefix for the queue key + */ + public void enqueuePendingInsert(@Nonnull final FDBRecordContext context, + @Nonnull final Tuple primaryKey, + @Nonnull final List fields, + final int incarnation) { + if (directory.shouldWritePendingQueueNewFormat()) { + final LucenePendingWriteQueueProto.PendingWriteItem item = PendingWritesQueueHelper.buildPendingWriteItem( + LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, primaryKey, fields, System.currentTimeMillis()); + LuceneConcurrency.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_GET_QUEUE_SIZE, + getPendingWriteQueue().enqueue(context, item, incarnation), context); + } else { + getLegacyPendingWriteQueue().enqueueInsert(context, primaryKey, fields, incarnation); + } + } + + /** + * Enqueue a DELETE into the pending-writes queue, choosing the on-disk format by + * {@link FDBDirectory#shouldWritePendingQueueNewFormat()}: the new format is written through + * the generic {@link PendingWritesQueue}; the legacy format through the legacy + * {@link PendingWriteQueue}. + * + * @param context the record context to enqueue within + * @param primaryKey the record's primary key to delete + * @param incarnation the incarnation prefix for the queue key + */ + public void enqueuePendingDelete(@Nonnull final FDBRecordContext context, + @Nonnull final Tuple primaryKey, + final int incarnation) { + if (directory.shouldWritePendingQueueNewFormat()) { + final LucenePendingWriteQueueProto.PendingWriteItem item = PendingWritesQueueHelper.buildPendingWriteItem( + LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE, primaryKey, null, System.currentTimeMillis()); + LuceneConcurrency.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_GET_QUEUE_SIZE, + getPendingWriteQueue().enqueue(context, item, incarnation), context); + } else { + getLegacyPendingWriteQueue().enqueueDelete(context, primaryKey, incarnation); + } + } + private static class FDBDirectorySerialMergeScheduler extends MergeScheduler { /** * This class is temporarily duplicating FDBDirectoryMergeScheduler. @@ -591,8 +689,8 @@ public CompletableFuture drainPendingQueue(@Nonnull final Tuple groupingKe private CompletableFuture drainPendingQueueNow(@Nonnull final Tuple groupingKey, @Nullable final Integer partitionId) { // Note - since this directory wrapper was already created, agility context should be unused in the next line's path - final PendingWriteQueue writeQueue = getPendingWriteQueue(); - final ThrottledRetryingIterator iterator = ThrottledRetryingIterator.builder( + final PendingWritesQueue writeQueue = getPendingWriteQueue(); + final ThrottledRetryingIterator> iterator = ThrottledRetryingIterator.builder( agilityContext.getCallerContext().getDatabase(), agilityContext.getCallerContext().getConfig().toBuilder(), cursorFactory(writeQueue), @@ -609,8 +707,9 @@ private CompletableFuture drainPendingQueueNow(@Nonnull final Tuple groupi }); } - private CursorFactory cursorFactory(PendingWriteQueue pendingWriteQueue) { - return (@Nonnull FDBRecordStore store, @Nullable RecordCursorResult lastResult, int rowLimit) -> { + private CursorFactory> cursorFactory( + PendingWritesQueue pendingWriteQueue) { + return (@Nonnull FDBRecordStore store, @Nullable RecordCursorResult> lastResult, int rowLimit) -> { byte[] continuation = lastResult == null ? null : lastResult.getContinuation().toBytes(); ScanProperties scanProperties = ScanProperties.FORWARD_SCAN.with(executeProperties -> executeProperties.setReturnedRowLimit(rowLimit)); // Note: null could have been used instead of continuation as the preceding items should have been deleted. However, @@ -619,24 +718,28 @@ private CursorFactory cursorFactory(PendingWriteQu }; } - private ItemHandler handleOneItemFactory(PendingWriteQueue pendingWriteQueue, - @Nonnull final Tuple groupingKey, - @Nullable final Integer partitionId) { + private ItemHandler> handleOneItemFactory( + PendingWritesQueue pendingWriteQueue, + @Nonnull final Tuple groupingKey, + @Nullable final Integer partitionId) { return (store, lastResult, quotamanager) -> { - final PendingWriteQueue.QueueEntry queueEntry = lastResult.get(); + final PendingWritesQueueEntry queueEntry = lastResult.get(); if (queueEntry == null) { return AsyncUtil.DONE; } + final LucenePendingWriteQueueProto.PendingWriteItem item = queueEntry.getPayload(); + final Tuple primaryKey = Tuple.fromBytes(item.getPrimaryKey().toByteArray()); try { final LuceneIndexMaintainer maintainer = (LuceneIndexMaintainer)store.getIndexMaintainer(state.index); CompletableFuture oneItemFuture = AsyncUtil.DONE; - switch (queueEntry.getOperationType()) { + switch (item.getOperationType()) { case INSERT: - maintainer.writeDocumentBypassQueue(groupingKey, partitionId, queueEntry.getPrimaryKeyParsed(), queueEntry.getDocumentFieldsParsed()); + maintainer.writeDocumentBypassQueue(groupingKey, partitionId, primaryKey, + PendingWritesQueueHelper.fromProtoFields(item.getFieldsList())); break; case DELETE: - final int countDeleted = maintainer.deleteDocumentBypassQueue(groupingKey, partitionId, queueEntry.getPrimaryKeyParsed()); + final int countDeleted = maintainer.deleteDocumentBypassQueue(groupingKey, partitionId, primaryKey); if (partitionId != null) { oneItemFuture = maintainer.postDeleteUpdatePartitionCounter(groupingKey, partitionId, countDeleted) .thenApply(ignore -> null); @@ -647,7 +750,7 @@ private ItemHandler handleOneItemFactory(PendingWr throw new PendingQueueDrainException("Unknown operation type", LogMessageKeys.GROUPING_KEY, groupingKey, LogMessageKeys.PARTITION_ID, partitionId, - LogMessageKeys.CODE, queueEntry.getOperationType()); + LogMessageKeys.CODE, item.getOperationType()); } pendingWriteQueue.clearEntry(store.getContext(), queueEntry); return oneItemFuture; diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/PendingWritesQueueHelper.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/PendingWritesQueueHelper.java index 6c38b897e0..e2e54aa773 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/PendingWritesQueueHelper.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/PendingWritesQueueHelper.java @@ -22,11 +22,23 @@ import com.apple.foundationdb.annotation.API; import com.apple.foundationdb.record.RecordCoreArgumentException; +import com.apple.foundationdb.record.RecordCoreInternalException; +import com.apple.foundationdb.record.RecordCoreStorageException; import com.apple.foundationdb.record.lucene.LuceneDocumentFromRecord; +import com.apple.foundationdb.record.lucene.LuceneExceptions; import com.apple.foundationdb.record.lucene.LuceneIndexExpressions; +import com.apple.foundationdb.record.lucene.LuceneIndexMaintainerHelper; +import com.apple.foundationdb.record.lucene.LuceneLogMessageKeys; import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; +import com.apple.foundationdb.record.metadata.Index; +import com.apple.foundationdb.tuple.Tuple; +import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; +import org.apache.lucene.index.IndexWriter; import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -165,4 +177,82 @@ public static List fromProtoFields( // No constructor for utility class private PendingWritesQueueHelper() { } + + /** + * Build a Lucene pending-write item payload for an enqueue. + * + * @param operationType the operation type (INSERT or DELETE) + * @param primaryKey the record's primary key + * @param fields the document fields to index (may be {@code null} or empty, e.g. for a DELETE) + * @param enqueueTimestamp the enqueue timestamp to record on the item + * + * @return the built {@link LucenePendingWriteQueueProto.PendingWriteItem} + */ + @Nonnull + public static LucenePendingWriteQueueProto.PendingWriteItem buildPendingWriteItem( + @Nonnull LucenePendingWriteQueueProto.PendingWriteItem.OperationType operationType, + @Nonnull Tuple primaryKey, + @Nullable List fields, + long enqueueTimestamp) { + LucenePendingWriteQueueProto.PendingWriteItem.Builder builder = + LucenePendingWriteQueueProto.PendingWriteItem.newBuilder() + .setOperationType(operationType) + .setPrimaryKey(ByteString.copyFrom(primaryKey.pack())) + .setEnqueueTimestamp(enqueueTimestamp); + if (fields != null && !fields.isEmpty()) { + for (LuceneDocumentFromRecord.DocumentField field : fields) { + builder.addFields(toProtoField(field)); + } + } + return builder.build(); + } + + /** + * Replay a single pending-write item directly into an {@link IndexWriter}. + * + * @param item the item to replay + * @param indexWriter the writer to apply the operation to + * @param index the index being written + */ + public static void replayItem(@Nonnull LucenePendingWriteQueueProto.PendingWriteItem item, + @Nonnull IndexWriter indexWriter, + @Nonnull Index index) { + final LucenePendingWriteQueueProto.PendingWriteItem.OperationType opType = item.getOperationType(); + try { + final Tuple primaryKey = Tuple.fromBytes(item.getPrimaryKey().toByteArray()); + switch (opType) { + case INSERT: + LuceneIndexMaintainerHelper.writeDocument(indexWriter, index, primaryKey, fromProtoFields(item.getFieldsList())); + break; + case DELETE: + LuceneIndexMaintainerHelper.deleteDocument(indexWriter, primaryKey, index); + break; + case OPERATION_TYPE_UNSPECIFIED: + default: + throw new RecordCoreInternalException("Unknown queue operation type", LuceneLogMessageKeys.OPERATION_TYPE, opType); + } + } catch (IOException ex) { + throw LuceneExceptions.toRecordCoreException("failed to replay message on writer", ex); + } + } + + /** + * Decode the raw stored bytes of a legacy (serializer-wrapped) queue entry into a Lucene + * pending-write item. This is the read-side fallback used by the generic + * {@code PendingWritesQueue} for entries that predate the versioned {@code Any}-payload format. + * + * @param serializer the serializer used to decode (decompress/decrypt) the raw bytes + * @param rawBytes the raw stored value bytes + * + * @return the decoded {@link LucenePendingWriteQueueProto.PendingWriteItem} + */ + @Nonnull + public static LucenePendingWriteQueueProto.PendingWriteItem decodeLegacyItem(@Nonnull LuceneSerializer serializer, + @Nonnull byte[] rawBytes) { + try { + return LucenePendingWriteQueueProto.PendingWriteItem.parseFrom(serializer.decode(rawBytes)); + } catch (InvalidProtocolBufferException ex) { + throw new RecordCoreStorageException("Failed to parse legacy pending writes queue item", ex); + } + } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java index fa99aa3aeb..471c29c7ed 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java @@ -397,7 +397,9 @@ private static FDBDirectoryWrapper getDirectoryWrapper(final LuceneIndexMaintain private PendingWriteQueue getPendingWriteQueue(FDBRecordStore store, Index index) { LuceneIndexMaintainer indexMaintainer = getIndexMaintainer(store, index); final FDBDirectoryWrapper directoryWrapper = getDirectoryWrapper(indexMaintainer); - return directoryWrapper.getPendingWriteQueue(); + // This test drives enqueue directly through the legacy queue (legacy on-disk format); the + // query path replays through the generic PendingWritesQueue, which reads the legacy format. + return directoryWrapper.getDirectory().createLegacyPendingWritesQueue(); } protected FDBRecordStore openRecordStore(FDBRecordContext context, Index index) { 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 607845f974..de8ef6a7ef 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 @@ -37,7 +37,6 @@ import com.apple.foundationdb.record.lucene.directory.FDBDirectoryManager; import com.apple.foundationdb.record.lucene.directory.FDBDirectoryWrapper; import com.apple.foundationdb.record.lucene.directory.NonAgileContext; -import com.apple.foundationdb.record.lucene.directory.PendingWriteQueue; import com.apple.foundationdb.record.metadata.Index; import com.apple.foundationdb.record.provider.common.FixedZeroKeyManager; import com.apple.foundationdb.record.provider.common.RollingTestKeyManager; @@ -53,6 +52,7 @@ import com.apple.foundationdb.record.provider.foundationdb.OnlineIndexer; import com.apple.foundationdb.record.provider.foundationdb.keyspace.KeySpacePath; import com.apple.foundationdb.record.provider.foundationdb.properties.RecordLayerPropertyStorage; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; import com.apple.foundationdb.record.query.expressions.Query; import com.apple.foundationdb.record.test.TestKeySpace; import com.apple.foundationdb.record.util.RandomSecretUtil; @@ -1197,7 +1197,7 @@ void testCreatePendingWritesQueue() throws IOException { final LuceneIndexMaintainer indexMaintainer = getIndexMaintainer(store, dataModel.index); final FDBDirectoryManager directoryManager = indexMaintainer.getDirectoryManager(); final FDBDirectoryWrapper directoryWrapper = directoryManager.getDirectoryWrapper(groupingKey, partitionId); - final PendingWriteQueue pendingWriteQueue = directoryWrapper.getPendingWriteQueue(); + final PendingWritesQueue pendingWriteQueue = directoryWrapper.getPendingWriteQueue(); assertNotNull(pendingWriteQueue); } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java index 9ae48b44dd..68a080b22b 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java @@ -31,9 +31,12 @@ import com.apple.foundationdb.record.lucene.LuceneEvents; import com.apple.foundationdb.record.lucene.LuceneIndexExpressions; import com.apple.foundationdb.record.lucene.LuceneIndexOptions; +import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; import com.apple.foundationdb.record.provider.common.StoreTimer; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; import com.apple.foundationdb.record.provider.foundationdb.FDBStoreTimer; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueueEntry; import com.apple.foundationdb.tuple.Tuple; import com.apple.test.Tags; import org.assertj.core.api.Assertions; @@ -391,7 +394,7 @@ void testQueueIndicatorLifeCycle() { assertTrue(newDirectory.shouldUseQueue(), "shouldUseQueue should return true in new transaction after commit"); - PendingWriteQueue queue = newDirectory.createPendingWritesQueue(); + PendingWriteQueue queue = newDirectory.createLegacyPendingWritesQueue(); queue.enqueueInsert(newContext, Tuple.from("testDoc", 1), createTestFields(), @@ -419,10 +422,10 @@ void testQueueIndicatorLifeCycle() { // Clear the queue try (FDBRecordContext newContext = fdb.openContext()) { FDBDirectory newDirectory = createDirectory(subspace, newContext, null); - PendingWriteQueue queue = newDirectory.createPendingWritesQueue(); + PendingWritesQueue queue = newDirectory.createPendingWritesQueue(); // Read and clear all queue entries - RecordCursor cursor = + RecordCursor> cursor = queue.getQueueCursor(newContext, ScanProperties.FORWARD_SCAN, null); diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueFormatCompatibilityTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueFormatCompatibilityTest.java new file mode 100644 index 0000000000..95b7f55d47 --- /dev/null +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueFormatCompatibilityTest.java @@ -0,0 +1,342 @@ +/* + * PendingWriteQueueFormatCompatibilityTest.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-2026 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.apple.foundationdb.record.lucene.directory; + +import com.apple.foundationdb.record.PendingWritesQueueProto; +import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; +import com.apple.foundationdb.record.provider.common.FixedZeroKeyManager; +import com.apple.foundationdb.record.util.RandomSecretUtil; +import com.apple.foundationdb.tuple.Tuple; +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import com.google.protobuf.InvalidProtocolBufferException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.crypto.SecretKey; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * De-risking tests for the Lucene pending-write-queue format migration (Plan #1). + * + *

The migration stores new-format entries as a plain envelope proto (discriminated by the + * {@code version} field, ≥1) and leaves legacy entries exactly as the old queue wrote them + * ({@code LuceneSerializer.encode(proto)}). The reader parses each entry directly as the new + * envelope and, only when that fails or yields a non-new version, falls back to the serializer + + * the legacy descriptor — so new writes are never forced through the serializer. + * These tests are pure-protobuf (no FDB) and verify the wire-level assumptions the plan relies + * on:

+ *
    + *
  • E1 — legacy bytes (fields 1-4) parse cleanly as the new envelope + * ({@link PendingWritesQueueProto.PendingWriteItem}, which reserves 1-4 and now has all + * fields {@code optional}) without throwing, and report {@code version == 0} with no + * {@code payload}; the + * legacy content survives as retained unknown fields.
    • + *
  • the legacy bytes remain decodable with the legacy descriptor (the reader's legacy + * fallback);
    • + *
  • E3 — a new entry round-trips through {@code Any.pack}/{@code unpack};
    • + *
  • new bytes are NOT parseable by the legacy descriptor (justifies the read-before-write + * gate — an old binary must never see a new-format entry);
    • + *
  • new-format entries parse directly with no serializer; legacy (serializer-wrapped) + * entries trigger the serializer fallback; and one reader recovers a stream mixing both, + * across all compression/encryption modes.
    • + *
+ */ +class PendingWriteQueueFormatCompatibilityTest { + + private static final int NEW_FORMAT_VERSION = 1; + + /** + * E1: legacy bytes parse as the new envelope without throwing, carry no {@code payload}, and + * keep the legacy fields as unknown fields. + */ + @Test + void legacyBytesParseAsNewEnvelopeWithoutPayload() throws InvalidProtocolBufferException { + byte[] legacyBytes = legacyInsert("record-1", "hello").toByteArray(); + + // This is the crux: with the envelope's fields relaxed to optional, parsing legacy bytes + // (which have none of fields 10-12) must not throw on missing required fields. + PendingWritesQueueProto.PendingWriteItem envelope = + PendingWritesQueueProto.PendingWriteItem.parseFrom(legacyBytes); + + assertThat(envelope.hasPayload()).as("legacy entry has no Any payload").isFalse(); + assertThat(envelope.hasVersion()).as("legacy entry has no version field").isFalse(); + assertThat(envelope.getVersion()).as("absent version defaults to 0").isEqualTo(0); + assertThat(envelope.hasEnqueueTimestamp()).isFalse(); + + // The legacy fields (1-4) are retained as unknown fields rather than lost or erroring. + assertThat(envelope.getUnknownFields().hasField(1)).as("operation_type retained").isTrue(); + assertThat(envelope.getUnknownFields().hasField(2)).as("primary_key retained").isTrue(); + assertThat(envelope.getUnknownFields().hasField(3)).as("enqueue_timestamp retained").isTrue(); + assertThat(envelope.getUnknownFields().hasField(4)).as("fields retained").isTrue(); + } + + /** + * The reader's legacy fallback: the same raw bytes are still a valid legacy message, so + * decoding them with the legacy descriptor recovers the original operation. + */ + @Test + void legacyBytesRemainDecodableWithLegacyDescriptor() throws InvalidProtocolBufferException { + LucenePendingWriteQueueProto.PendingWriteItem original = legacyInsert("record-2", "world"); + byte[] legacyBytes = original.toByteArray(); + + LucenePendingWriteQueueProto.PendingWriteItem roundTripped = + LucenePendingWriteQueueProto.PendingWriteItem.parseFrom(legacyBytes); + + assertThat(roundTripped).isEqualTo(original); + } + + /** + * E3: a new entry wraps the (Lucene) payload in an {@code Any}, and round-trips through + * serialize → parse → unpack. + */ + @Test + void newEnvelopeRoundTripsThroughAnyPayload() throws InvalidProtocolBufferException { + LucenePendingWriteQueueProto.PendingWriteItem payload = legacyInsert("record-3", "payload"); + byte[] newBytes = newEnvelope(payload).toByteArray(); + + PendingWritesQueueProto.PendingWriteItem envelope = + PendingWritesQueueProto.PendingWriteItem.parseFrom(newBytes); + + assertThat(envelope.hasPayload()).as("new entry carries an Any payload").isTrue(); + assertThat(envelope.getVersion()).isEqualTo(NEW_FORMAT_VERSION); + assertThat(envelope.getPayload().is(LucenePendingWriteQueueProto.PendingWriteItem.class)) + .as("payload type URL matches the Lucene message").isTrue(); + + LucenePendingWriteQueueProto.PendingWriteItem unpacked = + envelope.getPayload().unpack(LucenePendingWriteQueueProto.PendingWriteItem.class); + assertThat(unpacked).isEqualTo(payload); + } + + /** + * New-format bytes must NOT be parseable by the legacy descriptor: the legacy message has + * required fields 1-3 that a new entry lacks, so {@code parseFrom} throws. This is exactly + * why an old (pre-migration) binary must never encounter a new-format entry, and hence why + * new-format writes are gated behind a fleet-wide property. + */ + @Test + void newBytesAreNotParseableByLegacyDescriptor() { + byte[] newBytes = newEnvelope(legacyInsert("record-4", "nope")).toByteArray(); + + assertThrows(InvalidProtocolBufferException.class, + () -> LucenePendingWriteQueueProto.PendingWriteItem.parseFrom(newBytes)); + } + + /** + * New-format entries are stored as a plain envelope proto (NOT serializer-wrapped), so the + * reader parses them directly — no serializer needed. This is the common path for an index + * with no legacy backlog. + */ + @Test + void newFormatEntriesParseDirectlyWithoutSerializer() throws InvalidProtocolBufferException { + List expected = new ArrayList<>(); + List onDisk = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + LucenePendingWriteQueueProto.PendingWriteItem op = (i % 3 == 0) + ? legacyDelete("record-" + i) + : legacyInsert("record-" + i, "value-" + i); + expected.add(op); + onDisk.add(newEnvelope(op).toByteArray()); + } + + List decoded = new ArrayList<>(); + for (byte[] raw : onDisk) { + // No serializer: new entries parse directly, so the fallback is never needed. + decoded.add(decodeEntry(raw, null)); + } + + assertThat(decoded).containsExactlyElementsOf(expected); + } + + /** + * E2: the realistic mixed queue. Legacy entries are stored exactly as the old queue wrote + * them — {@code serializer.encode(proto)} — while new entries are stored as a plain envelope + * proto (NOT serializer-wrapped). A single reader parses each entry directly as the new + * envelope and, only when that fails (or yields a non-new version), falls back to + * {@link LuceneSerializer#decode} + the legacy descriptor. Exercised across all + * compression/encryption modes, since only the legacy wrapping varies. + */ + @ParameterizedTest(name = "compress={0}, encrypt={1}") + @MethodSource("serializerModes") + void mixedFormatStreamRoundTripsWithSerializerFallback(boolean compress, boolean encrypt) + throws InvalidProtocolBufferException { + final LuceneSerializer serializer = buildSerializer(compress, encrypt); + + final List expected = new ArrayList<>(); + final List onDisk = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + LucenePendingWriteQueueProto.PendingWriteItem op = (i % 3 == 0) + ? legacyDelete("record-" + i) + : legacyInsert("record-" + i, "value-" + i); + expected.add(op); + // Even indices are LEGACY (serializer-wrapped, as the old queue stored them); odd + // indices are NEW (plain envelope proto, not wrapped). + byte[] stored = (i % 2 == 0) + ? serializer.encode(op.toByteArray()) + : newEnvelope(op).toByteArray(); + onDisk.add(stored); + } + // Add large, compressible legacy entries so the compression path actually engages. + LucenePendingWriteQueueProto.PendingWriteItem bigLegacy = legacyInsert("big-legacy", "x".repeat(4096)); + expected.add(bigLegacy); + onDisk.add(serializer.encode(bigLegacy.toByteArray())); + LucenePendingWriteQueueProto.PendingWriteItem bigNew = legacyInsert("big-new", "y".repeat(4096)); + expected.add(bigNew); + onDisk.add(newEnvelope(bigNew).toByteArray()); + + final List decoded = new ArrayList<>(); + for (byte[] stored : onDisk) { + decoded.add(decodeEntry(stored, serializer)); + } + + assertThat(decoded).containsExactlyElementsOf(expected); + } + + /** + * The read-path fallback contract. A new-format entry is a plain envelope proto, so it + * parses directly (no serializer involved). A legacy entry is {@code serializer.encode}d, so + * parsing it directly as the new envelope fails — and THAT parse failure is what triggers + * the reader to fall back to {@link LuceneSerializer#decode} + the legacy descriptor. New + * entries are never forced through the serializer. + */ + @Test + void legacyEntryFallsBackToSerializerWhenEnvelopeParseFails() throws InvalidProtocolBufferException { + final LuceneSerializer serializer = buildSerializer(false, false); + + // New entry: plain envelope proto, parses directly as the new format. + final LucenePendingWriteQueueProto.PendingWriteItem newOp = legacyInsert("new-record", "v"); + final byte[] newStored = newEnvelope(newOp).toByteArray(); + assertThat(PendingWritesQueueProto.PendingWriteItem.parseFrom(newStored).getVersion()) + .isEqualTo(NEW_FORMAT_VERSION); + assertThat(decodeEntry(newStored, serializer)).isEqualTo(newOp); + + // Legacy entry: serializer-wrapped. Parsing it directly as the new envelope fails (even + // the minimal prefix byte is not a valid protobuf tag), which is the fallback trigger. + final LucenePendingWriteQueueProto.PendingWriteItem legacyOp = legacyInsert("legacy-record", "v"); + final byte[] legacyStored = serializer.encode(legacyOp.toByteArray()); + assertThrows(InvalidProtocolBufferException.class, + () -> PendingWritesQueueProto.PendingWriteItem.parseFrom(legacyStored)); + // The reader falls back through the serializer and recovers the original operation. + assertThat(decodeEntry(legacyStored, serializer)).isEqualTo(legacyOp); + } + + @Nonnull + static Stream serializerModes() { + return Stream.of( + Arguments.of(false, false), + Arguments.of(true, false), + Arguments.of(false, true), + Arguments.of(true, true)); + } + + @Nonnull + private static LuceneSerializer buildSerializer(boolean compress, boolean encrypt) { + if (!encrypt) { + return new LuceneSerializer(compress, false, null, true); + } + // Deterministic key + IV source so the test is reproducible. + final Random random = new Random(0xF00DL); + final SecretKey key = RandomSecretUtil.randomSecretKey(random); + return new LuceneSerializer(compress, true, new FixedZeroKeyManager(key, null, random), true); + } + + /** + * Mirrors the planned reader dispatch. New-format entries are stored as a plain envelope + * proto, so they parse directly and report {@code version >= }{@link #NEW_FORMAT_VERSION}; + * their payload is unpacked from the {@code Any}. Anything that does not parse as a new-format + * envelope (a legacy, serializer-wrapped entry) falls back to {@link LuceneSerializer#decode} + * followed by the legacy descriptor. The serializer is therefore only consulted on the + * fallback path, and may be {@code null} when no legacy entries are expected. + */ + @Nonnull + private static LucenePendingWriteQueueProto.PendingWriteItem decodeEntry( + @Nonnull byte[] raw, @Nullable LuceneSerializer serializer) + throws InvalidProtocolBufferException { + final PendingWritesQueueProto.PendingWriteItem envelope = tryParseEnvelope(raw); + if (envelope != null && envelope.getVersion() >= NEW_FORMAT_VERSION) { + return envelope.getPayload().unpack(LucenePendingWriteQueueProto.PendingWriteItem.class); + } + // Fallback: a legacy, serializer-wrapped entry (it either failed to parse as the new + // envelope, or parsed as version 0 — new entries always carry version >= 1). + if (serializer == null) { + throw new IllegalStateException("entry is not new-format and no serializer was provided to decode it"); + } + return LucenePendingWriteQueueProto.PendingWriteItem.parseFrom(serializer.decode(raw)); + } + + /** + * Parse the bytes as the new envelope, returning {@code null} if they are not a valid + * envelope (e.g. a serializer-wrapped legacy blob). + */ + @Nullable + private static PendingWritesQueueProto.PendingWriteItem tryParseEnvelope(@Nonnull byte[] raw) { + try { + return PendingWritesQueueProto.PendingWriteItem.parseFrom(raw); + } catch (InvalidProtocolBufferException e) { + return null; + } + } + + @Nonnull + private static PendingWritesQueueProto.PendingWriteItem newEnvelope( + @Nonnull LucenePendingWriteQueueProto.PendingWriteItem payload) { + return PendingWritesQueueProto.PendingWriteItem.newBuilder() + .setVersion(NEW_FORMAT_VERSION) + .setPayload(Any.pack(payload)) + .setEnqueueTimestamp(System.currentTimeMillis()) + .build(); + } + + @Nonnull + private static LucenePendingWriteQueueProto.PendingWriteItem legacyInsert( + @Nonnull String recordId, @Nonnull String fieldValue) { + return LucenePendingWriteQueueProto.PendingWriteItem.newBuilder() + .setOperationType(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT) + .setPrimaryKey(ByteString.copyFrom(Tuple.from(recordId).pack())) + .setEnqueueTimestamp(1234567890L) + .addFields(LucenePendingWriteQueueProto.DocumentField.newBuilder() + .setFieldName("f") + .setStored(true) + .setSorted(false) + .setStringValue(fieldValue) + .build()) + .build(); + } + + @Nonnull + private static LucenePendingWriteQueueProto.PendingWriteItem legacyDelete(@Nonnull String recordId) { + return LucenePendingWriteQueueProto.PendingWriteItem.newBuilder() + .setOperationType(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE) + .setPrimaryKey(ByteString.copyFrom(Tuple.from(recordId).pack())) + .setEnqueueTimestamp(1234567890L) + .build(); + } +} diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java index 4a0cd52dd1..93f5d82fff 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java @@ -20,7 +20,9 @@ package com.apple.foundationdb.record.lucene.directory; +import com.apple.foundationdb.KeyValue; import com.apple.foundationdb.record.IndexEntry; +import com.apple.foundationdb.record.PendingWritesQueueProto; import com.apple.foundationdb.record.RecordCursor; import com.apple.foundationdb.record.ScanProperties; import com.apple.foundationdb.record.TestHelpers; @@ -34,6 +36,7 @@ import com.apple.foundationdb.record.lucene.LucenePartitionInfoProto; import com.apple.foundationdb.record.lucene.LucenePartitioner; import com.apple.foundationdb.record.lucene.LucenePendingWriteQueueProto; +import com.apple.foundationdb.record.lucene.LuceneRecordContextProperties; import com.apple.foundationdb.record.lucene.LuceneScanBounds; import com.apple.foundationdb.record.metadata.Index; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; @@ -41,11 +44,15 @@ import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreTestBase; import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; import com.apple.foundationdb.record.provider.foundationdb.keyspace.KeySpacePath; +import com.apple.foundationdb.record.provider.foundationdb.properties.RecordLayerPropertyStorage; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueueEntry; import com.apple.foundationdb.record.test.TestKeySpace; import com.apple.foundationdb.subspace.Subspace; import com.apple.foundationdb.tuple.Tuple; import com.apple.test.BooleanSource; import com.apple.test.Tags; +import com.google.protobuf.InvalidProtocolBufferException; import org.apache.lucene.index.IndexWriter; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Tag; @@ -838,16 +845,16 @@ void testConcurrentMixedOperations(boolean withMerge) { FDBDirectoryManager directoryManager = FDBDirectoryManager.getManager(state); FDBDirectory directory = directoryManager.getDirectory(null, null); - PendingWriteQueue queue = directory.createPendingWritesQueue(); - List entries = queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) + PendingWritesQueue queue = directory.createPendingWritesQueue(); + List> entries = queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) .asList().join(); // Count operation types long insertOps = entries.stream() - .filter(e -> e.getOperationType() == LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT) + .filter(e -> e.getPayload().getOperationType() == LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT) .count(); long deleteOps = entries.stream() - .filter(e -> e.getOperationType() == LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE) + .filter(e -> e.getPayload().getOperationType() == LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE) .count(); if (withMerge) { @@ -953,8 +960,8 @@ void testPendingQueueIncarnationIndexOption(boolean incarnationEnabled) { IndexMaintainerState state = new IndexMaintainerState(recordStore, index, recordStore.getIndexMaintenanceFilter()); FDBDirectory directory = FDBDirectoryManager.getManager(state).getDirectory(null, null); - PendingWriteQueue queue = directory.createPendingWritesQueue(); - List entries = + PendingWritesQueue queue = directory.createPendingWritesQueue(); + List> entries = queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null).asList().join(); assertEquals(1, entries.size()); // With incarnation: key is (incarnation, versionstamp); without: key is (versionstamp) @@ -1024,6 +1031,80 @@ void testMergeDrainsMultipleIncarnations() { verifyExpectedDocIds(schemaSetup, index, Set.of(1001L, 1002L, 1003L)); } + @Test + void testPendingQueueMixedFormatDrains() { + // End-to-end: land a genuine mix of legacy-format and new-format entries in one partition's + // queue by flipping LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT between writes, then drain + // through the real merge path and confirm the index reflects every operation regardless of + // the on-disk format each entry was written in. + // + // Incarnation is enabled so that both the legacy queue and the new (generic) queue write + // size-2 (incarnation, versionstamp) keys; this matches the real migration precondition + // (incarnation is in the key before the new-format write is switched on) and keeps the two + // formats ordered by enqueue time in a single subspace. + final Index index = simpleTextSuffixesIndex(options -> + options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true")); + final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); + final Function schemaSetup = context -> + LuceneIndexTestUtils.rebuildIndexMetaData(context, path, + TestRecordsTextProto.SimpleDocument.getDescriptor().getName(), + index, useCascadesPlanner).getLeft(); + + final RecordLayerPropertyStorage newFormatProps = RecordLayerPropertyStorage.newBuilder() + .addProp(LuceneRecordContextProperties.LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT, true) + .build(); + + // Enable queue mode. + setOngoingMergeIndicator(schemaSetup, index, null, null); + + // Write two records in the LEGACY format (property off — the default). + try (FDBRecordContext context = openContext()) { + FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); + recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "legacy one", 1)); + recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1002L, "legacy two", 1)); + commit(context); + } + + // Write two records in the NEW format (property on). + try (FDBRecordContext context = openContext(newFormatProps)) { + FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); + recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1003L, "new three", 1)); + recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1004L, "new four", 1)); + commit(context); + } + + // Delete a legacy-written document with the new-format writer, so a new-format DELETE lands + // in the same queue after the legacy INSERTs. + try (FDBRecordContext context = openContext(newFormatProps)) { + FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); + recordStore.deleteRecord(Tuple.from(1001L)); + commit(context); + } + + // The queue really does hold both on-disk formats side by side. + int[] formatCounts = countQueueEntriesByFormat(schemaSetup, index); + assertEquals(2, formatCounts[0], "expected two legacy-format entries"); + assertEquals(3, formatCounts[1], "expected three new-format entries"); + + // A single reader recovers the mixed stream in enqueue order (4 INSERTs then the DELETE). + verifyExpectedQueueAndIndicator(schemaSetup, index, null, null, + List.of(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, + LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, + LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, + LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, + LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE)); + + // Query replay over the mixed queue: 1001 was deleted, the rest are visible. + verifyExpectedDocIds(schemaSetup, index, Set.of(1002L, 1003L, 1004L)); + + // Merge drains the mixed-format queue through the real drain path. + mergeIndexNow(schemaSetup, index); + + // Queue cleared, indicator removed, and the index reflects all operations from both formats. + verifyClearedQueueAndIndicator(schemaSetup, index, null, null); + verifyExpectedDocIds(schemaSetup, index, Set.of(1002L, 1003L, 1004L)); + } + protected static void snooze(int millis) { try { Thread.sleep(millis); @@ -1033,6 +1114,49 @@ protected static void snooze(int millis) { } } + /** + * Read the raw queue entries for a non-partitioned index and classify each by on-disk format. + * + * @param schemaSetup the schema setup function + * @param index the index whose queue subspace is inspected + * @return a two-element array {@code [legacyCount, newFormatCount]} + */ + private int[] countQueueEntriesByFormat(Function schemaSetup, Index index) { + try (FDBRecordContext context = openContext()) { + FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); + Subspace queueSubspace = recordStore.indexSubspace(index) + .subspace(Tuple.from(FDBDirectory.PENDING_WRITE_QUEUE_SUBSPACE)); + List kvs = context.ensureActive().getRange(queueSubspace.range()).asList().join(); + int legacy = 0; + int newFormat = 0; + for (KeyValue kv : kvs) { + if (isNewFormatValue(kv.getValue())) { + newFormat++; + } else { + legacy++; + } + } + commit(context); + return new int[] {legacy, newFormat}; + } + } + + /** + * Detect whether raw stored value bytes are in the new (versioned {@code Any}-payload) format. + * New-format entries parse as the envelope with {@code version >= 1}; legacy serializer-wrapped + * bytes either fail to parse as the envelope or parse with {@code version == 0}. + * + * @param value the raw stored value bytes + * @return {@code true} if the bytes are the new format + */ + private static boolean isNewFormatValue(byte[] value) { + try { + return PendingWritesQueueProto.PendingWriteItem.parseFrom(value).getVersion() >= 1; + } catch (InvalidProtocolBufferException e) { + return false; + } + } + private void verifyClearedQueueAndIndicator(Function schemaSetup, Index index, @Nullable Tuple groupingKey, @Nullable Integer partitionId) { try (FDBRecordContext context = openContext()) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); @@ -1043,8 +1167,8 @@ private void verifyClearedQueueAndIndicator(Function entries = new ArrayList<>(); + PendingWritesQueue queue = directory.createPendingWritesQueue(); + List> entries = new ArrayList<>(); queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) .forEach(entries::add).join(); assertTrue(entries.isEmpty(), "Pending queue should have been empty"); @@ -1064,14 +1188,14 @@ private void verifyExpectedQueueAndIndicator(Function queue = directory.createPendingWritesQueue(); - RecordCursor queueCursor = queue.getQueueCursor( + RecordCursor> queueCursor = queue.getQueueCursor( recordStore.getContext(), ScanProperties.FORWARD_SCAN, null); assertEquals(expectedOperations, queueCursor.asList().join().stream() - .map(PendingWriteQueue.QueueEntry::getOperationType) + .map(entry -> entry.getPayload().getOperationType()) .collect(Collectors.toList())); commit(context); diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java index a718326fd8..efbcbe643b 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java @@ -41,6 +41,8 @@ import com.apple.foundationdb.record.provider.foundationdb.IndexMaintainerState; import com.apple.foundationdb.record.provider.foundationdb.indexes.TextIndexTestUtils; import com.apple.foundationdb.record.provider.foundationdb.properties.RecordLayerPropertyStorage; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueueEntry; import com.apple.foundationdb.tuple.Tuple; import com.apple.test.Tags; import org.junit.jupiter.api.Tag; @@ -283,8 +285,8 @@ private void verifyClearedQueueAndIndicator(Index index, @Nullable Tuple groupin assertFalse(directory.shouldUseQueue(), "Ongoing merge indicator should be cleared"); - PendingWriteQueue queue = directory.createPendingWritesQueue(); - List entries = new ArrayList<>(); + PendingWritesQueue queue = directory.createPendingWritesQueue(); + List> entries = new ArrayList<>(); queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) .forEach(entries::add).join(); assertTrue(entries.isEmpty(), "Pending queue should have been empty"); @@ -304,14 +306,14 @@ private void verifyExpectedQueueAndIndicator(Index index, @Nullable Tuple groupi assertTrue(directory.shouldUseQueue(), "Ongoing merge indicator should have been set"); - PendingWriteQueue queue = directory.createPendingWritesQueue(); + PendingWritesQueue queue = directory.createPendingWritesQueue(); - RecordCursor queueCursor = queue.getQueueCursor( + RecordCursor> queueCursor = queue.getQueueCursor( recordStore.getContext(), ScanProperties.FORWARD_SCAN, null); assertEquals(expectedOperations, queueCursor.asList().join().stream() - .map(PendingWriteQueue.QueueEntry::getOperationType) + .map(entry -> entry.getPayload().getOperationType()) .collect(Collectors.toList())); commit(context); From b18d1cf675811bd71a28a60b18edbbe7456e477b Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 9 Jul 2026 14:58:28 -0400 Subject: [PATCH 2/5] Some refactoring to code --- .../queue/PendingWritesQueue.java | 155 ++++++++++-------- .../queue/PendingWritesQueueEntry.java | 15 +- .../PendingWritesQueueLegacyFallbackTest.java | 25 ++- .../record/lucene/LuceneEvents.java | 2 + .../record/lucene/directory/FDBDirectory.java | 26 ++- .../lucene/directory/FDBDirectoryWrapper.java | 53 +++--- 6 files changed, 155 insertions(+), 121 deletions(-) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java index 51ca103f4f..f267a1035b 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java @@ -127,39 +127,13 @@ public class PendingWritesQueue { private final long maxQueueSize; @Nonnull private final Class payloadClass; - /** - * Optional fallback used on read when an on-disk entry is not in the current envelope format - * (i.e. it has no {@code version >= 1}). It receives the raw stored value bytes and returns - * the payload {@code T}. This exists to read entries written by a predecessor queue - * implementation whose value layout differs from {@link PendingWritesQueueProto.PendingWriteItem} - * (for example, entries wrapped by a caller-specific serializer). {@code null} when there is - * no legacy format to support, in which case a non-current entry is a storage error. - */ - @Nullable - private final Function legacyDecoder; /** - * Construct a pending writes queue with no legacy-format support. + * Construct a pending writes queue. * - * @param queueSubspace subspace under which queue entries live; the caller owns its layout - * @param queueSizeSubspace subspace under which the atomic size counter lives; must not - * overlap with {@code queueSubspace} - * @param maxQueueSize maximum number of entries allowed in the queue; {@link #enqueue} - * rejects further entries when the counter reaches this value. Pass {@code 0} to disable - * the cap. - * @param payloadClass class of the payload message type, e.g. {@code MyPayload.class}; used - * to verify and unpack the payload on read - */ - public PendingWritesQueue(@Nonnull Subspace queueSubspace, - @Nonnull Subspace queueSizeSubspace, - long maxQueueSize, - @Nonnull Class payloadClass) { - this(queueSubspace, queueSizeSubspace, maxQueueSize, payloadClass, null); - } - - /** - * Construct a pending writes queue that can also read entries written by a predecessor - * implementation via a legacy decoder. + *

The queue only ever writes the current envelope format. The queue does support + * reading of a previous encoded format through passing in a legacy encoder to + * {@link #getQueueCursor(FDBRecordContext, ScanProperties, byte[], Function)}.

* * @param queueSubspace subspace under which queue entries live; the caller owns its layout * @param queueSizeSubspace subspace under which the atomic size counter lives; must not @@ -169,20 +143,15 @@ public PendingWritesQueue(@Nonnull Subspace queueSubspace, * the cap. * @param payloadClass class of the payload message type, e.g. {@code MyPayload.class}; used * to verify and unpack the payload on read - * @param legacyDecoder fallback that turns the raw stored bytes of a non-current-format entry - * into a payload {@code T}; {@code null} if there is no legacy format to read. The queue only - * ever writes the current format; the decoder is used on the read path. */ public PendingWritesQueue(@Nonnull Subspace queueSubspace, @Nonnull Subspace queueSizeSubspace, long maxQueueSize, - @Nonnull Class payloadClass, - @Nullable Function legacyDecoder) { + @Nonnull Class payloadClass) { this.queueSubspace = queueSubspace; this.queueSizeSubspace = queueSizeSubspace; this.maxQueueSize = maxQueueSize; this.payloadClass = payloadClass; - this.legacyDecoder = legacyDecoder; } /** @@ -210,6 +179,27 @@ public CompletableFuture enqueue(@Nonnull FDBRecordContext context, return capacityCheck(context).thenAccept(ignored -> writeEntry(context, payload, incarnation)); } + /** + * Return a cursor that iterates through the queue entries in {@code (incarnation, + * versionstamp)} order. + * + *

Equivalent to {@link #getQueueCursor(FDBRecordContext, ScanProperties, byte[], Function)} + * with no legacy decoder.

+ * + * @param context the record context to scan within + * @param scanProperties scan properties; the isolation level is always forced to {@link IsolationLevel#SNAPSHOT} + * @param continuation continuation from a previous cursor invocation, or {@code null} to + * start from the beginning + * + * @return a cursor over {@link PendingWritesQueueEntry} values + */ + @Nonnull + public RecordCursor> getQueueCursor(@Nonnull FDBRecordContext context, + @Nonnull ScanProperties scanProperties, + @Nullable byte[] continuation) { + return getQueueCursor(context, scanProperties, continuation, null); + } + /** * Return a cursor that iterates through the queue entries in {@code (incarnation, * versionstamp)} order. @@ -224,6 +214,10 @@ public CompletableFuture enqueue(@Nonnull FDBRecordContext context, * @param scanProperties scan properties; the isolation level is always forced to {@link IsolationLevel#SNAPSHOT} * @param continuation continuation from a previous cursor invocation, or {@code null} to * start from the beginning + * @param legacyDecoder fallback that turns the raw stored bytes of a previous-format entry + * (one that fails to parse as the current envelope) into a + * payload {@code T}; {@code null} if there is no legacy format to read, in which case such an + * entry is a storage error. * * @return a cursor over {@link PendingWritesQueueEntry} values */ @@ -231,7 +225,8 @@ public CompletableFuture enqueue(@Nonnull FDBRecordContext context, @Nonnull public RecordCursor> getQueueCursor(@Nonnull FDBRecordContext context, @Nonnull ScanProperties scanProperties, - @Nullable byte[] continuation) { + @Nullable byte[] continuation, + @Nullable Function legacyDecoder) { // Inner-cursor properties: clear per-row limits (the unsplitter applies entry-level // limits) and force snapshot isolation so the read never installs a read-conflict // range over the queue subspace. The unsplitter below still receives the ORIGINAL @@ -254,7 +249,7 @@ public RecordCursor> getQueueCursor(@Nonnull FDBRecor context, queueSubspace, inner, false, null, scanProperties) .limitRowsTo(scanProperties.getExecuteProperties().getReturnedRowLimit()); - return unsplitter.map(rawRecord -> toQueueEntry(rawRecord.getPrimaryKey(), rawRecord.getRawRecord())); + return unsplitter.map(rawRecord -> toQueueEntry(rawRecord.getPrimaryKey(), rawRecord.getRawRecord(), legacyDecoder)); } /** @@ -366,44 +361,68 @@ private CompletableFuture capacityCheck(@Nonnull FDBRecordContext context) } @Nonnull - private PendingWritesQueueEntry toQueueEntry(@Nonnull Tuple keyTuple, @Nonnull byte[] valueBytes) { + private PendingWritesQueueEntry toQueueEntry(@Nonnull Tuple keyTuple, @Nonnull byte[] valueBytes, + @Nullable Function legacyDecoder) { final PendingWritesQueueProto.PendingWriteItem item = tryParseEnvelope(valueBytes); if (item != null && item.getVersion() > 0) { // Current format: a versioned envelope carrying an Any payload. - if (item.getVersion() > CURRENT_VERSION) { - throw new RecordCoreStorageException("Pending writes queue entry version is newer than this reader supports") - .addLogInfo(LogMessageKeys.VERSION, CURRENT_VERSION) - .addLogInfo(LogMessageKeys.STORED_VERSION, item.getVersion()) - .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); - } - final Any storedPayload = item.getPayload(); - // Any#is derives the expected type URL from the bound message class internally and - // compares it to the stored URL. If it doesn't match, we surface both URLs for - // diagnostics before unpacking would itself throw. - if (!storedPayload.is(payloadClass)) { - throw new RecordCoreStorageException("Pending writes queue entry payload type does not match the queue's bound type") - .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) - .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()) - .addLogInfo(LogMessageKeys.ACTUAL_TYPE, storedPayload.getTypeUrl()); - } - final T payload; - try { - payload = storedPayload.unpack(payloadClass); - } catch (InvalidProtocolBufferException ex) { - throw new RecordCoreStorageException("Failed to unpack pending writes queue entry payload", ex) - .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) - .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()); - } - return new PendingWritesQueueEntry<>(keyTuple, payload, storedPayload.getTypeUrl(), item.getEnqueueTimestamp()); + return currentFormatEntry(keyTuple, item); } - - // Not the current format (parse failed, or the entry has no version). If a legacy decoder - // is configured, hand it the raw stored bytes; otherwise this is a storage error. + // Not the current format (parse failed, or the entry has no version): fall back to the + // legacy decoder if one is configured. if (legacyDecoder == null) { throw new RecordCoreStorageException("Failed to parse pending writes queue entry and no legacy decoder is configured") .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) .addLogInfo(LogMessageKeys.STORED_VERSION, item == null ? -1 : item.getVersion()); } + return legacyFormatEntry(keyTuple, valueBytes, item, legacyDecoder); + } + + /** + * Build an entry from a current-format envelope: validate the version and payload type, then + * unpack the {@code Any} into the queue's bound message type. + */ + @Nonnull + private PendingWritesQueueEntry currentFormatEntry(@Nonnull Tuple keyTuple, + @Nonnull PendingWritesQueueProto.PendingWriteItem item) { + if (item.getVersion() > CURRENT_VERSION) { + throw new RecordCoreStorageException("Pending writes queue entry version is newer than this reader supports") + .addLogInfo(LogMessageKeys.VERSION, CURRENT_VERSION) + .addLogInfo(LogMessageKeys.STORED_VERSION, item.getVersion()) + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); + } + final Any storedPayload = item.getPayload(); + // Any#is derives the expected type URL from the bound message class internally and + // compares it to the stored URL. If it doesn't match, we surface both URLs for + // diagnostics before unpacking would itself throw. + if (!storedPayload.is(payloadClass)) { + throw new RecordCoreStorageException("Pending writes queue entry payload type does not match the queue's bound type") + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) + .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()) + .addLogInfo(LogMessageKeys.ACTUAL_TYPE, storedPayload.getTypeUrl()); + } + final T payload; + try { + payload = storedPayload.unpack(payloadClass); + } catch (InvalidProtocolBufferException ex) { + throw new RecordCoreStorageException("Failed to unpack pending writes queue entry payload", ex) + .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) + .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()); + } + return new PendingWritesQueueEntry<>(keyTuple, payload, storedPayload.getTypeUrl(), item.getEnqueueTimestamp()); + } + + /** + * Build an entry from a previous format value using the supplied {@code legacyDecoder}. + * + * @param keyTuple the key for the queue entry + * @param valueBytes the serialized bytes to decode + * @param legacyDecoder the read-path decoder for non-current-format entries + */ + @Nonnull + private PendingWritesQueueEntry legacyFormatEntry(@Nonnull Tuple keyTuple, + @Nonnull byte[] valueBytes, + @Nonnull Function legacyDecoder) { final T payload; try { payload = legacyDecoder.apply(valueBytes); @@ -419,7 +438,7 @@ private PendingWritesQueueEntry toQueueEntry(@Nonnull Tuple keyTuple, @Nonnul /** * Parse the stored bytes as the current envelope, returning {@code null} if they are not a * valid envelope (e.g. an entry written by a predecessor implementation in a different - * layout, which the {@link #legacyDecoder} handles instead). + * layout, which the read-path legacy decoder handles instead). */ @Nullable private static PendingWritesQueueProto.PendingWriteItem tryParseEnvelope(@Nonnull byte[] valueBytes) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java index 90a4294dd4..bdb4970f73 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java @@ -50,11 +50,8 @@ public final class PendingWritesQueueEntry { @Nonnull T payload, @Nonnull String payloadTypeUrl, long enqueueTimestamp) { - // Sanity-check the key shape. The queue itself always writes (incarnation, versionstamp) - // (size 2). Reads additionally tolerate a bare (versionstamp) key (size 1) so that entries - // written by a predecessor queue that did not prefix an incarnation can still be read back - // — see the legacy-decoder path on the read side. - if (keyTuple.size() != 1 && keyTuple.size() != 2) { + // Sanity-check the key shape — the queue always writes (incarnation, versionstamp). + if (keyTuple.size() != 2) { throw new RecordCoreStorageException("Unexpected queue key shape") .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); } @@ -69,14 +66,8 @@ public Tuple getKeyTuple() { return keyTuple; } - /** - * Returns the incarnation prefix of this entry's key, or {@code 0} when the key carries no - * incarnation (a legacy, size-1 key). - * - * @return the incarnation, or {@code 0} for an incarnation-less legacy key - */ public int getIncarnation() { - return keyTuple.size() == 2 ? (int)keyTuple.getLong(0) : 0; + return (int)keyTuple.getLong(0); } public long getEnqueueTimestamp() { diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java index d40dd370e9..37c98ef7eb 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java @@ -31,14 +31,13 @@ import com.apple.foundationdb.record.provider.foundationdb.SplitHelper; import com.apple.foundationdb.subspace.Subspace; import com.apple.foundationdb.tuple.Tuple; -import com.google.protobuf.InvalidProtocolBufferException; import com.apple.test.Tags; +import com.google.protobuf.InvalidProtocolBufferException; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import java.util.Arrays; import java.util.List; import java.util.function.Function; @@ -86,14 +85,14 @@ class PendingWritesQueueLegacyFallbackTest extends FDBRecordStoreTestBase { void readsMixOfNewAndLegacyEntries() { PendingWritesQueue queue; try (FDBRecordContext context = openContext()) { - queue = getQueue(context, BARE_LEGACY_DECODER); + queue = getQueue(context); queue.enqueue(context, payload("new-0"), 0).join(); writeRawLegacyEntry(context, bareLegacy("legacy-1")); queue.enqueue(context, payload("new-2"), 0).join(); commit(context); } - List> entries = readAll(queue); + List> entries = readAll(queue, BARE_LEGACY_DECODER); assertEquals(3, entries.size()); assertEquals("new-0", entries.get(0).getPayload().getLabel()); assertEquals("legacy-1", entries.get(1).getPayload().getLabel()); // via the legacy decoder @@ -108,26 +107,26 @@ void readsMixOfNewAndLegacyEntries() { void legacyEntryThatFailsEnvelopeParseUsesDecoder() { PendingWritesQueue queue; try (FDBRecordContext context = openContext()) { - queue = getQueue(context, PREFIXED_LEGACY_DECODER); + queue = getQueue(context); writeRawLegacyEntry(context, prefixedLegacy("wrapped-legacy")); queue.enqueue(context, payload("new"), 0).join(); commit(context); } - List> entries = readAll(queue); + List> entries = readAll(queue, PREFIXED_LEGACY_DECODER); assertEquals(2, entries.size()); assertEquals("wrapped-legacy", entries.get(0).getPayload().getLabel()); assertEquals("new", entries.get(1).getPayload().getLabel()); } /** - * A non-current-format entry with no legacy decoder configured is a storage error. + * A non-current-format entry read without a legacy decoder is a storage error. */ @Test void nonCurrentEntryWithoutLegacyDecoderThrows() { PendingWritesQueue queue; try (FDBRecordContext context = openContext()) { - queue = getQueue(context, null); + queue = getQueue(context); writeRawLegacyEntry(context, bareLegacy("orphan")); commit(context); } @@ -164,10 +163,9 @@ private static byte[] prefixedLegacy(@Nonnull String text) { } @Nonnull - private PendingWritesQueue getQueue(@Nonnull FDBRecordContext context, - @Nullable Function legacyDecoder) { + private PendingWritesQueue getQueue(@Nonnull FDBRecordContext context) { return new PendingWritesQueue<>(queueSubspaceFor(context), counterSubspaceFor(context), 0, - TestQueuePayload.class, legacyDecoder); + TestQueuePayload.class); } @Nonnull @@ -182,9 +180,10 @@ private Subspace counterSubspaceFor(@Nonnull FDBRecordContext context) { @Nonnull private List> readAll( - @Nonnull PendingWritesQueue queue) { + @Nonnull PendingWritesQueue queue, + @Nonnull Function legacyDecoder) { try (FDBRecordContext context = openContext()) { - return queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null).asList().join(); + return queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, legacyDecoder).asList().join(); } } diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneEvents.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneEvents.java index 891fe33ce0..f02a8c7e10 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneEvents.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/LuceneEvents.java @@ -186,6 +186,8 @@ public enum Waits implements StoreTimer.Wait { WAIT_LUCENE_REPLAY_QUEUE("lucene replay pending writes queue"), /** Get the number of entries in the pending writes queue. */ WAIT_LUCENE_GET_QUEUE_SIZE("lucene get pending writes queue size"), + /** Wait for an enqueue into the pending writes queue (includes the capacity-check read). */ + WAIT_LUCENE_PENDING_QUEUE_WRITE("lucene pending queue write"), WAIT_LUCENE_SERIALIZE("lucene serialize data"), WAIT_LUCENE_DESERIALIZE("lucene deserialize data"); private final String title; diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java index 9f3b46ac8d..5d5fe58afc 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java @@ -94,6 +94,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -1054,19 +1055,30 @@ public CompletableFuture clearOngoingMergeIndicatorIfQueueEmptyAsync() { } /** - * Create the pending-writes queue used for reading the queue (drain, replay, size, - * emptiness) and for writing new-format entries. This is the generic + * Create the pending-writes queue used for reading the queue (drain, replay, size) + * and for writing new-format entries. This is the generic * {@link PendingWritesQueue} carrying a {@link LucenePendingWriteQueueProto.PendingWriteItem} - * payload. It reads both the new (versioned {@code Any}-payload) format and the legacy - * (serializer-wrapped) format — the latter via a legacy decoder. + * payload. It only ever writes the new (versioned {@code Any}-payload) format. * * @return the generic pending-writes queue bound to the Lucene payload type */ public PendingWritesQueue createPendingWritesQueue() { return new PendingWritesQueue<>( pendingWritesQueueSubspace, pendingQueueSizeSubspace, maxPendingQueueSize, - LucenePendingWriteQueueProto.PendingWriteItem.class, - rawBytes -> PendingWritesQueueHelper.decodeLegacyItem(serializer, rawBytes)); + LucenePendingWriteQueueProto.PendingWriteItem.class); + } + + /** + * The read-path decoder that turns a legacy (serializer-wrapped) queue value into a + * {@link LucenePendingWriteQueueProto.PendingWriteItem}. Pass this to + * {@link PendingWritesQueue#getQueueCursor(FDBRecordContext, com.apple.foundationdb.record.ScanProperties, byte[], java.util.function.Function)} + * so the reader understands entries written before the switch to the new format. + * + * @return the legacy-format decoder for this directory's serializer + */ + @Nonnull + public Function getPendingWriteQueueLegacyDecoder() { + return rawBytes -> PendingWritesQueueHelper.decodeLegacyItem(serializer, rawBytes); } /** @@ -1098,7 +1110,7 @@ public int getMaxPendingWritesToReplay() { * @return {@code true} to write the new format, {@code false} to write the legacy format */ public boolean shouldWritePendingQueueNewFormat() { - return Boolean.TRUE.equals(agilityContext.getPropertyValue(LuceneRecordContextProperties.LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT)); + return Objects.requireNonNullElse(agilityContext.getPropertyValue(LuceneRecordContextProperties.LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT), false); } public 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 02f8b772b5..731c330e22 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 @@ -360,7 +360,7 @@ private CompletableFuture replayQueuedOperations(@Nonnull final FDBRecordC .build() .asScanProperties(false); } - return getPendingWriteQueue().getQueueCursor(context, scanProperties, null) + return getPendingWriteQueue().getQueueCursor(context, scanProperties, null, directory.getPendingWriteQueueLegacyDecoder()) .forEachResult(result -> PendingWritesQueueHelper.replayItem(result.get().getPayload(), indexWriter, state.index)) .thenAccept(lastResult -> { @@ -413,10 +413,7 @@ private PendingWriteQueue getLegacyPendingWriteQueue() { } /** - * Enqueue an INSERT into the pending-writes queue, choosing the on-disk format by - * {@link FDBDirectory#shouldWritePendingQueueNewFormat()}: the new format is written through - * the generic {@link PendingWritesQueue}; the legacy format through the legacy - * {@link PendingWriteQueue}. + * Enqueue an INSERT into the pending-writes queue. * * @param context the record context to enqueue within * @param primaryKey the record's primary key @@ -427,21 +424,11 @@ public void enqueuePendingInsert(@Nonnull final FDBRecordContext context, @Nonnull final Tuple primaryKey, @Nonnull final List fields, final int incarnation) { - if (directory.shouldWritePendingQueueNewFormat()) { - final LucenePendingWriteQueueProto.PendingWriteItem item = PendingWritesQueueHelper.buildPendingWriteItem( - LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, primaryKey, fields, System.currentTimeMillis()); - LuceneConcurrency.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_GET_QUEUE_SIZE, - getPendingWriteQueue().enqueue(context, item, incarnation), context); - } else { - getLegacyPendingWriteQueue().enqueueInsert(context, primaryKey, fields, incarnation); - } + enqueuePending(context, LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, primaryKey, fields, incarnation); } /** - * Enqueue a DELETE into the pending-writes queue, choosing the on-disk format by - * {@link FDBDirectory#shouldWritePendingQueueNewFormat()}: the new format is written through - * the generic {@link PendingWritesQueue}; the legacy format through the legacy - * {@link PendingWriteQueue}. + * Enqueue a DELETE into the pending-writes queue. * * @param context the record context to enqueue within * @param primaryKey the record's primary key to delete @@ -450,13 +437,36 @@ public void enqueuePendingInsert(@Nonnull final FDBRecordContext context, public void enqueuePendingDelete(@Nonnull final FDBRecordContext context, @Nonnull final Tuple primaryKey, final int incarnation) { + enqueuePending(context, LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE, primaryKey, null, incarnation); + } + + /** + * Enqueue a pending operation, choosing the on-disk format by + * {@link FDBDirectory#shouldWritePendingQueueNewFormat()}: the new format is written through + * the generic {@link PendingWritesQueue}; the legacy format through the legacy + * {@link PendingWriteQueue}. + * + * @param context the record context to enqueue within + * @param operationType the operation to enqueue (INSERT or DELETE) + * @param primaryKey the record's primary key + * @param fields the document fields to index for an INSERT, or {@code null} for a DELETE + * @param incarnation the incarnation prefix for the queue key + */ + private void enqueuePending(@Nonnull final FDBRecordContext context, + @Nonnull final LucenePendingWriteQueueProto.PendingWriteItem.OperationType operationType, + @Nonnull final Tuple primaryKey, + @Nullable final List fields, + final int incarnation) { if (directory.shouldWritePendingQueueNewFormat()) { final LucenePendingWriteQueueProto.PendingWriteItem item = PendingWritesQueueHelper.buildPendingWriteItem( - LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE, primaryKey, null, System.currentTimeMillis()); - LuceneConcurrency.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_GET_QUEUE_SIZE, + operationType, primaryKey, fields, System.currentTimeMillis()); + // Wait on the enqueue itself (it includes the capacity-check read), not just the size lookup. + LuceneConcurrency.asyncToSync(LuceneEvents.Waits.WAIT_LUCENE_PENDING_QUEUE_WRITE, getPendingWriteQueue().enqueue(context, item, incarnation), context); - } else { + } else if (operationType == LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE) { getLegacyPendingWriteQueue().enqueueDelete(context, primaryKey, incarnation); + } else { + getLegacyPendingWriteQueue().enqueueInsert(context, primaryKey, fields, incarnation); } } @@ -714,7 +724,8 @@ private CursorFactory executeProperties.setReturnedRowLimit(rowLimit)); // Note: null could have been used instead of continuation as the preceding items should have been deleted. However, // using a continuation will prevent an infinite loop in case of a bug that doesn't clear items. - return pendingWriteQueue.getQueueCursor(store.getContext(), scanProperties, continuation); + return pendingWriteQueue.getQueueCursor(store.getContext(), scanProperties, continuation, + directory.getPendingWriteQueueLegacyDecoder()); }; } From 06959f277c94dfc09947ffbc51867a0a2afd678f Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 9 Jul 2026 15:34:04 -0400 Subject: [PATCH 3/5] Some refactoring to code --- .../foundationdb/queue/PendingWritesQueue.java | 2 +- .../record/lucene/directory/FDBDirectory.java | 14 -------------- .../lucene/directory/FDBDirectoryWrapper.java | 5 +++-- .../directory/PendingWritesQueueHelper.java | 16 ++++++++++++++++ 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java index f267a1035b..037d670897 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java @@ -375,7 +375,7 @@ private PendingWritesQueueEntry toQueueEntry(@Nonnull Tuple keyTuple, @Nonnul .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) .addLogInfo(LogMessageKeys.STORED_VERSION, item == null ? -1 : item.getVersion()); } - return legacyFormatEntry(keyTuple, valueBytes, item, legacyDecoder); + return legacyFormatEntry(keyTuple, valueBytes, legacyDecoder); } /** diff --git a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java index 5d5fe58afc..edfbfcd48c 100644 --- a/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java +++ b/fdb-record-layer-lucene/src/main/java/com/apple/foundationdb/record/lucene/directory/FDBDirectory.java @@ -94,7 +94,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -1068,19 +1067,6 @@ public PendingWritesQueue createP LucenePendingWriteQueueProto.PendingWriteItem.class); } - /** - * The read-path decoder that turns a legacy (serializer-wrapped) queue value into a - * {@link LucenePendingWriteQueueProto.PendingWriteItem}. Pass this to - * {@link PendingWritesQueue#getQueueCursor(FDBRecordContext, com.apple.foundationdb.record.ScanProperties, byte[], java.util.function.Function)} - * so the reader understands entries written before the switch to the new format. - * - * @return the legacy-format decoder for this directory's serializer - */ - @Nonnull - public Function getPendingWriteQueueLegacyDecoder() { - return rawBytes -> PendingWritesQueueHelper.decodeLegacyItem(serializer, rawBytes); - } - /** * Create the legacy {@link PendingWriteQueue} used only to write legacy-format entries until * the deployment has switched over to the new format. Reads do not through this queue. 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 731c330e22..57aaa4e1a5 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 @@ -360,7 +360,8 @@ private CompletableFuture replayQueuedOperations(@Nonnull final FDBRecordC .build() .asScanProperties(false); } - return getPendingWriteQueue().getQueueCursor(context, scanProperties, null, directory.getPendingWriteQueueLegacyDecoder()) + return getPendingWriteQueue().getQueueCursor(context, scanProperties, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())) .forEachResult(result -> PendingWritesQueueHelper.replayItem(result.get().getPayload(), indexWriter, state.index)) .thenAccept(lastResult -> { @@ -725,7 +726,7 @@ private CursorFactory legacyDecoder(@Nonnull LuceneSerializer serializer) { + return rawBytes -> decodeLegacyItem(serializer, rawBytes); + } + /** * Decode the raw stored bytes of a legacy (serializer-wrapped) queue entry into a Lucene * pending-write item. This is the read-side fallback used by the generic From 6c7b3e55cc49a6c65dabc3eed9eba6a7993f453e Mon Sep 17 00:00:00 2001 From: ohad Date: Tue, 14 Jul 2026 14:55:28 -0400 Subject: [PATCH 4/5] Some refactoring to code, fixed tests --- .../queue/PendingWritesQueue.java | 8 +- .../queue/PendingWritesQueueEntry.java | 14 ++ .../PendingWritesQueueLegacyFallbackTest.java | 47 +++- .../lucene/FDBLuceneQueuedDocQueryTest.java | 56 +++-- .../lucene/directory/FDBDirectoryTest.java | 17 +- .../PendingWriteQueueIntegrationTest.java | 234 ++++++++++-------- .../PendingWriteQueueSizeIntegrationTest.java | 100 +++++--- 7 files changed, 296 insertions(+), 180 deletions(-) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java index 037d670897..79922d34d1 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueue.java @@ -409,7 +409,7 @@ private PendingWritesQueueEntry currentFormatEntry(@Nonnull Tuple keyTuple, .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple) .addLogInfo(LogMessageKeys.EXPECTED_TYPE, payloadClass.getName()); } - return new PendingWritesQueueEntry<>(keyTuple, payload, storedPayload.getTypeUrl(), item.getEnqueueTimestamp()); + return new PendingWritesQueueEntry<>(keyTuple, payload, item.getVersion(), storedPayload.getTypeUrl(), item.getEnqueueTimestamp()); } /** @@ -430,9 +430,9 @@ private PendingWritesQueueEntry legacyFormatEntry(@Nonnull Tuple keyTuple, throw new RecordCoreStorageException("Failed to decode legacy pending writes queue entry", ex) .addLogInfo(LogMessageKeys.KEY_TUPLE, keyTuple); } - // Legacy entries were not Any-wrapped and carry no envelope timestamp, so the payload - // type URL is empty and the enqueue timestamp is unavailable at this layer. - return new PendingWritesQueueEntry<>(keyTuple, payload, "", 0L); + // Legacy entries were not Any-wrapped and carry no versioned envelope, so the version is + // 0, the payload type URL is empty, and the enqueue timestamp is unavailable at this layer. + return new PendingWritesQueueEntry<>(keyTuple, payload, 0, "", 0L); } /** diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java index bdb4970f73..9182da354b 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueEntry.java @@ -42,12 +42,14 @@ public final class PendingWritesQueueEntry { private final Tuple keyTuple; @Nonnull private final T payload; + private final int version; @Nonnull private final String payloadTypeUrl; private final long enqueueTimestamp; PendingWritesQueueEntry(@Nonnull Tuple keyTuple, @Nonnull T payload, + int version, @Nonnull String payloadTypeUrl, long enqueueTimestamp) { // Sanity-check the key shape — the queue always writes (incarnation, versionstamp). @@ -57,6 +59,7 @@ public final class PendingWritesQueueEntry { } this.keyTuple = keyTuple; this.payload = payload; + this.version = version; this.payloadTypeUrl = payloadTypeUrl; this.enqueueTimestamp = enqueueTimestamp; } @@ -70,6 +73,17 @@ public int getIncarnation() { return (int)keyTuple.getLong(0); } + /** + * Returns the on-disk schema version of this entry: the envelope {@code version} for a + * current-format entry, or {@code 0} for an entry read via the legacy decoder (which has no + * versioned envelope). + * + * @return the stored schema version, or {@code 0} for a legacy-format entry + */ + public int getVersion() { + return version; + } + public long getEnqueueTimestamp() { return enqueueTimestamp; } diff --git a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java index 37c98ef7eb..41e0fad7e2 100644 --- a/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java +++ b/fdb-record-layer-core/src/test/java/com/apple/foundationdb/record/provider/foundationdb/queue/PendingWritesQueueLegacyFallbackTest.java @@ -21,7 +21,6 @@ package com.apple.foundationdb.record.provider.foundationdb.queue; import com.apple.foundationdb.MutationType; -import com.apple.foundationdb.record.PendingWritesQueueTestProto.OtherTestQueuePayload; import com.apple.foundationdb.record.PendingWritesQueueTestProto.TestQueuePayload; import com.apple.foundationdb.record.RecordCoreStorageException; import com.apple.foundationdb.record.ScanProperties; @@ -51,27 +50,26 @@ * implementation in a different value layout. On read, an entry that is not the current format * (parse fails, or it has no {@code version >= 1}) is handed to the legacy decoder. * - *

Here the simulated legacy layout is a bare {@link OtherTestQueuePayload} (optionally wrapped - * with a one-byte prefix to stand in for a caller-specific serializer); the decoder maps it to a - * {@link TestQueuePayload}, so all entries surface as the queue's bound payload type.

+ *

Here the simulated legacy layout is a directly-serialized {@link TestQueuePayload}, optionally prefixed with a + * one-byte stand-in for such a serializer; the decoder parses it straight back into a + * {@link TestQueuePayload}.

*/ @Tag(Tags.RequiresFDB) class PendingWritesQueueLegacyFallbackTest extends FDBRecordStoreTestBase { - /** Legacy layout: a bare {@link OtherTestQueuePayload}. Decoder maps {@code text -> label}. */ + /** Legacy layout: a directly-serialized {@link TestQueuePayload}, parsed straight back. */ private static final Function BARE_LEGACY_DECODER = raw -> { try { - return TestQueuePayload.newBuilder().setLabel(OtherTestQueuePayload.parseFrom(raw).getText()).build(); + return TestQueuePayload.parseFrom(raw); } catch (InvalidProtocolBufferException e) { throw new IllegalStateException(e); } }; - /** Legacy layout: a one-byte {@code 0x00} prefix (a stand-in serializer) + {@link OtherTestQueuePayload}. */ + /** Legacy layout: a one-byte {@code 0x00} prefix (a stand-in serializer) + serialized {@link TestQueuePayload}. */ private static final Function PREFIXED_LEGACY_DECODER = raw -> { try { - byte[] unwrapped = Arrays.copyOfRange(raw, 1, raw.length); - return TestQueuePayload.newBuilder().setLabel(OtherTestQueuePayload.parseFrom(unwrapped).getText()).build(); + return TestQueuePayload.parseFrom(Arrays.copyOfRange(raw, 1, raw.length)); } catch (InvalidProtocolBufferException e) { throw new IllegalStateException(e); } @@ -138,10 +136,35 @@ void nonCurrentEntryWithoutLegacyDecoderThrows() { } } + /** + * An entry written with an old-format key that lacks the incarnation prefix (a size-1 + * versionstamp-only key) is rejected on read. + */ + @Test + void entryWithoutIncarnationInKeyThrows() { + PendingWritesQueue queue; + try (FDBRecordContext context = openContext()) { + queue = getQueue(context); + final FDBRecordVersion recordVersion = FDBRecordVersion.incomplete(context.claimLocalVersion()); + // Old-format key: versionstamp only, with no incarnation prefix. + writeRawEntry(context, bareLegacy("no-incarnation"), Tuple.from(recordVersion.toVersionstamp())); + commit(context); + } + + try (FDBRecordContext context = openContext()) { + Assertions.assertThatThrownBy( + () -> queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, BARE_LEGACY_DECODER).asList().join()) + .hasCauseInstanceOf(RecordCoreStorageException.class); + } + } + private void writeRawLegacyEntry(@Nonnull FDBRecordContext context, @Nonnull byte[] rawValue) { + final FDBRecordVersion recordVersion = FDBRecordVersion.incomplete(context.claimLocalVersion()); + writeRawEntry(context, rawValue, Tuple.from(0, recordVersion.toVersionstamp())); + } + + private void writeRawEntry(@Nonnull FDBRecordContext context, @Nonnull byte[] rawValue, @Nonnull Tuple keyTuple) { Subspace queueSubspace = queueSubspaceFor(context); - FDBRecordVersion recordVersion = FDBRecordVersion.incomplete(context.claimLocalVersion()); - Tuple keyTuple = Tuple.from(0, recordVersion.toVersionstamp()); SplitHelper.saveWithSplit(context, queueSubspace, keyTuple, rawValue, null, true, false, false, null, null); // Keep the size counter consistent with the entry we wrote directly. context.ensureActive().mutate(MutationType.ADD, counterSubspaceFor(context).pack(), @@ -150,7 +173,7 @@ private void writeRawLegacyEntry(@Nonnull FDBRecordContext context, @Nonnull byt @Nonnull private static byte[] bareLegacy(@Nonnull String text) { - return OtherTestQueuePayload.newBuilder().setText(text).build().toByteArray(); + return TestQueuePayload.newBuilder().setLabel(text).build().toByteArray(); } @Nonnull diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java index 471c29c7ed..f417b0b706 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/FDBLuceneQueuedDocQueryTest.java @@ -26,6 +26,7 @@ import com.apple.foundationdb.record.TestRecordsTextProto; import com.apple.foundationdb.record.lucene.directory.FDBDirectoryWrapper; import com.apple.foundationdb.record.lucene.directory.PendingWriteQueue; +import com.apple.foundationdb.record.lucene.directory.PendingWritesQueueHelper; import com.apple.foundationdb.record.metadata.Index; import com.apple.foundationdb.record.provider.common.text.TextSamples; import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext; @@ -33,12 +34,16 @@ import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreTestBase; import com.apple.foundationdb.record.provider.foundationdb.indexes.TextIndexTestUtils; import com.apple.foundationdb.record.provider.foundationdb.properties.RecordLayerPropertyStorage; +import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; import com.apple.foundationdb.tuple.Tuple; import com.apple.test.Tags; +import com.google.protobuf.ByteString; +import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -261,7 +266,7 @@ void saveToQueueWithinTransaction() throws Exception { assertThat(actualKeys).isEqualTo(Set.of()); } // Save docs to the index - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); DOCUMENTS.forEach(doc -> enqueueInsert(context, queue, doc)); // Docs are not available try (RecordCursor cursor = recordStore.scanIndex(index, scanBounds, null, scanProperties)) { @@ -277,7 +282,7 @@ void saveToQueueWithinTransaction() throws Exception { private void enqueueInsertAllDocs(Index index) { try (FDBRecordContext context = openContext()) { openRecordStore(context, index); - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); DOCUMENTS.forEach(doc -> enqueueInsert(context, queue, doc)); commit(context); } @@ -286,7 +291,7 @@ private void enqueueInsertAllDocs(Index index) { private void enqueueDeleteAllDocs(Index index) { try (FDBRecordContext context = openContext()) { openRecordStore(context, index); - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); DOCUMENTS.forEach(doc -> enqueueDelete(context, queue, doc)); commit(context); } @@ -295,7 +300,7 @@ private void enqueueDeleteAllDocs(Index index) { private void enqueueDeleteSomeDocs(Index index) { try (FDBRecordContext context = openContext()) { openRecordStore(context, index); - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); enqueueDelete(context, queue, DOCUMENTS.get(2)); commit(context); } @@ -304,7 +309,7 @@ private void enqueueDeleteSomeDocs(Index index) { private void enqueueUpdateDoc(Index index, int docToUpdate, int textToUse) { try (FDBRecordContext context = openContext()) { openRecordStore(context, index); - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); enqueueUpdate(context, queue, DOCUMENTS.get(docToUpdate).getDocId(), DOCUMENTS.get(textToUse).getText()); commit(context); } @@ -321,7 +326,7 @@ private void saveAllDocsInIndex(Index index) { private void saveSomeDocsInQueueAndSomeInIndex(Index index) { try (FDBRecordContext context = openContext()) { openRecordStore(context, index); - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); DOCUMENTS.subList(0, 3).forEach(doc -> enqueueInsert(context, queue, doc)); DOCUMENTS.subList(3, 6).forEach(recordStore::saveRecord); commit(context); @@ -331,7 +336,7 @@ private void saveSomeDocsInQueueAndSomeInIndex(Index index) { private void clearAllDocsFromQueue(Index index) { try (FDBRecordContext context = openContext()) { openRecordStore(context, index); - PendingWriteQueue queue = getPendingWriteQueue(recordStore, index); + PendingWritesQueue queue = getPendingWriteQueue(recordStore, index); emptyQueue(context, queue); commit(context); } @@ -351,23 +356,23 @@ private void scanAndCompareDocs(Index index, String searchTerm, Set expect } } - private void enqueueInsert(FDBRecordContext context, PendingWriteQueue queue, TestRecordsTextProto.SimpleDocument doc) { + private void enqueueInsert(FDBRecordContext context, @MonotonicNonNull PendingWritesQueue queue, TestRecordsTextProto.SimpleDocument doc) { List fields = toDocumentFields(doc); - queue.enqueueInsert(context, Tuple.from(doc.getDocId()), fields, 0); + queue.enqueue(context, payload(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, doc.getDocId(), fields), 0).join(); } - private void enqueueUpdate(FDBRecordContext context, PendingWriteQueue queue, long docId, String text) { + private void enqueueUpdate(FDBRecordContext context, @MonotonicNonNull PendingWritesQueue queue, long docId, String text) { List fields = toDocumentFields(text); // Update is delete followed by insert - queue.enqueueDelete(context, Tuple.from(docId), 0); - queue.enqueueInsert(context, Tuple.from(docId), fields, 0); + queue.enqueue(context, payload(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE, docId, Collections.emptyList()), 0).join(); + queue.enqueue(context, payload(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.INSERT, docId, fields), 0).join(); } - private void enqueueDelete(FDBRecordContext context, PendingWriteQueue queue, TestRecordsTextProto.SimpleDocument doc) { - queue.enqueueDelete(context, Tuple.from(doc.getDocId()), 0); + private void enqueueDelete(FDBRecordContext context, @MonotonicNonNull PendingWritesQueue queue, TestRecordsTextProto.SimpleDocument doc) { + queue.enqueue(context, payload(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE, doc.getDocId(), Collections.emptyList()), 0).join(); } - private void emptyQueue(FDBRecordContext context, PendingWriteQueue queue) { + private void emptyQueue(FDBRecordContext context, @MonotonicNonNull PendingWritesQueue queue) { queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) .forEach(entry -> queue.clearEntry(context, entry)).join(); } @@ -394,12 +399,10 @@ private static FDBDirectoryWrapper getDirectoryWrapper(final LuceneIndexMaintain return indexMaintainer.getDirectoryManager().getDirectoryWrapper(groupingKey, partitionId); } - private PendingWriteQueue getPendingWriteQueue(FDBRecordStore store, Index index) { + private PendingWritesQueue getPendingWriteQueue(FDBRecordStore store, Index index) { LuceneIndexMaintainer indexMaintainer = getIndexMaintainer(store, index); final FDBDirectoryWrapper directoryWrapper = getDirectoryWrapper(indexMaintainer); - // This test drives enqueue directly through the legacy queue (legacy on-disk format); the - // query path replays through the generic PendingWritesQueue, which reads the legacy format. - return directoryWrapper.getDirectory().createLegacyPendingWritesQueue(); + return directoryWrapper.getDirectory().createPendingWritesQueue(); } protected FDBRecordStore openRecordStore(FDBRecordContext context, Index index) { @@ -411,4 +414,19 @@ protected FDBRecordStore openRecordStore(FDBRecordContext context, Index index) }); return recordStore; } + + private LucenePendingWriteQueueProto.PendingWriteItem payload( + final LucenePendingWriteQueueProto.PendingWriteItem.OperationType operationType, + final long pk, + final List fields) { + final LucenePendingWriteQueueProto.PendingWriteItem.Builder builder = LucenePendingWriteQueueProto.PendingWriteItem.newBuilder() + .setEnqueueTimestamp(System.currentTimeMillis()) + .setOperationType(operationType) + .setPrimaryKey(ByteString.copyFrom(Tuple.from(pk).pack())); + for (LuceneDocumentFromRecord.DocumentField field : fields) { + builder.addFields(PendingWritesQueueHelper.toProtoField(field)); + } + + return builder.build(); + } } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java index 68a080b22b..088d3cdcfe 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/FDBDirectoryTest.java @@ -37,8 +37,8 @@ import com.apple.foundationdb.record.provider.foundationdb.FDBStoreTimer; import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueueEntry; -import com.apple.foundationdb.tuple.Tuple; import com.apple.test.Tags; +import com.google.protobuf.ByteString; import org.assertj.core.api.Assertions; import org.hamcrest.Matchers; import org.junit.jupiter.api.Tag; @@ -394,11 +394,8 @@ void testQueueIndicatorLifeCycle() { assertTrue(newDirectory.shouldUseQueue(), "shouldUseQueue should return true in new transaction after commit"); - PendingWriteQueue queue = newDirectory.createLegacyPendingWritesQueue(); - queue.enqueueInsert(newContext, - Tuple.from("testDoc", 1), - createTestFields(), - 0); + PendingWritesQueue queue = newDirectory.createPendingWritesQueue(); + queue.enqueue(newContext, payload(), 0).join(); newContext.commit(); } @@ -461,6 +458,14 @@ void testQueueIndicatorLifeCycle() { } } + private LucenePendingWriteQueueProto.PendingWriteItem payload() { + return LucenePendingWriteQueueProto.PendingWriteItem.newBuilder() + .setEnqueueTimestamp(System.currentTimeMillis()) + .setOperationType(LucenePendingWriteQueueProto.PendingWriteItem.OperationType.DELETE) + .setPrimaryKey(ByteString.copyFrom("Hello".getBytes())) + .build(); + } + private Map indexOptionAllowQueue() { return Map.of(LuceneIndexOptions.ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE, "true"); } diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java index 93f5d82fff..48ecfa87b9 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueIntegrationTest.java @@ -20,9 +20,7 @@ package com.apple.foundationdb.record.lucene.directory; -import com.apple.foundationdb.KeyValue; import com.apple.foundationdb.record.IndexEntry; -import com.apple.foundationdb.record.PendingWritesQueueProto; import com.apple.foundationdb.record.RecordCursor; import com.apple.foundationdb.record.ScanProperties; import com.apple.foundationdb.record.TestHelpers; @@ -52,12 +50,12 @@ import com.apple.foundationdb.tuple.Tuple; import com.apple.test.BooleanSource; import com.apple.test.Tags; -import com.google.protobuf.InvalidProtocolBufferException; import org.apache.lucene.index.IndexWriter; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -77,7 +75,6 @@ import static com.apple.foundationdb.record.lucene.LuceneIndexOptions.ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE; import static com.apple.foundationdb.record.lucene.LuceneIndexOptions.INDEX_PARTITION_BY_FIELD_NAME; import static com.apple.foundationdb.record.lucene.LuceneIndexOptions.INDEX_PARTITION_HIGH_WATERMARK; -import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.SIMPLE_TEXT_SUFFIXES; import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.simpleTextSuffixesIndex; import static com.apple.foundationdb.record.metadata.Key.Expressions.concat; import static com.apple.foundationdb.record.metadata.Key.Expressions.field; @@ -97,8 +94,35 @@ */ @Tag(Tags.RequiresFDB) public class PendingWriteQueueIntegrationTest extends FDBRecordStoreTestBase { - @Test - void testPendingQueueSimple() { + /** + * The non-partitioned index used by these tests. Incarnation is enabled so queue keys are + * {@code (incarnation, versionstamp)} — the shape the generic reader requires. We assume the + * old incarnation-less key format is fully converted before this code ships. + */ + private static final Index SIMPLE_TEXT_SUFFIXES = + simpleTextSuffixesIndex(options -> + options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true")); + + /** + * Open a context whose enqueues write the new (versioned {@code Any}-payload) queue format when + * {@code writeNewFormat} is {@code true}, or the legacy format when {@code false}. Reads + * understand both formats regardless. + * + * @param writeNewFormat whether enqueues within this context write the new format + * @return the opened context + */ + private FDBRecordContext openContext(boolean writeNewFormat) { + if (writeNewFormat) { + return openContext(RecordLayerPropertyStorage.newBuilder() + .addProp(LuceneRecordContextProperties.LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT, true) + .build()); + } + return openContext(); + } + + @ParameterizedTest + @BooleanSource + void testPendingQueueSimple(boolean writeNewFormat) { // Test simple non-partitioned pending queue life cycle final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); @@ -111,7 +135,7 @@ void testPendingQueueSimple() { setOngoingMergeIndicator(schemaSetup, index, null, null); // Write a record - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord( LuceneIndexTestUtils.createSimpleDocument(1001L, "test document", 1) @@ -136,8 +160,9 @@ void testPendingQueueSimple() { verifyExpectedDocIds(schemaSetup, index, Set.of(1001L)); } - @Test - void testPendingQueueSimpleWithDelete() { + @ParameterizedTest + @BooleanSource + void testPendingQueueSimpleWithDelete(boolean writeNewFormat) { // Test pending queue with both INSERT and DELETE operations final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); @@ -150,7 +175,7 @@ void testPendingQueueSimpleWithDelete() { setOngoingMergeIndicator(schemaSetup, index, null, null); // Write multiple records - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord( LuceneIndexTestUtils.createSimpleDocument(1001L, "first document", 1) @@ -162,7 +187,7 @@ void testPendingQueueSimpleWithDelete() { } // Delete one of the records - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.deleteRecord(Tuple.from(1001L)); commit(context); @@ -187,8 +212,9 @@ void testPendingQueueSimpleWithDelete() { verifyExpectedDocIds(schemaSetup, index, Set.of(1002L)); } - @Test - void testPendingQueueMultiDelete() { + @ParameterizedTest + @BooleanSource + void testPendingQueueMultiDelete(boolean writeNewFormat) { final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); final Function schemaSetup = context -> @@ -198,7 +224,7 @@ void testPendingQueueMultiDelete() { // Write multiple records - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord( LuceneIndexTestUtils.createSimpleDocument(1001L, "first document", 1) @@ -213,7 +239,7 @@ void testPendingQueueMultiDelete() { setOngoingMergeIndicator(schemaSetup, index, null, null); // Write multiple records - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord( LuceneIndexTestUtils.createSimpleDocument(1003L, "third document", 1) @@ -225,7 +251,7 @@ void testPendingQueueMultiDelete() { } // Delete of two records, one written before and one during the merge - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.deleteRecord(Tuple.from(1001L)); recordStore.deleteRecord(Tuple.from(1004L)); @@ -253,11 +279,13 @@ void testPendingQueueMultiDelete() { } @Disabled("This test fails due to https://github.com/FoundationDB/fdb-record-layer/issues/3885") - @Test - void pendingQueueTestMultiplePartitions() { + @ParameterizedTest + @BooleanSource + void pendingQueueTestMultiplePartitions(boolean writeNewFormat) { // Test pending queue with partitioned index - documents in different partitions final Map options = Map.of( ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE, "true", + LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true", INDEX_PARTITION_BY_FIELD_NAME, "timestamp", LuceneIndexOptions.PRIMARY_KEY_SEGMENT_INDEX_V2_ENABLED, "true", INDEX_PARTITION_HIGH_WATERMARK, String.valueOf(3)); @@ -270,7 +298,7 @@ void pendingQueueTestMultiplePartitions() { index, useCascadesPlanner).getLeft(); // Insert a few documents when "ongoing merge" indicator is clear. - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createComplexDocument(2001L, "document foo", 1L, 30L)); @@ -293,7 +321,7 @@ void pendingQueueTestMultiplePartitions() { Tuple primaryKey1; // Insert documents when "ongoing merge" indicator is set. - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); primaryKey1 = recordStore.saveRecord( @@ -310,7 +338,7 @@ void pendingQueueTestMultiplePartitions() { verifyExpectedDocIds(schemaSetup, index, "*:*", groupingKey.getLong(0), Set.of(1001L, 1002L, 1003L, 2001L, 2002L, 2003L)); // Delete one document from partition 0 - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.deleteRecord(primaryKey1); commit(context); @@ -347,11 +375,13 @@ void pendingQueueTestMultiplePartitions() { verifyPartitionCount(schemaSetup, index, groupingKey, List.of(2, 3)); } - @Test - void testPendingQueueAcrossFivePartitions() { + @ParameterizedTest + @BooleanSource + void testPendingQueueAcrossFivePartitions(boolean writeNewFormat) { // Test pending queue in one of 5 partitions final Map options = Map.of( ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE, "true", + LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true", INDEX_PARTITION_BY_FIELD_NAME, "timestamp", LuceneIndexOptions.PRIMARY_KEY_SEGMENT_INDEX_V2_ENABLED, "true", INDEX_PARTITION_HIGH_WATERMARK, String.valueOf(4)); // 4 docs per partition @@ -369,7 +399,7 @@ void testPendingQueueAcrossFivePartitions() { // Write initial documents to create 5 partitions (before queue enabled) // Each partition gets 4 documents with different timestamp ranges Map> partitionPrimaryKeys = new HashMap<>(); - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); for (int partition = 0; partition < numPartitions; partition++) { @@ -399,7 +429,7 @@ void testPendingQueueAcrossFivePartitions() { setOngoingMergeIndicator(schemaSetup, index, groupingKey, 3); // Delete one document from each partition (partition 3 will be queued) - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); for (int partition = 0; partition < numPartitions; partition++) { // Delete first document from each partition @@ -419,7 +449,7 @@ void testPendingQueueAcrossFivePartitions() { } // Add a few docs to partition 4 - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); long partition = 4; long baseTimestamp = 1000L + (partition * 100); @@ -450,11 +480,13 @@ void testPendingQueueAcrossFivePartitions() { verifyPartitionCount(schemaSetup, index, groupingKey, List.of(3, 4, 3, 3, 3, 3)); } - @Test - void testPendingQueueAcrossMultiplePartitions() { + @ParameterizedTest + @BooleanSource + void testPendingQueueAcrossMultiplePartitions(boolean writeNewFormat) { // Test pending queue with operations queued across multiple partitions final Map options = Map.of( ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE, "true", + LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true", INDEX_PARTITION_BY_FIELD_NAME, "timestamp", LuceneIndexOptions.PRIMARY_KEY_SEGMENT_INDEX_V2_ENABLED, "true", INDEX_PARTITION_HIGH_WATERMARK, String.valueOf(5)); // Small watermark for simple test @@ -472,7 +504,7 @@ void testPendingQueueAcrossMultiplePartitions() { // Write initial documents to create 2 partitions (before queue enabled) Map primaryKeys = new HashMap<>(); - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); for (long i = 3001L; i <= 3005L; i++) { Tuple primaryKey = recordStore.saveRecord(LuceneIndexTestUtils.createComplexDocument(i, "doc p1-" + i, 1L, 10L + i)).getPrimaryKey(); @@ -493,7 +525,7 @@ void testPendingQueueAcrossMultiplePartitions() { setOngoingMergeIndicator(schemaSetup, index, groupingKey, partition1); // Delete one document from each partition (will be queued) - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.deleteRecord(primaryKeys.get(2001L)); // Delete from partition 0 recordStore.deleteRecord(primaryKeys.get(3001L)); // Delete from partition 1 @@ -527,8 +559,9 @@ void testPendingQueueAcrossMultiplePartitions() { groupingKey.getLong(0), Set.of(2002L, 2003L, 2004L, 2005L, 3002L, 3003L, 3004L, 3005L)); } - @Test - void testPendingQueueWithUpdate() { + @ParameterizedTest + @BooleanSource + void testPendingQueueWithUpdate(boolean writeNewFormat) { // Test update operation in pending queue final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); @@ -538,7 +571,7 @@ void testPendingQueueWithUpdate() { index, useCascadesPlanner).getLeft(); // Insert document before queue is enabled - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord( LuceneIndexTestUtils.createSimpleDocument(1001L, "original text", 1) @@ -550,7 +583,7 @@ void testPendingQueueWithUpdate() { setOngoingMergeIndicator(schemaSetup, index, null, null); // Update the document - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord( LuceneIndexTestUtils.createSimpleDocument(1001L, "updated text", 1) @@ -578,8 +611,9 @@ void testPendingQueueWithUpdate() { verifyExpectedDocIds(schemaSetup, index, "original", null, Set.of()); } - @Test - void testPendingQueueMixedOperations() { + @ParameterizedTest + @BooleanSource + void testPendingQueueMixedOperations(boolean writeNewFormat) { // Test INSERT, UPDATE, DELETE sequence on same document final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); @@ -592,21 +626,21 @@ void testPendingQueueMixedOperations() { setOngoingMergeIndicator(schemaSetup, index, null, null); // INSERT - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "version 1", 1)); commit(context); } // UPDATE (generates DELETE + INSERT since doc was inserted while queue was active) - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "version 2", 1)); commit(context); } // DELETE - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.deleteRecord(Tuple.from(1001L)); commit(context); @@ -632,8 +666,9 @@ void testPendingQueueMixedOperations() { verifyExpectedDocIds(schemaSetup, index, Set.of()); } - @Test - void testQueryTransactionNotClosed() { + @ParameterizedTest + @BooleanSource + void testQueryTransactionNotClosed(boolean writeNewFormat) { // This test runs a query with queue elements replayed but does not commit the transaction // The DirectoryManager listens to close hooks to figure out that it needs to close all resources // (or else the read only transaction remains open) @@ -648,14 +683,14 @@ void testQueryTransactionNotClosed() { setOngoingMergeIndicator(schemaSetup, index, null, null); // INSERT - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "version 1", 1)); commit(context); } // Run a query (with replay) - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); LuceneScanBounds scanBounds = LuceneIndexTestUtils.fullTextSearch(recordStore, index, "*:*", false); @@ -717,8 +752,9 @@ void pendingQueueTestDrainException2() { }); } - @Test - void testMergeRequiredIndexesWithPendingQueue() { + @ParameterizedTest + @BooleanSource + void testMergeRequiredIndexesWithPendingQueue(boolean writeNewFormat) { // Test that store's getMergeRequiredIndexes does the right thing final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); @@ -728,7 +764,7 @@ void testMergeRequiredIndexesWithPendingQueue() { index, useCascadesPlanner).getLeft(); // Insert without queue indicator - should request deferred merge - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "first document", 1)); @@ -743,7 +779,7 @@ void testMergeRequiredIndexesWithPendingQueue() { setOngoingMergeIndicator(schemaSetup, index, null, null); // Insert with queue indicator - should request deferred merge - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1002L, "second document", 1)); @@ -755,7 +791,7 @@ void testMergeRequiredIndexesWithPendingQueue() { } // Call explicit merge - should not request deferred merge - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); final LuceneIndexMaintainer indexMaintainer = getIndexMaintainer(recordStore, index); indexMaintainer.mergeIndex().join(); @@ -768,8 +804,8 @@ void testMergeRequiredIndexesWithPendingQueue() { } @ParameterizedTest - @BooleanSource - void testConcurrentMixedOperations(boolean withMerge) { + @CsvSource({"false,false", "false,true", "true,false", "true,true"}) + void testConcurrentMixedOperations(boolean withMerge, boolean writeNewFormat) { // Test concurrent INSERT/UPDATE/DELETE operations from multiple threads while in pending queue mode // Note - if multiple partitions mode is enabled, concurrently attempting to adjust the documents count will cause // commit conflicts. This is "not worse" than what happens without an ongoing merge. @@ -781,7 +817,7 @@ void testConcurrentMixedOperations(boolean withMerge) { index, useCascadesPlanner).getLeft(); // Setup: Create initial records before queue mode - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); for (long i = 1001L; i <= 1010L; i++) { recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(i, "original " + i, 1)); @@ -800,7 +836,7 @@ void testConcurrentMixedOperations(boolean withMerge) { IntStream.rangeClosed(1, numThreads).parallel().forEach(threadId -> { snooze(1); // allow other threads to kick in - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); long baseId = 2000L + (threadId * 100); @@ -838,7 +874,7 @@ void testConcurrentMixedOperations(boolean withMerge) { // Verify queue contains all operations // Expected: 10 INSERTs (new) + 5*2 (UPDATE=DELETE+INSERT) + 5 DELETEs = 25 operations - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); IndexMaintainerState state = new IndexMaintainerState(recordStore, index, recordStore.getIndexMaintenanceFilter()); @@ -846,7 +882,8 @@ void testConcurrentMixedOperations(boolean withMerge) { FDBDirectory directory = directoryManager.getDirectory(null, null); PendingWritesQueue queue = directory.createPendingWritesQueue(); - List> entries = queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) + List> entries = queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())) .asList().join(); // Count operation types @@ -934,10 +971,11 @@ void testFailedLockMergeShouldNotDrain() { @ParameterizedTest @BooleanSource - void testPendingQueueIncarnationIndexOption(boolean incarnationEnabled) { - // Verify that the PENDING_WRITE_QUEUE_INCARNATION_ENABLED index option flows through to the queue - final Index index = simpleTextSuffixesIndex(options -> - options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, Boolean.toString(incarnationEnabled))); + void testPendingQueueIncarnationIndexOption(boolean writeNewFormat) { + // The queue key always carries an incarnation prefix (incarnation, versionstamp): we assume + // the old incarnation-less key format is fully converted before this code ships, so the + // generic reader requires the size-2 key shape. + final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); final Function schemaSetup = context -> LuceneIndexTestUtils.rebuildIndexMetaData(context, path, @@ -948,25 +986,25 @@ void testPendingQueueIncarnationIndexOption(boolean incarnationEnabled) { setOngoingMergeIndicator(schemaSetup, index, null, null); // Write a record — goes through the index option path - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "test document", 1)); commit(context); } - // Verify record is in queue and key tuple size reflects the incarnation option - try (FDBRecordContext context = openContext()) { + // Verify record is in queue and the key is (incarnation, versionstamp) + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); IndexMaintainerState state = new IndexMaintainerState(recordStore, index, recordStore.getIndexMaintenanceFilter()); FDBDirectory directory = FDBDirectoryManager.getManager(state).getDirectory(null, null); PendingWritesQueue queue = directory.createPendingWritesQueue(); List> entries = - queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null).asList().join(); + queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())).asList().join(); assertEquals(1, entries.size()); - // With incarnation: key is (incarnation, versionstamp); without: key is (versionstamp) - int expectedKeySize = incarnationEnabled ? 2 : 1; - assertEquals(expectedKeySize, entries.get(0).getKeyTuple().size()); + // Key is always (incarnation, versionstamp) — size 2. + assertEquals(2, entries.get(0).getKeyTuple().size()); commit(context); } @@ -980,8 +1018,9 @@ void testPendingQueueIncarnationIndexOption(boolean incarnationEnabled) { verifyExpectedDocIds(schemaSetup, index, Set.of(1001L)); } - @Test - void testMergeDrainsMultipleIncarnations() { + @ParameterizedTest + @BooleanSource + void testMergeDrainsMultipleIncarnations(boolean writeNewFormat) { final Index index = SIMPLE_TEXT_SUFFIXES; final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); final Function schemaSetup = context -> @@ -993,14 +1032,14 @@ void testMergeDrainsMultipleIncarnations() { setOngoingMergeIndicator(schemaSetup, index, null, null); // Incarnation 0: insert a record - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1001L, "first incarnation", 1)); commit(context); } // Bump incarnation to 5 and insert another record - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.updateIncarnation(current -> 5).join(); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1002L, "second incarnation", 2)); @@ -1008,7 +1047,7 @@ void testMergeDrainsMultipleIncarnations() { } // Bump incarnation to 10 and insert a third record - try (FDBRecordContext context = openContext()) { + try (FDBRecordContext context = openContext(writeNewFormat)) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); recordStore.updateIncarnation(current -> 10).join(); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(1003L, "third incarnation", 3)); @@ -1033,15 +1072,10 @@ void testMergeDrainsMultipleIncarnations() { @Test void testPendingQueueMixedFormatDrains() { - // End-to-end: land a genuine mix of legacy-format and new-format entries in one partition's + // End-to-end: mix legacy-format and new-format entries in one partition's // queue by flipping LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT between writes, then drain // through the real merge path and confirm the index reflects every operation regardless of // the on-disk format each entry was written in. - // - // Incarnation is enabled so that both the legacy queue and the new (generic) queue write - // size-2 (incarnation, versionstamp) keys; this matches the real migration precondition - // (incarnation is in the key before the new-format write is switched on) and keeps the two - // formats ordered by enqueue time in a single subspace. final Index index = simpleTextSuffixesIndex(options -> options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true")); final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); @@ -1115,25 +1149,33 @@ protected static void snooze(int millis) { } /** - * Read the raw queue entries for a non-partitioned index and classify each by on-disk format. + * Read the queue entries for a non-partitioned index (through the queue itself, so both the + * new and legacy on-disk formats are decoded) and classify each by format using the entry's + * schema version: a new-format entry reports the envelope version ({@code >= 1}), while a + * legacy entry decoded via the fallback reports version {@code 0}. * * @param schemaSetup the schema setup function - * @param index the index whose queue subspace is inspected + * @param index the index whose queue is inspected * @return a two-element array {@code [legacyCount, newFormatCount]} */ private int[] countQueueEntriesByFormat(Function schemaSetup, Index index) { try (FDBRecordContext context = openContext()) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); - Subspace queueSubspace = recordStore.indexSubspace(index) - .subspace(Tuple.from(FDBDirectory.PENDING_WRITE_QUEUE_SUBSPACE)); - List kvs = context.ensureActive().getRange(queueSubspace.range()).asList().join(); + IndexMaintainerState state = new IndexMaintainerState(recordStore, index, + recordStore.getIndexMaintenanceFilter()); + FDBDirectory directory = FDBDirectoryManager.getManager(state).getDirectory(null, null); + PendingWritesQueue queue = directory.createPendingWritesQueue(); + + List> entries = + queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())).asList().join(); int legacy = 0; int newFormat = 0; - for (KeyValue kv : kvs) { - if (isNewFormatValue(kv.getValue())) { - newFormat++; - } else { + for (PendingWritesQueueEntry entry : entries) { + if (entry.getVersion() == 0) { legacy++; + } else { + newFormat++; } } commit(context); @@ -1141,22 +1183,6 @@ private int[] countQueueEntriesByFormat(Function= 1}; legacy serializer-wrapped - * bytes either fail to parse as the envelope or parse with {@code version == 0}. - * - * @param value the raw stored value bytes - * @return {@code true} if the bytes are the new format - */ - private static boolean isNewFormatValue(byte[] value) { - try { - return PendingWritesQueueProto.PendingWriteItem.parseFrom(value).getVersion() >= 1; - } catch (InvalidProtocolBufferException e) { - return false; - } - } - private void verifyClearedQueueAndIndicator(Function schemaSetup, Index index, @Nullable Tuple groupingKey, @Nullable Integer partitionId) { try (FDBRecordContext context = openContext()) { FDBRecordStore recordStore = Objects.requireNonNull(schemaSetup.apply(context)); @@ -1169,7 +1195,8 @@ private void verifyClearedQueueAndIndicator(Function queue = directory.createPendingWritesQueue(); List> entries = new ArrayList<>(); - queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) + queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())) .forEach(entries::add).join(); assertTrue(entries.isEmpty(), "Pending queue should have been empty"); commit(context); @@ -1191,7 +1218,8 @@ private void verifyExpectedQueueAndIndicator(Function queue = directory.createPendingWritesQueue(); RecordCursor> queueCursor = queue.getQueueCursor( - recordStore.getContext(), ScanProperties.FORWARD_SCAN, null); + recordStore.getContext(), ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())); assertEquals(expectedOperations, queueCursor.asList().join().stream() diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java index efbcbe643b..ba5dce6139 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSizeIntegrationTest.java @@ -44,9 +44,10 @@ import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueue; import com.apple.foundationdb.record.provider.foundationdb.queue.PendingWritesQueueEntry; import com.apple.foundationdb.tuple.Tuple; +import com.apple.test.BooleanSource; import com.apple.test.Tags; import org.junit.jupiter.api.Tag; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -60,7 +61,7 @@ import static com.apple.foundationdb.record.lucene.LuceneIndexOptions.ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE; import static com.apple.foundationdb.record.lucene.LuceneIndexOptions.INDEX_PARTITION_BY_FIELD_NAME; import static com.apple.foundationdb.record.lucene.LuceneIndexOptions.INDEX_PARTITION_HIGH_WATERMARK; -import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.SIMPLE_TEXT_SUFFIXES; +import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.simpleTextSuffixesIndex; import static com.apple.foundationdb.record.metadata.Key.Expressions.concat; import static com.apple.foundationdb.record.metadata.Key.Expressions.field; import static com.apple.foundationdb.record.metadata.Key.Expressions.function; @@ -76,35 +77,41 @@ */ @Tag(Tags.RequiresFDB) class PendingWriteQueueSizeIntegrationTest extends FDBRecordStoreTestBase { - private final Index index = SIMPLE_TEXT_SUFFIXES; - - @Test - void testQueueLimitsSingleTransaction() { + // Incarnation is enabled so queue keys are (incarnation, versionstamp) — the shape the + // generic reader requires. We assume the old incarnation-less key format is fully converted + // before this code ships. + private final Index index = simpleTextSuffixesIndex(options -> + options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true")); + + @ParameterizedTest + @BooleanSource + void testQueueLimitsSingleTransaction(boolean writeNewFormat) { // Test that too many entries in a single transaction fail to queue // Set "ongoing merge" indicator setOngoingMergeIndicator(index, null, null, simpleMetadataHook()); // Write too many records in a single transaction final int maxQueueSize = 5; - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); for (int i = 0; i < maxQueueSize; i++) { recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(100L + i, "test document", 1)); } - assertThrows(PendingWriteQueue.PendingWritesQueueTooLargeException.class, + assertThrows(tooLargeException(writeNewFormat), () -> recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(999L, "test document", 1))); // The index is now in an inconsistent state, do don't commit } } - @Test - void testQueueLimitsMultipleTransactions() { + @ParameterizedTest + @BooleanSource + void testQueueLimitsMultipleTransactions(boolean writeNewFormat) { // Test that too many entries in 2 transactions fail to queue // Set "ongoing merge" indicator setOngoingMergeIndicator(index, null, null, simpleMetadataHook()); final int maxQueueSize = 5; - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); for (int i = 0; i < maxQueueSize; i++) { recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(100L + i, "test document", 1)); @@ -112,22 +119,23 @@ void testQueueLimitsMultipleTransactions() { commit(context); } - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); - assertThrows(PendingWriteQueue.PendingWritesQueueTooLargeException.class, + assertThrows(tooLargeException(writeNewFormat), () -> recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(999L, "test document", 1))); // The index is now in an inconsistent state, do don't commit } } - @Test - void testQueueLimitsAfterMerge() { + @ParameterizedTest + @BooleanSource + void testQueueLimitsAfterMerge(boolean writeNewFormat) { // Test that too many entries are cleared after merge // Set "ongoing merge" indicator setOngoingMergeIndicator(index, null, null, simpleMetadataHook()); final int maxQueueSize = 5; - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); for (int i = 0; i < maxQueueSize; i++) { recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(100L + i, "test document", 1)); @@ -140,7 +148,7 @@ void testQueueLimitsAfterMerge() { // Merge cleared the indicator, queue is now empty setOngoingMergeIndicator(index, null, null, simpleMetadataHook()); - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(999L, "test document", 1)); commit(context); @@ -153,12 +161,13 @@ void testQueueLimitsAfterMerge() { verifyExpectedDocIds(index, Set.of(100L, 101L, 102L, 103L, 104L, 999L)); } - @Test - void testQueueLimitsWithUpdates() { + @ParameterizedTest + @BooleanSource + void testQueueLimitsWithUpdates(boolean writeNewFormat) { // Test that each update counts for 2 entries in the queue // Write 5 records final int maxQueueSize = 5; - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); for (int i = 0; i < maxQueueSize; i++) { recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(100L + i, "test document", 1)); @@ -170,11 +179,11 @@ void testQueueLimitsWithUpdates() { setOngoingMergeIndicator(index, null, null, simpleMetadataHook()); // Update 3 documents, third update fails - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, simpleMetadataHook()); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(100L, "test document updated", 1)); recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(101L, "test document updated", 1)); - assertThrows(PendingWriteQueue.PendingWritesQueueTooLargeException.class, () -> + assertThrows(tooLargeException(writeNewFormat), () -> recordStore.saveRecord(LuceneIndexTestUtils.createSimpleDocument(102L, "test document updated", 1))); commit(context); } @@ -193,8 +202,9 @@ void testQueueLimitsWithUpdates() { verifyExpectedDocIds(index, Set.of(100L, 101L, 103L, 104L)); } - @Test - void testQueueLimitMultiplePartitions() { + @ParameterizedTest + @BooleanSource + void testQueueLimitMultiplePartitions(boolean writeNewFormat) { // Test pending queue size limit with partitioned index - documents in different partitions final int partitionSize = 3; final Index complexIndex = complexPartitionedIndex(partitionSize); @@ -203,7 +213,7 @@ void testQueueLimitMultiplePartitions() { final int maxQueueSize = 5; final int someDocsCount = 3; // Insert a few documents when "ongoing merge" indicator is clear. - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, hook); for (int i = 0; i < 6; i++) { recordStore.saveRecord(LuceneIndexTestUtils.createComplexDocument(2000L + i, "document foo", 1L, 30L + i)); @@ -223,7 +233,7 @@ void testQueueLimitMultiplePartitions() { setOngoingMergeIndicator(complexIndex, groupingKey, partition1, hook); // Insert documents when "ongoing merge" indicator is set. - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, hook); // Put 3 docs in each partition for (int i = 0; i < someDocsCount; i++) { @@ -239,13 +249,13 @@ void testQueueLimitMultiplePartitions() { commit(context); } - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, hook); // additional doc fails on new queue - assertThrows(PendingWriteQueue.PendingWritesQueueTooLargeException.class, () -> + assertThrows(tooLargeException(writeNewFormat), () -> recordStore.saveRecord(LuceneIndexTestUtils.createComplexDocument(999, "second document", 1L, 100L + 9))); } - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, hook); // additional docs succeed on old queue for (int i = someDocsCount; i < maxQueueSize; i++) { @@ -253,12 +263,12 @@ void testQueueLimitMultiplePartitions() { } commit(context); } - try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize))) { + try (FDBRecordContext context = openContext(getContextProperties(maxQueueSize, writeNewFormat))) { FDBRecordStore recordStore = LuceneIndexTestUtils.openRecordStore(context, path, hook); // additional docs fail on both queues - assertThrows(PendingWriteQueue.PendingWritesQueueTooLargeException.class, () -> + assertThrows(tooLargeException(writeNewFormat), () -> recordStore.saveRecord(LuceneIndexTestUtils.createComplexDocument(999, "second document", 1L, 100L + 9))); - assertThrows(PendingWriteQueue.PendingWritesQueueTooLargeException.class, () -> + assertThrows(tooLargeException(writeNewFormat), () -> recordStore.saveRecord(LuceneIndexTestUtils.createComplexDocument(999, "third document", 1L, 30L - 9))); } @@ -287,7 +297,8 @@ private void verifyClearedQueueAndIndicator(Index index, @Nullable Tuple groupin PendingWritesQueue queue = directory.createPendingWritesQueue(); List> entries = new ArrayList<>(); - queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null) + queue.getQueueCursor(context, ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())) .forEach(entries::add).join(); assertTrue(entries.isEmpty(), "Pending queue should have been empty"); commit(context); @@ -309,7 +320,8 @@ private void verifyExpectedQueueAndIndicator(Index index, @Nullable Tuple groupi PendingWritesQueue queue = directory.createPendingWritesQueue(); RecordCursor> queueCursor = queue.getQueueCursor( - recordStore.getContext(), ScanProperties.FORWARD_SCAN, null); + recordStore.getContext(), ScanProperties.FORWARD_SCAN, null, + PendingWritesQueueHelper.legacyDecoder(directory.getSerializer())); assertEquals(expectedOperations, queueCursor.asList().join().stream() @@ -391,7 +403,7 @@ private LuceneIndexMaintainer getIndexMaintainer(FDBRecordStore store, Index ind RecordMetaDataHook simpleMetadataHook() { return metaDataBuilder -> { metaDataBuilder.removeIndex(TextIndexTestUtils.SIMPLE_DEFAULT_NAME); - metaDataBuilder.addIndex(TestRecordsTextProto.SimpleDocument.getDescriptor().getName(), SIMPLE_TEXT_SUFFIXES); + metaDataBuilder.addIndex(TestRecordsTextProto.SimpleDocument.getDescriptor().getName(), index); }; } @@ -406,6 +418,7 @@ RecordMetaDataHook complexMetadataHook(Index index) { public Index complexPartitionedIndex(int highWatermark) { final Map options = Map.of( ENABLE_PENDING_WRITE_QUEUE_DURING_MERGE, "true", + LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true", INDEX_PARTITION_BY_FIELD_NAME, "timestamp", LuceneIndexOptions.PRIMARY_KEY_SEGMENT_INDEX_V2_ENABLED, "true", INDEX_PARTITION_HIGH_WATERMARK, String.valueOf(highWatermark)); @@ -422,10 +435,25 @@ public Index complexPartitionedIndex(final Map options) { options); } - private RecordLayerPropertyStorage getContextProperties(int maxQueueSize) { + private RecordLayerPropertyStorage getContextProperties(int maxQueueSize, boolean writeNewFormat) { return RecordLayerPropertyStorage.newBuilder() .addProp(LuceneRecordContextProperties.LUCENE_MAX_PENDING_QUEUE_SIZE, maxQueueSize) + .addProp(LuceneRecordContextProperties.LUCENE_PENDING_WRITE_QUEUE_WRITE_NEW_FORMAT, writeNewFormat) .build(); } + /** + * The "queue too large" exception thrown when the capacity is exceeded. The new-format writer + * uses the generic {@link PendingWritesQueue}'s exception; the legacy writer uses the Lucene + * {@link PendingWriteQueue}'s exception. + * + * @param writeNewFormat whether the new-format writer is in use + * @return the expected exception class for the enqueue capacity rejection + */ + private Class tooLargeException(boolean writeNewFormat) { + return writeNewFormat + ? PendingWritesQueue.PendingWritesQueueTooLargeException.class + : PendingWriteQueue.PendingWritesQueueTooLargeException.class; + } + } From a8011b24b25fc2f1aaf3c494dc0a613f4a5d148b Mon Sep 17 00:00:00 2001 From: ohad Date: Tue, 14 Jul 2026 16:34:27 -0400 Subject: [PATCH 5/5] Fix another test --- .../directory/PendingWriteQueueSerializationTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSerializationTest.java b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSerializationTest.java index 5796046eba..d0a17e2bc5 100644 --- a/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSerializationTest.java +++ b/fdb-record-layer-lucene/src/test/java/com/apple/foundationdb/record/lucene/directory/PendingWriteQueueSerializationTest.java @@ -25,6 +25,7 @@ import com.apple.foundationdb.record.ScanProperties; import com.apple.foundationdb.record.TestRecordsTextProto; import com.apple.foundationdb.record.lucene.LuceneIndexMaintainer; +import com.apple.foundationdb.record.lucene.LuceneIndexOptions; import com.apple.foundationdb.record.lucene.LuceneIndexTestUtils; import com.apple.foundationdb.record.lucene.LuceneRecordCursor; import com.apple.foundationdb.record.lucene.LuceneScanBounds; @@ -48,8 +49,9 @@ import java.util.Objects; import java.util.function.Function; -import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.SIMPLE_TEXT_SUFFIXES; import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.TEXT_AND_STORED_COMPLEX; +import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.simpleTextSuffixesIndex; +import static com.apple.foundationdb.record.lucene.LuceneIndexTestUtils.textAndStoredComplexIndex; import static com.apple.foundationdb.record.provider.foundationdb.indexes.TextIndexTestUtils.COMPLEX_DOC; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -65,7 +67,8 @@ class PendingWriteQueueSerializationTest extends FDBRecordStoreTestBase { @BooleanSource void testEndToEndSerialization(boolean overwrite) { // Compare two identical records - one of them was written through the pending write queue - final Index index = SIMPLE_TEXT_SUFFIXES; + final Index index = simpleTextSuffixesIndex(options -> + options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true")); final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); final Function schemaSetup = context -> LuceneIndexTestUtils.rebuildIndexMetaData(context, path, @@ -115,7 +118,8 @@ void testEndToEndSerialization(boolean overwrite) { @BooleanSource void testEndToEndSerializationComplex(boolean overwrite) { // compare two identical records - one of them was written through the pending write queue, but use complex index - final Index index = TEXT_AND_STORED_COMPLEX; + final Index index = textAndStoredComplexIndex(options -> + options.put(LuceneIndexOptions.PENDING_WRITE_QUEUE_INCARNATION_ENABLED, "true")); final KeySpacePath path = pathManager.createPath(TestKeySpace.RECORD_STORE); final Function schemaSetup = context -> LuceneIndexTestUtils.rebuildIndexMetaData(context, path, COMPLEX_DOC, index, useCascadesPlanner).getLeft();