Skip to content

Fix RUBY-2558 :secondary_preferred read preference in 3.6+ sharded clusters performs primary read #2200

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

Merged
merged 3 commits into from
Apr 9, 2021

Conversation

p-mongo
Copy link
Contributor

@p-mongo p-mongo commented Mar 26, 2021

No description provided.

@p-mongo p-mongo changed the title RUBY-2558 2558 wip Fix RUBY-2558 :secondary_preferred read preference in 3.6+ sharded clusters performs primary read Apr 3, 2021
@p-mongo
Copy link
Contributor Author

p-mongo commented Apr 3, 2021

@p-mongo p-mongo force-pushed the 2558 branch 6 times, most recently from 407d360 to 66bd58d Compare April 5, 2021 16:24
# In replica sets and sharded clusters, read preference is passed
# to the server if one is specified by the application, and there
# is no default.
read && read.slave_ok? || false
Copy link
Member

Choose a reason for hiding this comment

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

Question without knowing internal semantics, why does this require || false to be included? Asking because A || false will always evaluate to A in boolean algebra, so I'm wondering about what Ruby does when read evaluates to a false-ish value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The || false converts nil to false (when read is nil, in this case).

Comment on lines 97 to 99
# If the read preference contains only mode and mode is secondary
# preferred and we are sending to a pre-OP_MSG server, this read
# preference is indicated by the :slave_ok wire protocol flag.
Copy link
Member

Choose a reason for hiding this comment

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

Took me a moment to realise that the comment explains when the $readPreference option doesn't need to be added. Maybe add another sentence like "In all other cases, add the $readPreference option" might help clarify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I added a note to this effect.

p added 2 commits April 8, 2021 12:03
* master:
  Use frozen strings by default  (mongodb#2202)
  RUBY-2561 Remove skipReason from estimatedDocumentCount test in versioned API (mongodb#2203)
@p-mongo p-mongo merged commit 696147d into mongodb:master Apr 9, 2021
p-mongo added a commit that referenced this pull request Apr 9, 2021
…usters performs primary read

 (#2200)

* Fix RUBY-2558 :secondary_preferred read preference in 3.6+ sharded clusters performs primary read

* clarify

Co-authored-by: Oleg Pudeyev <oleg@bsdpower.com>
p-mongo added a commit that referenced this pull request Apr 9, 2021
…usters performs primary read

 (#2200)

* Fix RUBY-2558 :secondary_preferred read preference in 3.6+ sharded clusters performs primary read

* clarify

Co-authored-by: Oleg Pudeyev <oleg@bsdpower.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