JAVA-6055 Implement prose backpressure retryable writes tests by stIncMale · Pull Request #1929 · mongodb/mongo-java-driver

stIncMale · 2026-03-27T00:15:23Z

The relevant spec changes:

https://github.com/mongodb/specifications/blame/ba14b6bdc1dc695aa9cc20ccf9378592da1b2329/source/retryable-writes/tests/README.md#L265-L418
See also https://jira.mongodb.org/browse/DRIVERS-3432 for the required fixes for "Test 3 Case 3"

I recommend reviewing the first two commits one by one to simplify reviewing: the first commit does the refactoring, including the necessary refactoring (the introduction of ConfigureFailPointCommandListener), the second commit adds the new tests.

I used the TODO-BACKPRESSURE Valentin comments as described in #1918.

JAVA-6055

Copilot

Pull request overview

Adds/updates prose tests to cover the retryable writes “backpressure retryable writes” scenarios from the spec updates, including refactoring test infrastructure to configure fail points in reaction to command events.

Changes:

Introduce ConfigureFailPointCommandListener to enable a failpoint once upon matching a CommandEvent, and automatically disable it via try-with-resources.
Refactor existing retryable reads/writes prose tests to use the new helper and updated spec link references.
Add new “error propagation after encountering multiple errors” retryable writes prose test cases (with Case 1 temporarily disabled behind TODO-BACKPRESSURE).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
driver-sync/src/test/functional/com/mongodb/internal/event/ConfigureFailPointCommandListener.java	New test utility to configure/cleanup failpoints triggered by command events.
driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java	Refactor existing prose tests; add new multi-error propagation cases.
driver-sync/src/test/functional/com/mongodb/client/RetryableReadsProseTest.java	Update prose-test references and align helper usage/signatures.
driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableWritesProseTest.java	Mirror sync prose test updates via sync-adapter delegation; add new cases.
driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableReadsProseTest.java	Update prose-test references and align helper usage/signatures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

...-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableReadsProseTest.java

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...r-sync/src/test/functional/com/mongodb/internal/event/ConfigureFailPointCommandListener.java

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

JAVA-6055

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableReadsProseTest.java

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

The relevant spec changes: - https://github.com/mongodb/specifications/blame/ba14b6bdc1dc695aa9cc20ccf9378592da1b2329/source/retryable-writes/tests/README.md#L265-L418 - See also https://jira.mongodb.org/browse/DRIVERS-3432 for the required fixes for "Test 3 Case 3" JAVA-6055

nhachicha

Good idea extracting ConfigureFailPointCommandListener as a reusable utility 👍
some minor comments from me & Claude review

nhachicha · 2026-03-28T16:33:01Z

...r-sync/src/test/functional/com/mongodb/internal/event/ConfigureFailPointCommandListener.java

+                fail("The listener was closed before (in the happens-before order) it attempted to configure the fail point");
+            } else {
+                assertTrue(failPointFuture.isDone());
+                assertNotNull(failPointFuture.get()).close();


[minor]
If failPointFuture is completed exceptionally (via completeExceptionally), then failPointFuture.get() in close() will throw, so the normal FailPoint.close() is skipped and the fail point may remain enabled.

This is, however, low-risk since there may be nothing to clean up if FailPoint.enable() fails ...

If failPointFuture is completed exceptionally (via completeExceptionally), then failPointFuture.get() in close() will throw, so the normal FailPoint.close() is skipped

No, the execution you are proposing is forbidden, and the conclusion "so the normal FailPoint.close() is skipped" is incorrect, assuming that you imply that a reference to a FailPoint was returned (I have to assume this, as otherwise there is no reference which we could call close through):

Due to the synchronized blocks, there is a guarantee that either the whole onEvent is ordered before (in the happens-before order) close, or close is ordered before onEvent¹.

failPointFuture.get() is not part of any execution, where close is ordered before onEvent.

In all executions where onEvent is ordered before close, failPointFuture.get() throws if and only if failPointFuture.completeExceptionally(e) was executed as part of onEvent. When the latter is the case, no fail point were enabled², and, therefore, no fail point needs to be closed.

¹ Strictly speaking, that's not the right way to say what I am saying, but I don't want to make things more complex, because that will change nothing.

² Actually, FailPoint.enable throwing does not guarantee that the configureFailPoint command failed to configure the fail point. That's a bug in FailPoint.enable, I fixed it in main: #1931.

If, however, you were talking exactly about the bug in FailPoin.enable (I introduced it in the past when added this class):

there may be nothing to clean up if FailPoint.enable() fails

then no code added/modified in the current PR is relevant, and any usage of FailPoint is problematic because of that bug.

If that's the case, then it should be fine when #1931 is merged.

nhachicha · 2026-03-28T16:37:05Z

...r-sync/src/test/functional/com/mongodb/internal/event/ConfigureFailPointCommandListener.java

+    @Override
+    public void close() throws InterruptedException, ExecutionException {
+        synchronized (lock) {
+            if (failPointFuture.cancel(true)) {


[TIL]
mayInterruptIfRunning is no-op

mayInterruptIfRunning - this value has no effect in this implementation because interrupts are not used to control processing.

Thank you, I know. But we have to pass something.

nhachicha · 2026-03-28T16:55:19Z

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

                + "    data: {\n"
-                + "        failCommands: [\"" + operationName + "\"],\n"
+                + "        failCommands: [\"" + expectedCommandName + "\"],\n"
+                + "        errorCode: 6,\n"


[claude-review]
Trailing comma in generated JSON when write=false.

The old code avoided this by placing errorCode last

The same pattern appears in retriesOnSameMongosWhenAnotherNotAvailable

nhachicha · 2026-03-28T17:25:14Z

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

+        Predicate<CommandEvent> configureFailPointEventMatcher = event -> {
+            if (event instanceof CommandFailedEvent) {
+                CommandFailedEvent commandFailedEvent = (CommandFailedEvent) event;
+                MongoException cause = assertInstanceOf(MongoException.class, commandFailedEvent.getThrowable());


[claude-review]
The predicates use assertInstanceOf(MongoException.class, ...) which throws AssertionFailedError if the type doesn't match. A Predicate.test() should return false, not throw. The exception is caught by onEvent() and routed to completeExceptionally, so it works, but the semantics are surprising. Consider using a plain instanceof check instead.

nhachicha · 2026-03-28T17:25:36Z

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

+        Predicate<CommandEvent> configureFailPointEventMatcher = event -> {
+            if (event instanceof CommandFailedEvent) {
+                CommandFailedEvent commandFailedEvent = (CommandFailedEvent) event;
+                MongoException cause = assertInstanceOf(MongoException.class, commandFailedEvent.getThrowable());


Ditto https://github.com/mongodb/mongo-java-driver/pull/1929/changes#diff-8fe5319aef55e451d009c31e961108720a7d361d60dd5c5d2c0a99e088d0545dR378

nhachicha · 2026-03-28T17:49:56Z

...r-sync/src/test/functional/com/mongodb/internal/event/ConfigureFailPointCommandListener.java

+import static com.mongodb.assertions.Assertions.assertTrue;
+import static com.mongodb.assertions.Assertions.fail;
+
+public final class ConfigureFailPointCommandListener implements CommandListener, AutoCloseable {


Worth annotating with @ThreadSafe ?

nhachicha · 2026-03-28T18:26:16Z

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

+                + "        failCommands: ['" + commandName + "'],\n"
+                + "        errorCode: 91,\n"
+                + "        blockConnection: true,\n"
+                + "        blockTimeMS: 1000,\n"


Trailing comma issue when write == false. I wonder if we should keep the append approach of building the BsonDocument

similar to https://github.com/mongodb/mongo-java-driver/pull/1929/changes#r3005045523

nhachicha · 2026-03-28T20:43:11Z

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

                + "    data: {\n"
-                + "        failCommands: [\"" + operationName + "\"],\n"
+                + "        failCommands: [\"" + expectedCommandName + "\"],\n"
+                + "        errorCode: 6,\n"


Ditto https://github.com/mongodb/mongo-java-driver/pull/1929/changes#r3005045523

nhachicha · 2026-03-30T06:48:28Z

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java

+                + "', '" + NO_WRITES_PERFORMED_ERROR_LABEL + "']\n"
+                + "    }\n"
+                + "}\n");
+        Predicate<CommandEvent> configureFailPointEventMatcher = event -> event instanceof CommandFailedEvent;


[minor]
Both the initial and the listener fail points use error code 91, for consistency and defensiveness, checking code == 91 wouldn't hurt — if something unexpected fails first (e.g., a different command), the test would configure the second fail point at the wrong time and produce a confusing failure.

stIncMale requested a review from nhachicha March 27, 2026 00:15

stIncMale self-assigned this Mar 27, 2026

vbabanin requested a review from Copilot March 27, 2026 00:25

Copilot started reviewing on behalf of vbabanin March 27, 2026 00:25 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java Show resolved Hide resolved

...-streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableReadsProseTest.java Outdated Show resolved Hide resolved

stIncMale force-pushed the backpressure_JAVA-6055_proseTests branch 3 times, most recently from 53e526f to 44c8119 Compare March 27, 2026 18:31

vbabanin requested a review from Copilot March 27, 2026 18:34

Copilot started reviewing on behalf of vbabanin March 27, 2026 18:34 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

stIncMale force-pushed the backpressure_JAVA-6055_proseTests branch from 44c8119 to 973ee55 Compare March 27, 2026 18:45

vbabanin requested a review from Copilot March 27, 2026 18:58

Copilot started reviewing on behalf of vbabanin March 27, 2026 18:58 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

...r-sync/src/test/functional/com/mongodb/internal/event/ConfigureFailPointCommandListener.java Outdated Show resolved Hide resolved

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java Outdated Show resolved Hide resolved

stIncMale force-pushed the backpressure_JAVA-6055_proseTests branch from 973ee55 to 758b89e Compare March 27, 2026 21:02

vbabanin requested a review from Copilot March 27, 2026 21:05

Copilot started reviewing on behalf of vbabanin March 27, 2026 21:06 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java Outdated Show resolved Hide resolved

driver-sync/src/test/functional/com/mongodb/client/RetryableWritesProseTest.java Outdated Show resolved Hide resolved

Refactor prose retryable writes/reads tests

e79eee1

JAVA-6055

stIncMale force-pushed the backpressure_JAVA-6055_proseTests branch from 758b89e to d270460 Compare March 27, 2026 21:36

vbabanin requested a review from Copilot March 27, 2026 21:53

Copilot started reviewing on behalf of vbabanin March 27, 2026 21:53 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

stIncMale force-pushed the backpressure_JAVA-6055_proseTests branch from d270460 to 72d70e4 Compare March 27, 2026 22:14

stIncMale marked this pull request as ready for review March 27, 2026 22:15

stIncMale requested a review from a team as a code owner March 27, 2026 22:15

stIncMale changed the title ~~Implement prose backpressure retryable writes tests~~ JAVA-6055 Implement prose backpressure retryable writes tests Mar 28, 2026

stIncMale changed the title ~~JAVA-6055 Implement prose backpressure retryable writes tests~~ JAVA-6055: Implement prose backpressure retryable writes tests Mar 28, 2026

stIncMale changed the title ~~JAVA-6055: Implement prose backpressure retryable writes tests~~ JAVA-6055 Implement prose backpressure retryable writes tests Mar 28, 2026

nhachicha reviewed Mar 30, 2026

View reviewed changes

Conversation

stIncMale commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nhachicha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stIncMale Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

stIncMale commented Mar 27, 2026 •

edited

Loading

stIncMale Mar 31, 2026 •

edited

Loading