Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-5981): read preference not applied to commands properly #4010

Merged
merged 31 commits into from
Mar 11, 2024

Conversation

alenakhineika
Copy link
Contributor

@alenakhineika alenakhineika commented Mar 1, 2024

Description

Add $readPrefernce to commands according to spec.

Topology type: Single

A deployment of topology type Single contains only a single server of any type. Topology type Single signifies a direct connection intended to receive all read and write operations.

Therefore, read preference is ignored during server selection with topology type Single. The single server is always suitable for reads if it is available. Depending on server type, the read preference is communicated to the server differently:

  • Type Mongos: the read preference is sent to the server using the rules for Passing read preference to mongos and load balancers.
  • Type Standalone: clients MUST NOT send the read preference to the server
  • For all other types, using OP_QUERY: clients MUST always set the SecondaryOk wire protocol flag on reads to ensure that any server type can handle the request.
  • For all other types, using OP_MSG: If no read preference is configured by the application, or if the application read preference is Primary, then $readPreference MUST be set to { "mode": "primaryPreferred" } to ensure that any server type can handle the request. If the application read preference is set otherwise, $readPreference MUST be set following Document structure.

The single server is always suitable for write operations if it is available.

What is changing?

Implementation

  • When preparing a command to the server / prepareCommand(), apply $readPrefernce to commands according to spec
  • Pass the directConnection option when we create a connection in the connection pool

Tests

  • Update test runner to allow overwriting host and port in configuration URL in order to connect separately to each host in replicaset.
  • Convert test/integration/server-selection/readpreference.test.js to TypeScript
  • Unskip readPreference tests (fixes NODE-5263)
  • Sync recent run-command spec tests
  • Run max staleness on replicaset, because accordion to spec, clients MUST NOT send the read preference to the standalone server
Is there new documentation needed for these changes?

None

What is the motivation for this change?

NODE-5981

Release Highlight

Fixed applying read preference to commands depending on topology

When connecting to a secondary in a replica set with a direct connection, if a read operation is performed, the driver attaches a read preference of primaryPreferred to the command.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken changed the title fix(NODE-5981): Implement 6.x: Read preference not applied to commands properly fix(NODE-5981): read preference not applied to commands properly Mar 1, 2024
@alenakhineika alenakhineika marked this pull request as ready for review March 5, 2024 22:38
@nbbeeken nbbeeken self-assigned this Mar 6, 2024
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Mar 6, 2024
@nbbeeken nbbeeken self-requested a review March 6, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants