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 phrasing 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

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

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

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 ?

Done in f9fd19e.

None of our other named (non-anonymous) implementations of CommandListener are annotated with @ThreadSafe, even though some of them are thread-safe, and some don't have to be thread-safe due to the way they are used in tests. I marked all of them with @ThreadSafe/@NotThreadSafe.

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

…ailPointCommandListener`

…`/`@NotThreadSafe`.

…unteringMultipleErrorsCase3` This made its `configureFailPointEventMatcher` the same as the one in cases 1 and 2, which lead to a nice simplification.

stIncMale

I am ready for another review round. @nhachicha, thank you for marking AI-generated concerns with [claude-review].

The new wording provides guarantees that allow assertions in event matchers.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

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

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

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 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

...streams/src/test/functional/com/mongodb/reactivestreams/client/RetryableWritesProseTest.java

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

driver-core/src/test/functional/com/mongodb/internal/connection/TestCommandListener.java:71

@ThreadSafe on this TestCommandListener appears incorrect: ignoreNextSucceededOrFailedEvent is read/written outside of lock (e.g., set in commandStarted and read/reset in commandSucceeded/commandFailed before locking), which is a data race under concurrent command event delivery. Either remove the @ThreadSafe annotation, or make the state used for filtering (ignoreNextSucceededOrFailedEvent) properly concurrency-safe (and consider whether a single global flag is valid when multiple operations/threads can interleave).

@ThreadSafe
public class TestCommandListener implements CommandListener {
    private final List<String> eventTypes;
    private final List<String> ignoredCommandMonitoringEvents;
    private final List<CommandEvent> events = new ArrayList<>();
    @Nullable
    private final TestListener listener;
    private final Lock lock = new ReentrantLock();
    private final Condition commandCompletedCondition = lock.newCondition();
    private final Condition commandAnyEventCondition = lock.newCondition();
    private final boolean observeSensitiveCommands;
    private boolean ignoreNextSucceededOrFailedEvent;

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

stIncMale · 2026-04-09T20:15:46Z

I agree with AI in #1929 (review) - the code looks suspicious. But that does not change the fact that the TestCommandListener contract is such that it must be thread-safe. I don't have the bandwidth to investigate that code (it wasn't modified in the PR), so I'll leave it be as is.

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

stIncMale added 3 commits March 31, 2026 03:08

Replace comparisons with assertions in event matchers for `ConfigureF…

fcd0a1c

…ailPointCommandListener`

Annotate named implementations of CommandListener with `@ThreadSafe…

f9fd19e

…`/`@NotThreadSafe`.

Add assertions not required by the spec to `errorPropagationAfterEnco…

7891545

…unteringMultipleErrorsCase3` This made its `configureFailPointEventMatcher` the same as the one in cases 1 and 2, which lead to a nice simplification.

stIncMale commented Mar 31, 2026

View reviewed changes

stIncMale requested a review from nhachicha March 31, 2026 10:05

Improve the documentation of ConfigureFailPointCommandListener

391c317

The new wording provides guarantees that allow assertions in event matchers.

nhachicha previously approved these changes Mar 31, 2026

View reviewed changes

Make the new tests work when run against MongoDB 6

4b9a565

stIncMale dismissed nhachicha’s stale review via 4b9a565 April 1, 2026 19:32

stIncMale requested a review from Copilot April 1, 2026 19:32

Copilot started reviewing on behalf of stIncMale April 1, 2026 19:32 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

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

stIncMale requested a review from nhachicha April 1, 2026 20:19

nhachicha reviewed Apr 9, 2026

View reviewed changes

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

nhachicha requested a review from Copilot April 9, 2026 11:10

Copilot started reviewing on behalf of nhachicha April 9, 2026 11:11 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

stIncMale added 2 commits April 9, 2026 12:59

Enable errorPropagationAfterEncounteringMultipleErrorsCase3

68fdf37

Enable reactive errorPropagationAfterEncounteringMultipleErrorsCase3

3156205

stIncMale requested review from Copilot and nhachicha April 9, 2026 19:07

Copilot started reviewing on behalf of stIncMale April 9, 2026 19:08 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

nhachicha approved these changes Apr 10, 2026

View reviewed changes

stIncMale merged commit fa82369 into mongodb:backpressure Apr 10, 2026
57 checks passed

stIncMale deleted the backpressure_JAVA-6055_proseTests branch April 10, 2026 18:29

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nhachicha Mar 28, 2026

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!

Uh oh!

Uh oh!

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

stIncMale commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

stIncMale commented Mar 27, 2026 •

edited

Loading

stIncMale Mar 31, 2026 •

edited

Loading