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

Yet another sharding support implementation #1025

Closed
wants to merge 32 commits into from

Conversation

clslrns
Copy link
Contributor

@clslrns clslrns commented Feb 4, 2015

This one is inspired by #691.

Brief summary:

  • shardKey property was added to ClassMetadataInfo, mapping support were added to all drivers. It's a hash with keys and options. keys was originally called fields, but I decided to conform to the indexes definition.
  • shardKey is class-based, since we need to keep the order of the keys to ensure compound index.
  • ensureSharding method were added to SchemaManager. It's similar to ensureIndex. For non-empty collections it will run ensureIndex and retry shardCollection again.
  • DocumentPersister uses shardKey to generate query part for upsert, update, remove, findOne.

I tried to cover all the new code with both unit and functional tests. Tests were run against sharded clusters of MongoDB 2.2.3, 2.4.9 and 2.6.5. Functional tests were tagged to avoid running against non-sharded setup.

Now I'm testing it on production. I'd like to receive some feedback from you guys to improve or change something.

@clslrns
Copy link
Contributor Author

clslrns commented Feb 4, 2015

It seems, tests failed because of long MongoDB initialization. Would be great if somebody re-ran them.

@jmikola jmikola added this to the 1.0.0-BETA13 milestone Mar 11, 2015
@jmikola
Copy link
Member

jmikola commented Mar 11, 2015

@clslrns: I will attempt to get to this before the next beta tag.

@clslrns
Copy link
Contributor Author

clslrns commented Mar 11, 2015

Btw, wouldn't it be better to squash all this commits?

@malarzm malarzm modified the milestones: 1.0.0-BETA13, 1.0.0-BETA14 May 20, 2015
@notrix
Copy link
Contributor

notrix commented Jun 1, 2015

👍

@malarzm malarzm modified the milestones: 1.0.0-BETA14, 1.x Jun 11, 2015
@alcaeus
Copy link
Member

alcaeus commented Oct 8, 2015

@clslrns sorry for the long time without updates. Could you please rebase the branch to see if it still works against the current master?

One more note:

Btw, wouldn't it be better to squash all this commits?

Leave them as is - we can always talk about squashing commits before merging. For now this preserves important history.

* @param object $document The document to refresh.
*/
public function refresh($id, $document)
public function refresh($document)
Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid this is a BC break. As useless as the ID parameter has become, it has to stay. Feel free to deprecate it though.

@clslrns
Copy link
Contributor Author

clslrns commented Oct 18, 2015

@alcaeus, thanks for the feedback.
I have rebased it and fixed two issues you pointed to. I hope, in a few days I'll test rebased version against the app.

@darceee2
Copy link

👍

@malarzm malarzm modified the milestones: 1.x, 1.1 Oct 31, 2015
@alcaeus
Copy link
Member

alcaeus commented Mar 14, 2016

Superseded by #1385.

@alcaeus alcaeus closed this Mar 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants