-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
Introduce support for sharded collections #1385
Conversation
👍 |
aa5a539
to
2cb1a58
Compare
*/ | ||
public function setShardKey(array $keys, array $options = array()) | ||
{ | ||
if ($this->inheritanceType == self::INHERITANCE_TYPE_SINGLE_COLLECTION && !is_null($this->shardKey)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
===
should be used
e51ed56
to
ba16cf2
Compare
90447c3
to
60e11cd
Compare
@@ -489,3 +489,54 @@ it on a per query basis: | |||
$qb->requireIndexes(true); | |||
$query = $qb->getQuery(); | |||
$results = $query->execute(); | |||
|
|||
Shard keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capital "K" here.
This avoids people having to exclude or include a test group depending on the environment they're running and instead relies on feature detection to run the correct tests.
* Since multi-key indexes are not allowed, we're not allowing collections and many relationships to be used as shard keys * Only allow fields using SET storage strategy being used as shard keys
* @param object $document | ||
* @throws \BadMethodCallException | ||
*/ | ||
protected function processDocumentCollection(SchemaManager $sm, $document) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this and methods below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are defined as abstract in the AbstractCommand
class: https://github.com/doctrine/mongodb-odm/blob/b93cae185896cfeeb7c4b6dcef5d9c7da82be1a2/lib/Doctrine/ODM/MongoDB/Tools/Console/Command/Schema/AbstractCommand.php#L31..L41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wow I had no idea we have something like this 👍
Good to go once tests are green, yay! 👍 |
This PR mainly builds on top of the work done by @clslrns in #1025. Changes include running travis tests on sharded cluster and some more relaxed error handling to account for different error messages depending on the MongoDB server version used. It also adds documentation for the sharding mapping as well as the
@ShardKey
annotation.This also supersedes the work done in #691 and #325 which have been too long-running to be useful at this stage.
One feature that will be added at a later stage is the support for denormalizing the shard key values in non-simple references; this is not required but can greatly improve query times when fetching references on a sharded collection. This will be another PR as I'm waiting for #1349 to finalize the changes made to simple/full references.
TODO:
increment
strategy are excluded from shard keys. While this is technically a valid solution it might make more sense to just not update these fields in the query builder instead of throwing an error in the mapping.Store shard key values along with dbRef objects (waiting on storeAs option for references #1349)