Pending writes queue by ohadzeliger · Pull Request #4309 · FoundationDB/fdb-record-layer

ohadzeliger · 2026-06-29T20:28:05Z

Create a write-pending-queue for use with the indexer when in write-only mode.
The created queue provides the following guarantees:

Conflict free enqueue and clear
Strict insertion order traversal (for the same incarnation)
Incarnation support for use after moving to a different FDB cluster
Conflict-free size getter
Transactionally consistant ensureEmpty operation
Generic payload extensible by caller (caller controls payload creation)
Configurable maximum size

Testing involves 3 test classes:

Correctness tests - ensure guarantees on conflicts and ordering are preserved
Size tests - ensure size getter matches queue size
Concurrency tests - random threads performing operations while preserving consistency

Resolves #4311

jjezra

Since this queue is unused, I'll be happy to have it in the code as a foundation to the write_only_with_queue development while adding checks and improvements.

jjezra · 2026-06-29T21:10:06Z

+ * <p>The queue is intended to hold pending operations that arrive while a background indexer
+ * (or any other background worker) is running. Front-end transactions enqueue items


How about -
The queue is intended to hold pending foreground operations that arrive while a background worker is running and possibly holding resources.

jjezra · 2026-06-29T21:29:40Z

+    /**
+     * Default maximum queue size; protects against unbounded growth on persistent failure.
+     */
+    public static final int DEFAULT_MAX_QUEUE_SIZE = 10_000;


Can this default be increased to 100K?
Since the consequences are severe (either re-indexing the whole record store or rejecting user IO) the buffer should be big enough to absorb occasional indexing/merging failures that may keep an active queue for a while.

This is only used by one test, perhaps it should be moved there, rather than here.

Done. Changed to 100K. Assuming that all callers will actually tune on a per-case basis.

jjezra · 2026-06-30T16:19:47Z

