Adds overload retry for not read/write retryable commands by vbabanin · Pull Request #2048 · mongodb/mongo-java-driver

vbabanin · 2026-09-06T21:45:36Z

Summary

Adds overload-only retry support to 11 write-command operations that previously dispatched commands without any retry wrapper.

The retry policy is gated on retryWrites and uses:

includeOverload(maxAdaptiveRetriesSetting, ErrorPropagation.AS_WRITE_POLICY)

It intentionally does not use includeWrite(), because these commands are not retryable writes under the retryable-writes spec.

Commands covered

Command	Operation
`createIndexes`	`CreateIndexesOperation`
`dropIndexes`	`DropIndexOperation`
`create`	`CreateCollectionOperation`
`create` view	`CreateViewOperation`
`drop`	`DropCollectionOperation`
`dropDatabase`	`DropDatabaseOperation`
`renameCollection`	`RenameCollectionOperation`
`aggregate` with `$out` / `$merge`	`AggregateToCollectionOperation`
`createSearchIndexes`	`CreateSearchIndexesOperation`
`updateSearchIndex`	`UpdateSearchIndexesOperation`
`dropSearchIndex`	`DropSearchIndexOperation`

Implementation details

Wraps sync dispatch with createSpecRetryControl + decorateWithRetries.
Wraps async dispatch with createSpecRetryControl + decorateWithRetriesAsync.
Each retry attempt performs a fresh connection selection.
Adds onCommand(this::getCommandName) for retry debug logging.
Threads retryWrites and maxAdaptiveRetriesSetting from the client layer through Operations into the affected operations.
Fixes sync aggregate so $out / $merge respects the actual retryWrites setting.

Scope notes

Already covered before this change:

updateMany / deleteMany / bulkWrite
findAndModify
runCommand
getMore
regular read operations through executeRetryableRead

Intentionally excluded:

Excluded	Reason
`mapReduce`	Deprecated / removed; no peer-driver precedent.
`{w:0}` writes	No server response exists to carry an overload error.
`killCursors`	Handled separately.
Monitoring / RTT / auth / connection setup	Explicitly excluded by the backpressure spec.

Test coverage

Removed JAVA-5956 skips for unified client-backpressure tests:
- createIndex
- dropIndex
- dropIndexes
- aggregate write retry
Added prose tests for:
- create collection/view
- drop collection/database
- rename collection
- create/update/drop search index

JAVA-6308
JAVA-6119 - Fix client construction in backpressure unified tests

Reason to close: the TODO-JAVA-5956 skips that masked the client-construction gap are removed.

The unified backpressure suite now constructs per-test clients such as client_retryWrites_false and client_retryReads_false according to the spec and executes those scenarios instead of skipping them. No remaining driver-side client-construction issue is known.

JAVA-6124 - Test overload retry when retryReads / retryWrites is false

Reason to close: the 34 unified “does not retry if retry*=false” cases run and pass.

They verify gate behavior across command families: retryable reads are gated on retryReads, retryable writes on retryWrites, and runCommand on retryReads && retryWrites.

This was a verification ticket, not an implementation ticket, and the suite is now green.

JAVA-5956 - Exponential backoff and jitter in retry loops

Reason to close: the backoff/jitter implementation already exists in prior work.

All TODO-JAVA-5956 unified-test skips are now removed: createIndex / dropIndex / dropIndexes / aggregate-write skips via JAVA-6308, and the getMore backpressure skip via JAVA-6296.

The only remaining getMore-related skip is a reactive-only skipNoncompliantReactive for the cursor auto-close flake, which is not a TODO-JAVA-5956 marker. No TODO-JAVA-5956 markers remain.

…d/write retry policies - Implements overload-only retry (gated on retryWrites, per client-backpressure) for the write commands that previously dispatched with no retry wrapper: createIndexes, dropIndexes, create/drop/rename collection, aggregate with $out/$merge, and the search-index commands. - Removes the JAVA-5956 skips that hid these commands from the unified client-backpressure suite, and adds prose tests (not part of the spec suite) covering the commands that have no unified test coverage. JAVA-6308

Copilot

🟡 Changes recommended

Constructor compatibility, legacy integration, and multi-command retry-scope defects must be resolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds retryWrites-gated overload retries to previously non-retryable write commands across core and synchronous driver paths.

Changes:

Adds synchronous and asynchronous overload retry loops for DDL and search-index operations.
Propagates retry settings to write aggregations.
Enables unified backpressure tests and adds prose coverage.

Human review is required. Blocking issues include stale constructor call sites, incomplete legacy-driver wiring, and incorrect retry scopes for encrypted collection operations.

File summaries

