Skip to content

Fix read_preference #1042

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 2 commits into from
Nov 24, 2015
Merged

Fix read_preference #1042

merged 2 commits into from
Nov 24, 2015

Conversation

wojcikstefan
Copy link
Member

I noticed something disturbing in our query logs earlier today - some of the queries were incorrectly routed to the primary when using ReadPreference.SECONDARY. Eventually, it came to my realization that read_preference wasn't working if it was put after limit, skip, or hint (and probably some other methods that allow chaining).

Note: As of now, the fix only works for pymongo ver < 3.0. I think read_preference is completely broken with ver >= 3.0, but I don't have full clarity on the situation yet. It seems that self._collection.find doesn't accept read_preference as a valid kwarg any more and if I'm reading pymongo's docs right, you can only set read preference on a client/database/collection, not a particular cursor. It doesn't seem to be set either way:

In [10]: import mongoengine

In [11]: from mongoengine import *

In [12]: from pymongo.read_preferences import ReadPreference

In [13]: import pymongo

In [14]: pymongo.version
Out[14]: '3.0.2'

In [15]: class A(Document):
    txt = StringField()
   ....:

In [16]: qs = A.objects.read_preference(ReadPreference.SECONDARY)

In [17]: qs._cursor._Cursor__read_preference
Out[17]: Primary()

In [18]: qs._cursor.collection.read_preference
Out[18]: Primary()

Review on Reviewable

@landscape-bot
Copy link

Code Health
Repository health increased by 0.01% when pulling 3f80337 on elasticsales:fix-read-preference into c6151e3 on MongoEngine:master.

@MRigal
Copy link
Member

MRigal commented Jun 25, 2015

Yes, it works quite differently with PyMongo3, it has to be defined at connection. We need at least to update the docs and also maybe to raise some additional warning.

In order to make your PR mergeable, please add an if for the assert statement if IS_PYMONGO3 adding a comment why it is not working anymore with PyMongo3

@MRigal MRigal added this to the 0.10.1 milestone Jun 25, 2015
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling bf169ac on elasticsales:fix-read-preference into c6151e3 on MongoEngine:master.

@wojcikstefan wojcikstefan force-pushed the fix-read-preference branch from bf169ac to 8b84014 Compare July 5, 2015 23:08
@wojcikstefan
Copy link
Member Author

@MRigal I rebased this PR against the latest master and also fixed read_preference for PyMongo 3+. Let me know what you think.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling 8b84014 on elasticsales:fix-read-preference into 9671ca5 on MongoEngine:master.

@wojcikstefan
Copy link
Member Author

@thedrow rebased.

@thedrow thedrow merged commit 9050869 into MongoEngine:master Nov 24, 2015
@amcgregor
Copy link
Contributor

@thedrow Ref #1169 could we get a Python Package Index release to go with the Git tags?

@wojcikstefan wojcikstefan deleted the fix-read-preference branch May 5, 2016 00:43
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.

4 participants