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

MongoDB as EventStore #151

Closed
wants to merge 1 commit into from
Closed

Conversation

jaymecd
Copy link

@jaymecd jaymecd commented Apr 8, 2015

Introducing support of MongoDB as EventStore.

Features included in this PR:

  • read queries against primary nodes for strong data consistency
  • read committed isolation level to avoid reading partial event commits
  • test cases adopted for MongoDB


if (count($events) == 1) {
try {
$this->saveEvent(reset($events));
Copy link
Member

Choose a reason for hiding this comment

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

Is there a specific reason you choose to do this separately if there is exactly 1 event? I'm not really seeing the benefit.

Copy link
Author

Choose a reason for hiding this comment

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

Single document insert does not require 2pc. It's done to save number of requests.

@wjzijderveld
Copy link
Member

Thanks for the PR, really appreciate it!

@jaymecd jaymecd force-pushed the eventstore_mongodb branch 3 times, most recently from 31be3bf to 035bfe9 Compare April 11, 2015 21:24
@jaymecd
Copy link
Author

jaymecd commented Apr 11, 2015

@wjzijderveld updated, but scrutinizer failed by some unexpected reason.
Any idea? I've contacted support, waiting.

@wjzijderveld
Copy link
Member

I just restarted The scrutrnizer job, they fsil from time to time.

I'll look at you other comments later :-)

}

/**
* {@inheritdoc}
Copy link
Member

Choose a reason for hiding this comment

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

This method isn't in the interface, no doc-block required here.

Copy link
Author

Choose a reason for hiding this comment

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

updated

- read against primary node
- read committed isolation level
@francescotrucchia
Copy link
Contributor

👍

@wjzijderveld
Copy link
Member

@qandidate-labs/broadway I'm fine with merging this as is (can optionally fix the exception assignment separate). What do others think?

@jaymecd Have you been running this in production? If not, maybe we should add a note in the README that this is experimental?

@jaymecd
Copy link
Author

jaymecd commented Aug 7, 2015

@wjzijderveld nope, not yet, so experimental would be important.

*/
public function configureCollection()
{
$this->eventCollection->ensureIndex(array(
Copy link

Choose a reason for hiding this comment

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

ensureIndex is deprecated. use createIndex instead.

Copy link

Choose a reason for hiding this comment

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

sorry, doctrine mongodb uses still the ensureIndex method ;)

@jaymecd
Copy link
Author

jaymecd commented Aug 17, 2015

@jmikola tnx for tips, I review code a bit later.

@jmikola
Copy link

jmikola commented Aug 17, 2015

@jaymecd: No problem. @asm89 just sent me this PR over IRC and invited me to take a look :)

@danizord
Copy link

Any news on this one?

@prolic
Copy link

prolic commented Sep 20, 2015

MongoDB has no transaction support. Using the $isolated operator you can achieve atomic updates on a single collection, but not on multiple ones. Additionally the $isolated operator does not work in a sharded cluster environment. That said, it's not a good idea to run this in a very high performant situation with sharding enabled.
You can check out our implementation of the mongo-db-adapter for prooph/event-store, to see how this is working with manual transaction support. The PR to address the $isolated operator and changing README notes is here: prooph/event-store-mongodb-adapter#27.

@jaymecd
Copy link
Author

jaymecd commented Oct 7, 2015 via email

@wjzijderveld
Copy link
Member

@jaymecd No problem, hope you are okay!

If you want any help on it, let me know!

@jaymecd
Copy link
Author

jaymecd commented Nov 20, 2015

@wjzijderveld, I've made a deep dive into the problems and issues noted by @jmikola and @prolic and have doubts if solution should be based on:

So, I'm asking for your opinion on this issue before continue. Personally I'd go with mongodb extension/library and then, if it's required, with legacy extension support as separate storage class MongoDBLegacyEventStore.

@wjzijderveld
Copy link
Member

@jaymecd Thanks for diving into it!

Hmm, tricky choice there. But I think the better way would be to go with the new extension. Compatibility with HHVM is something I would definitely like to support :-)
Downsides: we use the deprecated mongo extension (with Doctrine) for Saga states at the moment. So we probable should do something about that as well.

@qandidate-labs/broadway What do you think?

@codeliner
Copy link

Also the legacy driver will not be updated to support php7. No HHVM support ok, but PHP7 should not be a question ;) If you also want to have legacy version you can wrap the 1.0 version of prooph/event-store-mongodb-adapter in a class implementing Broadway event store. Maybe faster than writing a legacy adapter yourself.
Our 2.0 version will use the new extension.

