Skip to content

Conversation

@vbabanin
Copy link
Member

@vbabanin vbabanin commented Nov 1, 2025

This PR addresses server bug SERVER-113344 by modifying the client-side bulk write operation handling to work around unexpected server responses.

The changes:

  • Remove an assertion that was failing due to server bug SERVER-113344 where successful operation results are unexpectedly returned in the cursor when verbose results are disabled.
  • Add unit tests for JAVA-6001 and JAVA-5986.

JAVA-6001
JAVA-5986

@vbabanin vbabanin self-assigned this Nov 1, 2025
@vbabanin vbabanin changed the title Remove assertion. Remove assertion for verbose result Nov 1, 2025
// Note: Previously, assertTrue(verboseResultsSetting) was used here because the server was not supposed
// to return successful operation results in the cursor when verboseResultsSetting is false.
// Due to server bug SERVER-113344, these unexpected results must be ignored until we stop supporting server
// versions with this bug. When that happens, restore assertTrue(verboseResultsSetting) here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave a TODO-<JAVA ticket ID> tag here, and create a corresponding ticket, which is linked as "blocked by" / "depends on" (not sure what relations we have) to the aforementioned server ticket.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have to keep this code for some time (years) - while we wait for servers to be updated.

Copy link
Member Author

@vbabanin vbabanin Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a TODO-JAVA-6002 tag and created a follow-up ticket to restore the assertion.

I linked it to the server ticket as blocked by. As @rozza mentioned, even after the server fix lands it may not be backported to 8.x, so we’ll likely re-enable the assertion once our minimum supported server version includes the fix (or gate it conditionally on server version).

writeModelIndex,
new ConcreteClientUpdateResult(
individualOperationResponse.getInt32("n").getValue(),
individualOperationResponse.getInt32("nModified", new BsonInt32(0)).getValue(),
Copy link
Member

@stIncMale stIncMale Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me that this change, coming from #1823, should have also had a corresponding JAVA ticket describing that it is to be rolled back once https://jira.mongodb.org/browse/SERVER-113026 (it is not the same as the aforementioned SERVER-113344) is fixed and no version where the bug is present is supported. The TODO-<Jira ticket ID> tag will be needed here as well, to point to the place that needs to be changed.

Copy link
Member Author

@vbabanin vbabanin Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the TODO tag and created the corresponding ticket TODO-JAVA-6005 . I also added a unit test for the behavior introduced in that PR.

@vbabanin vbabanin requested review from rozza and stIncMale November 13, 2025 06:36
fail(writeModel.getClass().toString());
}
if (okFieldValue == 1) {
collectSuccessfulIndividualOperationResult(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO comments made the method larger and harder to follow. To keep the control flow maintainable, I moved one of the logical branches into a separate method. This keeps the main if/else structure concise and makes each path easier to identify and understand.

@vbabanin vbabanin requested a review from Copilot November 13, 2025 22:36
@vbabanin vbabanin marked this pull request as ready for review November 13, 2025 22:36
@vbabanin vbabanin requested a review from a team as a code owner November 13, 2025 22:36

This comment was marked as resolved.

…tBulkWriteOperationTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants