Refactor MixedBulkWriteOperation to simplify retry logic there#1989
Open
stIncMale wants to merge 7 commits into
Open
Refactor MixedBulkWriteOperation to simplify retry logic there#1989stIncMale wants to merge 7 commits into
MixedBulkWriteOperation to simplify retry logic there#1989stIncMale wants to merge 7 commits into
Conversation
JAVA-6218
…FailPointCommandListener` JAVA-6218
…addRetryableLabelOrGetWriteAttemptFailureNotToBeRetried` JAVA-6218
- Make the parameter order of `SyncOperationHelper.withSourceAndConnection` consistent with `AsyncOperationHelper.withAsyncSourceAndConnection`. - Remove `throws` from `AsyncOperationHelper.withAsyncSourceAndConnection`/`withAsyncSuppliedResource`. JAVA-6218
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors MixedBulkWriteOperation retry handling to use per-batch RetryState instances and shared retry helpers, while updating related helper APIs, tests, and async control-flow conventions.
Changes:
- Reworks mixed bulk write sync/async retry flow and removes the custom bulk-write retry tracker.
- Updates shared retry/helper APIs and callers for source/connection handling and retry failure bookkeeping.
- Adjusts functional/unit tests and shared test utilities to reflect the new retry/resource lifecycle.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java |
Refactors bulk write batching, retry, async execution, and resource holder logic. |
driver-core/src/main/com/mongodb/internal/operation/SyncOperationHelper.java |
Reorders withSourceAndConnection parameters and updates retry failure handling. |
driver-core/src/main/com/mongodb/internal/operation/AsyncOperationHelper.java |
Updates async retry failure handling and helper signatures. |
driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java |
Renames retry-label helper and passes server deprioritization directly. |
driver-core/src/main/com/mongodb/internal/operation/ClientBulkWriteOperation.java |
Aligns helper usage and async control flow with the refactored retry helpers. |
driver-core/src/main/com/mongodb/internal/operation/retry/AttachmentKeys.java |
Removes obsolete bulk write tracker attachment key. |
driver-core/src/main/com/mongodb/internal/async/function/RetryState.java |
Removes external last-attempt marking API. |
driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java |
Documents MutableValue usage in async lambdas. |
driver-core/src/main/com/mongodb/internal/operation/FindOperation.java |
Updates withSourceAndConnection call signature. |
driver-core/src/main/com/mongodb/internal/operation/ListCollectionsOperation.java |
Updates withSourceAndConnection call signature. |
driver-core/src/main/com/mongodb/internal/operation/ListIndexesOperation.java |
Updates withSourceAndConnection call signature. |
driver-core/src/test/functional/com/mongodb/OperationFunctionalSpecification.groovy |
Adds configurable expected connection release counts. |
driver-core/src/test/functional/com/mongodb/internal/operation/MixedBulkWriteOperationSpecification.groovy |
Updates mixed bulk write retry expectations. |
driver-core/src/test/unit/com/mongodb/internal/async/function/RetryStateTest.java |
Removes tests for deleted markAsLastAttempt. |
driver-sync/src/test/functional/com/mongodb/client/MongoWriteConcernWithResponseExceptionTest.java |
Refactors test for sync/reactive reuse and failpoint listener utility. |
driver-sync/src/test/functional/com/mongodb/client/FailPoint.java |
Makes failpoint guard close idempotent. |
driver-reactive-streams/src/test/functional/com/mongodb/reactivestreams/client/MongoWriteConcernWithResponseExceptionTest.java |
Reuses sync test through reactive sync adapter override. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
The code is so good that Copilot failed to find anything! 💪 😃 |
stIncMale
commented
May 29, 2026
| getWriteAttemptFailureNotToBeRetriedOrAddRetryableLabel(retryState, mongoException); | ||
| resultAccumulator.onBulkWriteCommandErrorWithoutResponse(mongoException); | ||
| if (retryWritesSetting) { | ||
| // Adding the `RetryableError` label here is unnecessary at this point: |
Member
Author
There was a problem hiding this comment.
@stIncMale TODO Change to RetryableWriteError. Search for other occurrences in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For reviewers
I recommend reviewing the changes commit by commit. Review the sync code changes to assess the correctness compared to the pre-PR code. Then review the async code changes to assess whether the sync code was correctly translated into the async code.
AI usage
All the changes were handmade. AI was used only to review the changes.
JAVA-6218