@jaymecd
Copy link
Author

jaymecd commented Nov 20, 2015

@codeliner, I don't think wrapping prooph implementation would be a good idea, as it brings another level to diamond dependencies + prooph/common: one ES implementation partially includes another one - that's more than strange.

@jaymecd
Copy link
Author

jaymecd commented Nov 20, 2015

@codeliner @jmikola - $isolated operator does not work in a sharded cluster.

Is there any way to have same atomicity within sharded setup? As event store data could consume all disk space on single shard, isn't it?

@codeliner
Copy link

@jaymecd : @prolic is our mongoDB specialist. As far as I know there is no way. We are looking for alternatives, too. Depending on your domain contexts one shard per context is maybe enough. Also when using snapshots in combination with "archiving" old events you may not run out of disk space. So mongoDB is still a valid option.

one ES implementation partially includes another one

Well, Broadway provides only an ES interface and different independent ES implementations. prooph/event-store on the other hand uses different adapters. I did not say you should require prooph/event-store but only one of the available adapters. Our packages are stand-alone (btw. something that Broadway should consider, too). However, it was more a hint for people who may need to use the old extension but want to use other features of Broadway. I don't consider Broadway a competitor. Would like to use the Saga implementation but unfortunately it is not available as a stand-alone package and installing everything incl. the Sf bundle, aggregate root classes, etc. is really not an option.

@prolic
Copy link

prolic commented Nov 20, 2015

Running out of disk-space? Really? I can store all your events on my micro-sd card! Unless you are not twitter and have multiple thousands events per second, you never run out of disk space. Even if so, you should upgrade your hardware and buy a larger hard disk.

For $isolated: There is no other solution, you can't run mongodb event store in a sharded cluter environment. If you need more speed, consider using postgres.

@sstok
Copy link

sstok commented Nov 21, 2015

sharded cluter epic typo 😂

If you need more speed, consider using postgres.

Here 👍

@jaymecd
Copy link
Author

jaymecd commented Nov 24, 2015

In order to save time I've started to update implementation (using hints above) with mongodb extension and it's library to have MongoDB as event store, as PHP7 & HHVM really worth it.

@Konafets
Copy link

Konafets commented Feb 1, 2016

Hi @jaymecd,

In order to save time I've started to update implementation (using hints above) with mongodb extension and it's library to have MongoDB as event store, as PHP7 & HHVM really worth it.

How is your progress on that?

@wjzijderveld
Copy link
Member

I started to refactor the Saga\MongoDBRepository to use the new extension #236

@jaymecd
Copy link
Author

jaymecd commented Feb 2, 2016

@Konafets @wjzijderveld I didn't had a chance to continue on that, so far could not provide any ETA from my side.
I would highly appreciate if you could continue on his PR.

@othillo
Copy link
Member

othillo commented Feb 18, 2017

I'm closing this for now. Additional event stores are very welcome, though! We are actually splitting implementations to separate repositories at the moment. If someone wants to work on a new broadway/event-store-mongodb implementation, please ping us! /cc @broadway/broadway

Thank you for your effort!

@othillo othillo closed this Feb 18, 2017
@othillo
Copy link
Member

othillo commented Feb 22, 2017

work on this will continue in this repo: https://github.com/broadway/event-store-mongodb

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.

None yet

10 participants