event-driven-io / emmett

Emmett - a Node.js library taking your event-driven applications back to the future!
https://event-driven-io.github.io/emmett/
195 stars 19 forks source link

MongoDB Event Store Implentation #126

Closed alex-laycalvert closed 2 weeks ago

alex-laycalvert commented 3 weeks ago

Work in progress PR for an EventStore implementation for mongodb.

I am utilizing mongoose to assist in the model definition but this is only as a wrapper for the actual mongodb driver. If we don't want to use mongoose I can look at removing it's usage.

Another thing that might need to change is using number for the default stream version instead of a bigint. I only did this to make dealing with the version checking easier. In this impl., the stream version is determined by the number of events which is an array field on the document representing each event stream and that .length field is always a number type.

@oskardudycz looking forward on working on this one with you and getting this in once it's fully ready

alex-laycalvert commented 3 weeks ago

Updated to remove mongoose in favor of the mongodb nodejs driver.

I'll leave some comments on the PR about certain decisions I made here.

alex-laycalvert commented 3 weeks ago

Been thinking about a global log position and I'm not sure if it's going to be possible, or at least simple to implement with mongodb.

Please correct me if I'm wrong here but it would need to be something like:

// 1. Probably uising an aggregation, retrieve all events, order by global position, retrieve last one.
const lastEvent = collection.aggregate(...)

// 2. Using `lastEvent`, increment the global log position for each event we want to append
const eventInputs = [...]

// 3. Insert events into stream
collection.findOneAndUpdate(...)

But with the above impl, I don't know if we can guarantee that other events would have been made between step 2 and 3 creating a race condition.

We could use a global counter but I think it's susceptible to the same issues. Looking into possible locks on documents that we can have to make this simpler

oskardudycz commented 3 weeks ago

Been thinking about a global log position and I'm not sure if it's going to be possible, or at least simple to implement with mongodb.

Please correct me if I'm wrong here but it would need to be something like:

// 1. Probably uising an aggregation, retrieve all events, order by global position, retrieve last one.
const lastEvent = collection.aggregate(...)

// 2. Using `lastEvent`, increment the global log position for each event we want to append
const eventInputs = [...]

// 3. Insert events into stream
collection.findOneAndUpdate(...)

But with the above impl, I don't know if we can guarantee that other events would have been made between step 2 and 3 creating a race condition.

We could use a global counter but I think it's susceptible to the same issues. Looking into possible locks on documents that we can have to make this simpler

Definitely, for MongoDB we won't be providing the global ordering guarantee, as this will result in half-assed performance and unscalable solution.

oskardudycz commented 3 weeks ago

@alex-laycalvert could you add the basic set of tests in the tests projects (as other stores): https://github.com/event-driven-io/emmett/tree/main/src/packages/emmett-tests/src/eventStore? After that, I'll be fine with merging it. We can tackle issues that we were discussing in separate PRs so as not to do all at once.

And thank you again for creating this PR and contributing back to Emmett. That's much appreciated :+1:

alex-laycalvert commented 3 weeks ago

Sure thing @oskardudycz I'll clean up the thing about the updatedAt date condition (originally a typo), then add the tests.

oskardudycz commented 3 weeks ago

Sure thing @oskardudycz I'll clean up the thing about the updatedAt date condition (originally a typo), then add the tests.

Thanks a lot!

alex-laycalvert commented 2 weeks ago

Hey @oskardudycz , let me know if there's anything else you think we should add to this initial implementation before it can get merged, thanks!

oskardudycz commented 2 weeks ago

@alex-laycalvert, apologies for the delay. I've been constantly traveling for the last two weeks, and that got me a bit off schedule. I'm pulling that in. I'll try to write all the notes and issues based on the discussion in this PR about the next steps (like e.g. projections, retries, etc.). And tag you in there.

Would you like me to release the package as it is?

oskardudycz commented 2 weeks ago

@alex-laycalvert and again, thank you a lot for your work, and contributing it back ❤️