+     * read and the commit
+     */
+    @Nonnull
+    public CompletableFuture<Boolean> ensureQueueEmpty(@Nonnull FDBRecordContext context) {


isQueueEmpty might be a better name.

jjezra · 2026-06-30T16:53:27Z

+
+        try (FDBRecordContext txA = openContext()) {
+            // TX_A asserts the queue is empty
+            assertTrue(queue.ensureQueueEmpty(txA).join());


Can we add a test where ensureQueueEmpty tx succeeds and the add item fails?

That will not fail as such - the caller should provide another flag to guarantee it (since the enqueue does not read the range)

Added a comment for the class

jjezra · 2026-06-30T16:57:04Z

+     * versionstamped-key + atomic-counter idioms, not from anything queue-specific.</p>
+     */
+    @Test
+    void testDrainScanAndClearDoesNotConflictWithConcurrentEnqueue() {


I suppose that it's a "nice to have" :)

ScottDugas · 2026-06-30T17:16:01Z

+     * a way readers must distinguish. Readers reject entries whose stored version exceeds this
+     * constant.
+     */
+    public static final int CURRENT_VERSION = 0;


Note: IIUC this means that the caller can't use the version field to adjust what they write, that must be within the payload.

Yes. Payload versioning does not come for free with this field, though a specific payload provider can affect the overall version for certain use cases if needed.

ScottDugas · 2026-06-30T17:17:30Z

+    /**
+     * Default maximum queue size; protects against unbounded growth on persistent failure.
+     */
+    public static final int DEFAULT_MAX_QUEUE_SIZE = 10_000;


This is only used by one test, perhaps it should be moved there, rather than here.

ScottDugas · 2026-06-30T17:18:58Z

+ *   <li><b>Capacity.</b> A configurable maximum queue size; {@link #enqueue} fails with
+ *       {@link PendingWritesQueueTooLargeException} once the size counter reaches the limit.
+ *       A value of {@code 0} disables the cap.</li>


Perhaps worth noting that this is approximate (since it is written at snapshot, you can end up with a bigger queue by the amount written by concurrent transactions)

ScottDugas · 2026-06-30T17:22:14Z

+     * @param entry the entry to clear
+     */
+    public void clearEntry(@Nonnull FDBRecordContext context, @Nonnull PendingWritesQueueEntry<T> entry) {
+        SplitHelper.deleteSplit(context, queueSubspace, entry.getKeyTuple(), true, false, false, null);


Does deleteSplit result in a read-conflict on the entry? Will two transactions conflict if they both try to clear the same entry? (other modifications aren't allowed, anyways)

Normally, two clear operations wouldn't conflict. Discussed offline and agreed that the correct system behavior is that they should. Added read conflict for the clearEntry and modified the test.

ScottDugas · 2026-06-30T17:23:40Z

+     * @throws com.apple.foundationdb.record.provider.foundationdb.FDBExceptions.FDBStoreTransactionConflictException
+     * via the transaction's commit if another transaction enqueued into the queue between this
+     * read and the commit


this is misleading. This method does not throw this exception, nor does the future. IMO the explanation in the paragraph should be sufficient

Yeah, done.

ScottDugas · 2026-06-30T18:33:44Z

+                RecordCursor<PendingWritesQueueEntry<TestQueuePayload>> cursor =
+                        queue.getQueueCursor(context, scanProps, continuation);
+                RecordCursorResult<PendingWritesQueueEntry<TestQueuePayload>> last = null;
+                while (true) {


You could use cursor.forEachResult to simplify this code a bit.

ScottDugas · 2026-06-30T18:34:26Z

+                        last = next;
+                        break;
+                    }
+                    collected.add(next.get());


It would probably be good to assert that this is 7, or at least that we actually apply a limit, and don't just read it all at once, but maybe the limiting is already tested below.

Added assertion on the number of continuations hit

ScottDugas · 2026-06-30T18:36:53Z

+        try (FDBRecordContext ctxA = openContext();
+                FDBRecordContext ctxB = openContext()) {
+            for (int i = 0; i < 4; i++) {
+                queue.enqueue(ctxA, payload("A-" + i), 0).join();


Should you force a getReadVersion call?
I think it's possible that if these are all blind-writes, then GRV doesn't happen until commit, in which case they are not actually overlapping.

enqueue reads the queue size (via SNAPSHOT) so I think we should be ok

ScottDugas · 2026-06-30T18:42:48Z

+            }
+
+            // While the drain transaction is still open, another transaction enqueues a fresh
+            // entry. It must commit cleanly because enqueue never conflicts with anything.


Suggested change

// entry. It must commit cleanly because enqueue never conflicts with anything.

// entry. It must commit cleanly because enqueue only conflicts with ensureQueueEmpty.

ScottDugas · 2026-06-30T18:44:11Z

+        assertScanThrows(readerQueue, RecordCoreStorageException.class);
+    }
+
+    private <T extends com.google.protobuf.Message> void assertScanThrows(


Can Message be imported rather than fully-qualified.

Actually replaced by ?

ohadzeliger added 3 commits June 25, 2026 18:13

Initial commit for the pending writes queue

bc5a7f9

Make queue generic by using Any, remove specific indexing protobuf

b0a9473

Clean up tests

8d83feb

jjezra added relational issues related to relational FDB indexer Online Indexer issues labels Jun 29, 2026

ohadzeliger self-assigned this Jun 29, 2026

ohadzeliger added the enhancement New feature or request label Jun 29, 2026

Style

d64c4d9

jjezra marked this pull request as ready for review June 30, 2026 15:31

jjezra approved these changes Jun 30, 2026

View reviewed changes

jjezra reviewed Jun 30, 2026

View reviewed changes

PR comments

daa4693

ScottDugas reviewed Jun 30, 2026

View reviewed changes

ohadzeliger added 2 commits June 30, 2026 18:25

PR comments

8fa74dd

PR comments

c93fded

		* <p>The queue is intended to hold pending operations that arrive while a background indexer
		* (or any other background worker) is running. Front-end transactions enqueue items

	// entry. It must commit cleanly because enqueue never conflicts with anything.
	// entry. It must commit cleanly because enqueue only conflicts with ensureQueueEmpty.

Uh oh!

Conversation

ohadzeliger commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jjezra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ohadzeliger commented Jun 29, 2026 •

edited

Loading