File	Description
`UnifiedTestModifications.java`	Re-enables backpressure tests.
`BackpressureProseTest.java`	Adds command retry coverage.
`MongoDatabaseImpl.java`	Propagates aggregate retryWrites.
`MongoCollectionImpl.java`	Propagates aggregate retryWrites.
`AggregateIterableImpl.java`	Accepts write retry settings.
`UpdateSearchIndexesOperation.java`	Adds retry configuration.
`RenameCollectionOperation.java`	Adds overload retry loop.
`Operations.java`	Wires retry settings into operations.
`DropSearchIndexOperation.java`	Adds retry configuration.
`DropIndexOperation.java`	Adds overload retry loop.
`DropDatabaseOperation.java`	Adds overload retry loop.
`DropCollectionOperation.java`	Adds overload retry loop.
`CreateViewOperation.java`	Adds overload retry loop.
`CreateSearchIndexesOperation.java`	Adds retry configuration.
`CreateIndexesOperation.java`	Adds overload retry loop.
`CreateCollectionOperation.java`	Adds overload retry loop.
`AggregateToCollectionOperation.java`	Enables write-gated overload retries.
`AbstractWriteSearchIndexOperation.java`	Centralizes search-index retries.

Review details

Files reviewed: 18/18 changed files
Comments generated: 4
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…olicies - Wraps DDL and search-index write commands in an overload-only retry policy gated on retryWrites, and extends the prose suite to cover them. JAVA-6065

Copilot

🔵 Needs a closer look

Core retry and connection-selection behavior has unresolved command-scoping and retargeting issues and requires human review.

Review details

Suppressed comments (2)

driver-core/src/main/com/mongodb/internal/operation/CreateCollectionOperation.java:289

The async path likewise retains one AsyncConnectionSource for the entire sequence, so retries cannot perform server selection and cannot honor overload retargeting. Acquire the write source as part of each decorateWithRetriesAsync attempt rather than around ProcessCommandsCallback.

        withAsyncWriteConnectionSource(binding, operationContext, callback,
                (source, operationContextWithMinRtt, sourceReleasingCallback) ->
                        new ProcessCommandsCallback(binding, source, operationContextWithMinRtt, sourceReleasingCallback)
                                .onResult(null, null));

driver-core/src/main/com/mongodb/internal/operation/DropCollectionOperation.java:153

