Skip to content

Comments

fix(NODE-7298): ensure commonWireVersion is computed from server maxWireVersion#4805

Merged
tadjik1 merged 11 commits intomongodb:mainfrom
crehbichler:NODE-7298
Jan 23, 2026
Merged

fix(NODE-7298): ensure commonWireVersion is computed from server maxWireVersion#4805
tadjik1 merged 11 commits intomongodb:mainfrom
crehbichler:NODE-7298

Conversation

@crehbichler
Copy link
Contributor

@crehbichler crehbichler commented Nov 24, 2025 •
edited by tadjik1
Loading

Description

Summary of Changes

The TopologyDescription constructor initialized commonWireVersion to 0, but the update() method only updates this field when its previous value is null. As a result, commonWireVersion remained stuck at 0 even when all servers reported valid non-zero maxWireVersion values.

This caused secondaryWritableServerSelector() to treat the cluster as pre–MongoDB 5.0 (because wireVersion < 13) and always fall back to primary read preference. User-provided read preferences (e.g. secondary, secondaryPreferred) were therefore ignored for aggregation pipelines containing write stages ($merge and $out).

The fix initializes commonWireVersion to null, allowing it to be correctly updated to the minimum non-zero maxWireVersion across server descriptions.

A new test file test/unit/sdam/topology_description.test.ts has been added to cover initialization and update behavior.

Release Highlight

Fixed Read Preference adherence for $merge and $out aggregations.

Resolved an issue where the driver failed to detect MongoDB 5.0+ capabilities due to incorrect commonWireVersion initialization. This ensures that aggregations with write stages now correctly respect secondary and secondaryPreferred read preferences, rather than forcing execution on the Primary.

Huge thanks to @crehbichler for discovering, investigating this bug and for implementing a fix!

Double check the following

  • Lint is passing (npm run check:lint)
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@crehbichler crehbichler requested a review from a team as a code owner November 24, 2025 16:59
@tadjik1 tadjik1 added tracked-in-jira Ticket filed in MongoDB's Jira system External Submission PR submitted from outside the team labels Nov 25, 2025
@tadjik1 tadjik1 self-assigned this Nov 27, 2025
@tadjik1 tadjik1 added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Nov 27, 2025
@tadjik1
Copy link
Member

tadjik1 commented Nov 27, 2025

Fantastic job @crehbichler, thanks a lot! Can you please also add integration test for the specific scenario you've found in the jira ticket, i.e. aggregate with write stages that must be issued against secondary and not primary.
For this test we can utilize command monitoring feature, similar to one in the snippet that I've shared.

@crehbichler
Copy link
Contributor Author

crehbichler commented Nov 27, 2025 •
edited
Loading

Added!

I wasn’t 100% sure whether to place the integration test in aggregation or server-selection/read-preference, since the issue was more in server selection, but the result was a violation of the aggregation spec for writable stages, so I opted for aggregation.

Also, in the test I am checking the server address (I used the hello command) because the correct $readPreference was already attached to the command before the fix (just the wrong server was being selected).

Please let me know if this is fine or if I should adjust anything.

tadjik1
tadjik1 reviewed Dec 3, 2025
View reviewed changes
baileympearson
baileympearson requested changes Dec 3, 2025
View reviewed changes
@tadjik1
Copy link
Member

tadjik1 commented Dec 3, 2025

Also, in the test I am checking the server address (I used the hello command) because the correct $readPreference was already attached to the command before the fix (just the wrong server was being selected).

I think this is valid approach, another can be to issue 2 aggregate commands with and w/o readPreference and make sure their address fields are not the same (primary and secondary both being used).

@tadjik1
Copy link
Member

tadjik1 commented Dec 9, 2025

@crehbichler thanks a lot for updating the PR! Some tests are failing, we will investigate failures and come back with an update in upcoming days. Sorry for the delay 🙏

tadjik1
tadjik1 reviewed Dec 22, 2025
View reviewed changes
tadjik1
tadjik1 approved these changes Dec 22, 2025
View reviewed changes
Copy link
Member

@tadjik1 tadjik1 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @crehbichler, great work!

@dariakp dariakp added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Jan 5, 2026
@tadjik1 tadjik1 merged commit 2b2366d into mongodb:main Jan 23, 2026
26 of 29 checks passed
@github-actions github-actions bot mentioned this pull request Jan 22, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tadjik1 tadjik1 tadjik1 approved these changes

@baileympearson baileympearson baileympearson approved these changes

Assignees

@tadjik1 tadjik1

Labels

External Submission PR submitted from outside the team Team Review Needs review from team tracked-in-jira Ticket filed in MongoDB's Jira system

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@crehbichler @tadjik1 @baileympearson @dariakp