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

Add message-db Store #339

Merged
merged 45 commits into from
Nov 11, 2022
Merged

Add message-db Store #339

merged 45 commits into from
Nov 11, 2022

Conversation

nordfjord
Copy link
Contributor

@nordfjord nordfjord commented Nov 6, 2022

Implementation of a MessageDB storage backend for Equinox.

Notable exclusions:

  • MessageDB does not support reading stream backwards, for this reason there is no RollingSnapshots Access strategy provided

@CLAassistant
Copy link

CLAassistant commented Nov 6, 2022

CLA assistant check
All committers have signed the CLA.

type BatchOptions(getBatchSize : Func<int>, [<O; D(null)>]?batchCountLimit) =
new (batchSize) = BatchOptions(fun () -> batchSize)
member _.BatchSize = getBatchSize.Invoke()
member _.MaxBatches = batchCountLimit
Copy link
Collaborator

Choose a reason for hiding this comment

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

wont affect the IL but this can be a val too now ;)

@bartelink bartelink changed the title Add a message db store Add message-db Store Nov 11, 2022
@bartelink
Copy link
Collaborator

This looks done done to me ;)

@bartelink
Copy link
Collaborator

Thank you for the work, and for making reviewing it a pleasure!

@bartelink bartelink merged commit 4b07909 into jet:master Nov 11, 2022
module private Token =
let create streamVersion : StreamToken =
{ value = box streamVersion
version = streamVersion
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an oversight on my part... This needs to be streamVersion+1 due to

/// Exposes the underlying Store's internal Version for the underlying stream.
/// An empty stream is Version 0; one with a single event is Version 1 etc.
/// It's important to consider that this Version is more authoritative than counting the events seen, or adding 1 to
/// the `Index` of the last event passed to your `fold` function - the codec may opt to ignore events
abstract member Version : int64

This mess is due to the historic 'arguable' misdesign of having a stream with one event be 'Version' 0 in ESDB (that and the empty vs not created vs no events magic values).

(Cosmos and Dynamo use 0 for an empty stream - i.e. the version is events.Length)

If MessageDb is -1 based, then the chances are the only change required is adding 1 to the internal value (and adding the warning). If it's not, then we should take this opportunity to use 0-based indexes to make it all less mindbending.

https://github.com/jet/equinox/blob/master/src/Equinox.SqlStreamStore/SqlStreamStore.fs#L298

The other obvious thing here is that the tests should have caught this...
using Decider.QueryEx, the Version can be obtained by supplying a mapResult

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.

3 participants