Open mikecann opened 6 years ago
Hey, good question. The commit id is used for sorting when scanning events across all aggregates. In order to ensure events are ordered properly the commit id must be fixed length. We could have used the timestamp as a number so long as we left padded with zero but I preferred the human readable version personally. For this same reason, aggregate ids are also assumed to be fixed length, so I would recommend using uuid v4. I hope this helps clear up a few things.
Hey, thanks for the reply!
In order to ensure events are ordered properly the commit id must be fixed length
Why is this? Is it because on this line you are doing this comparison commitId >= :c
which is comparing against the commitID which is actually a string? Thus you need the length of the entire commitId to be constant in length for an accurate string comparison?
I was thinking that the only place you need to do a scan of the DB in ES is in projector "rehydration". Instead of doing a scan however, could you just use a query against the "active" field (which would be GlobalIndex'd against the committedAt field instead of commitId field) then use the LastEvaluatedKey in combination with the ExclusiveStartKey as a reference for paging.
Technically you might not need the commitId at all then?
I have been using very similar (mostly copied) structure to this one for a while now - https://github.com/domagojk/beenion/blob/rater/databases/eventstore/dynamoDbEventStore.ts (thank you @bakerface !)
but I'm not using fixed length strings on timestamp and version properties. For example, I had aggregateId where the last version is about 150, and it sorts that by version from 1 to 150 just fine.
It seems to me that could be an issue only when you are using strings, not numbers. But please correct me if I'm wrong. Maybe I should change that structure? @bakerface Is there some documentation on this where I can learn more about the subject (I didn't find any on "fixed length" props)?
Also, i agree with @mikecann about not needing commitId for querying. You can see that in my implementation I do have that field, but I don't actually use it. getByTimestamp
function that i use for projections is using committedAt
as Mike mentioned
awesome! thanks for the info. I will take a look at what you have and see where I can make updates to my repo.
Hi,
I was just looking through the code (good stuff btw!) and noticed here:
https://github.com/bakerface/dynamodb-event-store/blob/master/index.js#L116
You are taking the timestamp then converting to a date then getting the iso string and removing the dashes.
I guess im missing something here, why not just use the timestamp directly?