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

Naming convention #20

Open
prolic opened this Issue May 23, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@prolic
Member

prolic commented May 23, 2015

Hi,
first of all, many thanks for this great library. Expect some PR's in near future from me. (MongoDB EventStore Adapter could be the first!)

On thing I noticed during code review:

class Prooph\Proophessor\EventStore\AbstractRepositoryFactory

The name hints for an abstract class, but it's an abstract service factory (it should not get extended). So a better class name would be RepositoryAbstractFactory instead of AbstractRepositoryFactory.

Merely a small detail regarding the whole thing ;)

Keep on doing great stuff!

@codeliner

This comment has been minimized.

Show comment
Hide comment
@codeliner

codeliner May 23, 2015

Member

Hi,
good catch. I will change that.

PR's are always welcome and a mongoDB adapter is something I have on my todo for long time now.
Problem with mongoDB is the document size of 16 megabytes. I think the only good option for mongoDB is to use the AggregateStreamStrategy or work with snapshots and clean up old events. But yeah, an adapter would be great 👍
I will create a repo for the adapter.

Btw I've seen your HumusAmqpModule 👍
Another point on my todo is a RabbitMQ message dispatcher for prooph/service-bus :-)

Member

codeliner commented May 23, 2015

Hi,
good catch. I will change that.

PR's are always welcome and a mongoDB adapter is something I have on my todo for long time now.
Problem with mongoDB is the document size of 16 megabytes. I think the only good option for mongoDB is to use the AggregateStreamStrategy or work with snapshots and clean up old events. But yeah, an adapter would be great 👍
I will create a repo for the adapter.

Btw I've seen your HumusAmqpModule 👍
Another point on my todo is a RabbitMQ message dispatcher for prooph/service-bus :-)

@prolic

This comment has been minimized.

Show comment
Hide comment
@prolic

prolic May 23, 2015

Member

I could also imagine building that amqp integration for you. But first of all, I need to do some improvements on the amqp module itself and release 1.0.0 ;-)

Member

prolic commented May 23, 2015

I could also imagine building that amqp integration for you. But first of all, I need to do some improvements on the amqp module itself and release 1.0.0 ;-)

@prolic

This comment has been minimized.

Show comment
Hide comment
@prolic

prolic May 23, 2015

Member

For the mongodb adapter: I can also imagine modelling the documents slightly different. Instead of having all events mapped as embedded objects of the aggregate, simply persist all events to a distinct event collection.
I can't imagine that you would store more than 16MB of references for a single entity, but even then, we have some choices left ;-)

Member

prolic commented May 23, 2015

For the mongodb adapter: I can also imagine modelling the documents slightly different. Instead of having all events mapped as embedded objects of the aggregate, simply persist all events to a distinct event collection.
I can't imagine that you would store more than 16MB of references for a single entity, but even then, we have some choices left ;-)

@codeliner

This comment has been minimized.

Show comment
Hide comment
@codeliner

codeliner May 23, 2015

Member

👍 I'm looking forward to the implementation. I will create the repo tomorrow and post the link here. Now it's time to shutdown my system :-)

Thank you for your help and input. I really appreciate it.

Member

codeliner commented May 23, 2015

👍 I'm looking forward to the implementation. I will create the repo tomorrow and post the link here. Now it's time to shutdown my system :-)

Thank you for your help and input. I really appreciate it.

@codeliner

This comment has been minimized.

Show comment
Hide comment
@codeliner
Member

codeliner commented May 24, 2015

@prolic empty mongodb adapter repo is online: https://github.com/prooph/event-store-mongodb-adapter

@prolic

This comment has been minimized.

Show comment
Hide comment
@prolic

prolic May 25, 2015

Member

Great thanks!

Two questions come into my mind first:

a) Do we make a pure mongodb adapter or a doctrine-mongodb adapter? I really like the abstraction doctrine-mongodb provides!
b) MongoDB does not support transactions, but it has atomic modifiers and ttl documents, so enough tools to build transactions manually. For the event stream a transactional behavior would be useful, right?
I think of something like this:

- Start transaction sets a property into the adapter that transaction is running
- During in-transaction persistence we add a "inTransaction":true property on the document.
- All findById and findAll and similar queries check for the inTransaction property
- After transaction is finished, we can remove the inTransaction property with an atomic modifier to enable the new state
- An entry marked "inTransation" has a ttl (time-to-life) on the server side, in case the process dies, we don't want to have pending transactions in the database, so they get cleaned up after some time.

If we do something like this, which TTL will we use? MySQL InnoDB has a innodb_lock_wait_timeout of 50 secs by default f.e.

About maximum document size: We can perhabs have 2 adapters?
The first one has the 16MB per document limitation, but will work slightly faster. The second one stores a reference to a mongo grid fs entry and has no limitations at all.

Member

prolic commented May 25, 2015

Great thanks!

Two questions come into my mind first:

a) Do we make a pure mongodb adapter or a doctrine-mongodb adapter? I really like the abstraction doctrine-mongodb provides!
b) MongoDB does not support transactions, but it has atomic modifiers and ttl documents, so enough tools to build transactions manually. For the event stream a transactional behavior would be useful, right?
I think of something like this:

- Start transaction sets a property into the adapter that transaction is running
- During in-transaction persistence we add a "inTransaction":true property on the document.
- All findById and findAll and similar queries check for the inTransaction property
- After transaction is finished, we can remove the inTransaction property with an atomic modifier to enable the new state
- An entry marked "inTransation" has a ttl (time-to-life) on the server side, in case the process dies, we don't want to have pending transactions in the database, so they get cleaned up after some time.

If we do something like this, which TTL will we use? MySQL InnoDB has a innodb_lock_wait_timeout of 50 secs by default f.e.

About maximum document size: We can perhabs have 2 adapters?
The first one has the 16MB per document limitation, but will work slightly faster. The second one stores a reference to a mongo grid fs entry and has no limitations at all.

@codeliner

This comment has been minimized.

Show comment
Hide comment
@codeliner

codeliner May 25, 2015

Member

@prolic I've split your questions into issues ^^ so that we can discuss them individually

Member

codeliner commented May 25, 2015

@prolic I've split your questions into issues ^^ so that we can discuss them individually

@codeliner

This comment has been minimized.

Show comment
Hide comment
@codeliner

codeliner Oct 12, 2017

Member

crazy, this issue is still not closed. We should keep it open for historic reasons :D

Member

codeliner commented Oct 12, 2017

crazy, this issue is still not closed. We should keep it open for historic reasons :D

@prooph prooph locked and limited conversation to collaborators Jun 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.