Refactor MixedBulkWriteOperation to simplify retry logic there#1989
Refactor MixedBulkWriteOperation to simplify retry logic there#1989stIncMale wants to merge 8 commits into
MixedBulkWriteOperation to simplify retry logic there#1989Conversation
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.
|
The code is so good that Copilot failed to find anything! 💪 😃 |
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
JAVA-6218
6b6d44f to
9a03ec1
Compare
| import static com.mongodb.internal.operation.OperationHelper.LOGGER; | ||
| import static com.mongodb.internal.operation.OperationHelper.isRetryableWrite; | ||
| import static com.mongodb.internal.operation.OperationHelper.validateWriteRequests; | ||
| import static com.mongodb.internal.operation.OperationHelper.validateWriteRequestsAndCompleteIfInvalid; |
There was a problem hiding this comment.
TODO @stIncMale: delete OperationHelper.validateWriteRequestsAndCompleteIfInvalid.
@vbabanin Note that the beginAsync API allows us to use OperationHelper.validateWriteRequests in both sync and async implementations, so OperationHelper.validateWriteRequestsAndCompleteIfInvalid is no longer used, but I forgot to delete it.
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.
Ignore the
ServerDiscoveryAndMonitoringProseTests.testConnectionPoolBackpressureEvergreen failures. There are unrelated and we'll deal with them separately.AI usage
All the changes were handmade. AI was used only to review the changes.
JAVA-6218