The async retry supplier encloses ProcessCommandsCallback, which processes every encrypted-drop command. An overload on a later command therefore reruns prior successful drops and consumes a sequence-wide retry budget instead of retrying that command independently. Move retry-control creation into ProcessCommandsCallback for each dequeued command.

            RetryControl<SpecRetryPolicy> retryControl = createSpecRetryControl(
                    createSpecRetryPolicy(),
                    operationContext);
            AsyncCallbackSupplier<Void> retryingCommandExecutor = decorateWithRetriesAsync(retryControl, operationContext,

Files reviewed: 23/23 changed files
Comments generated: 2
Review effort level: Balanced

vbabanin · 2026-09-08T06:23:53Z

-               executeCommand(binding, operationContextWithMinRtt,  databaseName, commandCreator.get(), connection,
-                      writeConcernErrorTransformer(operationContextWithMinRtt.getTimeoutContext()))
-            );
+        return withWriteConnectionSource(binding, operationContext, (source, operationContextWithMinRtt) -> {


Closed by re-selecting the write source per retry attempt for every create/drop sub-command, matching the other unsupported write commands.

A conditional shape was considered: pin one source for the Queryable Encryption multi-command sequence, while re-selecting for the single non-encrypted command. Re-selecting for all commands was chosen instead.

The tradeoff is:

Under 462 overload, most cases should not result in stepdown, so pinning and re-selection are equivalent: writes target the primary either way.

If stepdown does happen, re-selection can recover the in-flight command on the new primary. Pinning would fail against the old primary with NotWritablePrimary.

The main case where pinning helps is a narrow partial-state edge case when a previously succeeded QE sub-command did not replicate before stepdown. For example: create esc succeeds on primary A, A steps down before esc replicates to B, and the next sub-command fails with 462. With re-selection, the retry can continue on B, so the sequence may complete with esc missing on B. With pinning, the retry stays on A, gets NotWritablePrimary, and aborts the sequence without a slient failure. Then the application-level can decide what to do with the failure.

Net: re-selecting gives better availability and keeps this consistent with the other unsupported write commands. The conditional pin/re-select shape did not seem worth the extra complexity

- Hoists the encryptedFields lookup out of the async drop retry loop so it is not retried under the write gate, matching the sync path. - Gives each drop sub-command its own retry budget, mirroring the create path. - Re-selects a write source per attempt for the single non-encrypted command (retargets on stepdown); pins one source for the encrypted multi-command sequence so it fails loudly rather than silently losing a state collection. JAVA-6065

Each create/drop sub-command now re-selects a write source per retry attempt via withConnection(binding, ...), matching the other unsupported write commands, instead of pinning one source across the Queryable Encryption sequence. Drops the source-accepting withConnection helpers that are no longer used. JAVA-6065

- Adds overloadForWrite/overloadForRead on SpecRetryPolicy.IndividualPolicies so the unsupported-write commands and getMore build their overload-only policy through a single shared entry point instead of repeating the IndividualPolicies+includeOverload+ErrorPropagation construction at each call site JAVA-6308

JAVA-6308

Copilot

🟡 Changes recommended

The legacy aggregate-to-collection path still disables overload retries.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 29/29 changed files
Comments generated: 1
Review effort level: Balanced

Copilot

🔵 Needs a closer look

Core retry, connection-selection, and multi-command execution semantics require final human review.

Review details

Files reviewed: 30/30 changed files
Comments generated: 0 new
Review effort level: Balanced

jyemin

I didn't read every line of every operation, but overall the design holds up and the changes appear (mostly) mechanical.

I posted one question, but LGTM regardless.

jyemin · 2026-09-09T22:46:32Z

        // backpressure

-        def.modify(WAIT_FOR_BATCH_CURSOR_CREATION)
+        def.modify(WAIT_FOR_BATCH_CURSOR_CREATION, IGNORE_EXTRA_EVENTS)


The backpressure changeStream tests were intermittently failing in reactive: after the aggregate succeeds, the reactive change-stream cursor proactively issues a getMore (awaitData), and those trailing events land non-deterministically relative to the runner's exact-count event snapshot. WAIT_FOR_BATCH_CURSOR_CREATION only synchronizes cursor creation (the aggregate); it doesn't account for the subsequent getMore/killCursors.

vbabanin requested a balanced review from Copilot September 6, 2026 21:45

Copilot started reviewing on behalf of vbabanin September 6, 2026 21:46 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

vbabanin added 2 commits September 7, 2026 18:38

Adds overload retry for commands not retryable under the read/write p…

bacc263

…olicies - Wraps DDL and search-index write commands in an overload-only retry policy gated on retryWrites, and extends the prose suite to cover them. JAVA-6065

Fix tests.

7143007

vbabanin requested a balanced review from Copilot September 8, 2026 02:07

vbabanin self-assigned this Sep 8, 2026

Copilot started reviewing on behalf of vbabanin September 8, 2026 02:07 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

vbabanin added 6 commits September 7, 2026 20:28

Fix drop collection retries and add tests.

af0c556

Add support for legacy API.

8ed9b2b

Remove helpers and javadoc.

751b92d

Use helpers.

6c537da

vbabanin changed the title ~~Adds overload retry for not retryable commands~~ Adds overload retry for not read/write retryable commands Sep 8, 2026

vbabanin added 5 commits September 8, 2026 13:01

Fix tests.

ce9afbc

Fix tests.

5f995b1

Ignore extra-events

a387f6d

Tightens backpressure prose test assertions

0455004

JAVA-6308

vbabanin requested a balanced review from Copilot September 8, 2026 23:57

Copilot started reviewing on behalf of vbabanin September 8, 2026 23:57 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread driver-core/src/main/com/mongodb/internal/operation/AggregateToCollectionOperation.java

vbabanin marked this pull request as ready for review September 9, 2026 00:23

vbabanin requested a review from a team as a code owner September 9, 2026 00:23

vbabanin requested review from jyemin and strogiyotec and removed request for strogiyotec September 9, 2026 00:23

vbabanin added 3 commits September 8, 2026 17:36

Fix aggregate operation.

a2c8fcd

Enable Search Index Model tests.

e55c4dd

Fix static checks.

6cbc50a

vbabanin requested a balanced review from Copilot September 9, 2026 01:28

Copilot started reviewing on behalf of vbabanin September 9, 2026 01:28 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Remove duplicates.

bd6537c

jyemin approved these changes Sep 10, 2026

View reviewed changes

vbabanin merged commit d9e3b16 into mongodb:backpressure Sep 10, 2026
36 of 53 checks passed

Navigation Menu

Uh oh!

Adds overload retry for not read/write retryable commands - #2048

Adds overload retry for not read/write retryable commands#2048
vbabanin merged 18 commits into
mongodb:backpressurefrom
vbabanin:support-non-retry-command

vbabanin commented Sep 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

vbabanin Sep 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

jyemin left a comment

Uh oh!

jyemin Sep 9, 2026

Uh oh!

vbabanin Sep 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

vbabanin commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commands covered

Implementation details

Scope notes

Test coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

vbabanin Sep 8, 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!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

jyemin left a comment

Choose a reason for hiding this comment

Uh oh!

jyemin Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

vbabanin Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vbabanin commented Sep 6, 2026 •

edited

Loading

vbabanin Sep 8, 2026 •

edited

Loading