DocumentManager::getDocumentCollection() always overwrites readPreference to 'primary' #759

Closed
hver opened this Issue Jan 15, 2014 · 6 comments

Comments

Projects
None yet
3 participants
Contributor

hver commented Jan 15, 2014

When you get a DocumentCollection using DocumentManager::getDocumentCollection()
( https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/DocumentManager.php),
the readPreference is alwas set to primary .

      public function getDocumentCollection($className)
       ...
        if (isset($metadata->slaveOkay)) {
            $collection->setSlaveOkay($metadata->slaveOkay);
        }
        ...

The problem is, that the default value for $metadata->slaveOkay is always set, even if there is no annotation in the document file, and the value of the property is set to false.
Because of this, every time you load a collection, its readPreference is explicitly set to $this->mongoCollection->setReadPreference(\MongoClient::RP_PRIMARY);

I see two possible workarounds:

  1. Add to every document annotation /** @Document(slaveOkay=true) */ or /** @Document(slaveOkay=false) */
  2. Remove default value from metadata info (https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php, line 189)
    public $slaveOkay; instead of public $slaveOkay = false;

I think, that setting slaveOkay as default to false instead of null is wrong, and prefer the second way but I don't know if the existing behavior is a 'feature' for compatibility with old driver versions. If it's not, I would like to make a pull request with changes.

Owner

jmikola commented Jan 17, 2014

@jwage: I only found ClassMetadataInfo::$slaveOkay referenced in DocumentManager::getDocumentCollection(), so removing its default value seems like it'd work in this case. Thoughts?

Owner

jwage commented Jan 17, 2014

If a class sets read pref in the mapping can a single query overwrite it still?

Owner

jmikola commented Jan 17, 2014

It actually doesn't look like the query builder inherits read preferences or slaveOkay at all.

Contributor

hver commented Jan 18, 2014

The query builder inherits readPreference from the connection. As stated in http://www.php.net/manual/en/mongo.readpreferences.php:

Read preferences may be specified on either a per-connection, per-database, or per-collection basis. Preferences defined at a higher level will be inherited by default

@jwage It is possible to set a readPreference for a single query with MongoCursor::setReadPreference: http://de1.php.net/manual/en/mongocursor.setreadpreference.php

Owner

jmikola commented Jan 18, 2014

@hver: The Builder class has a setReadPreference() method, which Query applies temporarily during execution. That's in doctrine/mongodb.

hver added a commit to hver/mongodb-odm that referenced this issue Jan 20, 2014

Contributor

hver commented Jan 20, 2014

@jmikola: Right, and in the function Query::withReadPreference the property slaveOkay is not mentioned at all. I've made a pull request, which removed the default value from slaveOkay.

hver added a commit to hver/mongodb-odm that referenced this issue Jan 21, 2014

jmikola added a commit that referenced this issue Jan 21, 2014

Use null default value for ClassMetadataInfo slaveOkay (fixes #759)
This ensures DocumentManager::getDocumentCollection() will not overwrite read preferences by default.

jmikola added a commit that referenced this issue Jan 21, 2014

Use null default value for ClassMetadataInfo slaveOkay (fixes #759)
This ensures DocumentManager::getDocumentCollection() will not overwrite read preferences by default.

@jmikola jmikola closed this in #781 Jan 21, 2014

jwage added a commit that referenced this issue Jan 27, 2014

Use null default value for ClassMetadataInfo slaveOkay (fixes #759)
This ensures DocumentManager::getDocumentCollection() will not overwrite read preferences by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment