-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
Optionally disable sequence numbers using optimistic locking (etags) #362
base: main
Are you sure you want to change the base?
Conversation
mumby0168
commented
Nov 2, 2023
•
edited
Loading
edited
- 1. Introduces a new way to sequence events without optimistic locking
- 2. Adds the ability to use in memory testing for events
- 3. Adds the ability to test change feed projections using an in-memory implementation
- 4. Shows how the in-memory library can be used with functional tests via a sample
- document non-sequenced option along with the aggregate root settings with the config option
- document how the in-memory testing works
|
||
namespace Microsoft.Azure.CosmosEventSourcing.Options; | ||
|
||
public class CosmosEventSourcingOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need triple slash for these public
APIs.
IEventStore<TEventItem> where TEventItem : EventItem | ||
{ | ||
private readonly IOptionsMonitor<CosmosEventSourcingOptions> _optionsMonitor = optionsMonitor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field isn't needed, you can access the optionsMonitor
anywhere within the partial class
. These are primary constructors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhh that makes sense now, I'll update this cheers!
@@ -22,9 +22,12 @@ internal partial class DefaultEventStore<TEventItem> | |||
return; | |||
} | |||
|
|||
if (eventItems.Count(x => x.EventName is nameof(AtomicEvent)) is not 1) | |||
if(_optionsMonitor.CurrentValue.IsSequenceNumberingDisabled is false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(_optionsMonitor.CurrentValue.IsSequenceNumberingDisabled is false) | |
if (optionsMonitor.CurrentValue.IsSequenceNumberingDisabled is false) |
typeof(TEventItem), | ||
aggregateRoot.AtomicEvent, | ||
partitionKey) as TEventItem); | ||
if(_optionsMonitor.CurrentValue.IsSequenceNumberingDisabled is false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(_optionsMonitor.CurrentValue.IsSequenceNumberingDisabled is false) | |
if (optionsMonitor.CurrentValue.IsSequenceNumberingDisabled is false) |
switch (domainEvent) | ||
{ | ||
case ReplayableEvent: | ||
ReplayedEvents++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any race condition concerns with multiple threads calling apply at the same time, and reads of the ReplayedEvents
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes there will be, kind of a long story to this change, I plan to update the docs alongside this as well. But in short, the sequence number and the strong consistency provided by the atomic event is great in a single region scenario.
However, when we have been assessing the ability to enable multi region writes where traffic would be load balanced across two regions both writing to there local cosmos db data store, we could not guarantee that this sequence number remains in sync and in some use cases for event sourcing it doesn't really matter. The plan here is to handle simultaneous writes, as long as that event is valid at the point in time we are going to let it go ahead.
This diagram is covering our use case, but I am still fleshing out all the details of whether this is 100% suitable via some POCs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with it if you are, let's
y is EventItemPartitionKeyAttribute)) | ||
.ToList(); | ||
|
||
switch (partitionKeyProperties.Count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer switch expressions.
@@ -36,4 +39,41 @@ eventItem.DomainEvent switch | |||
null => null, | |||
_ => throw new ArgumentOutOfRangeException() | |||
}; | |||
|
|||
public static TAggregateRoot Replay<TAggregateRoot, TEventItem>(this IEnumerable<TEventItem> events) | |||
where TEventItem : EventItem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where TEventItem : EventItem | |
where TEventItem : EventItem |
public static IEnumerable<TEventItem> SetCorrelationId<TEventItem>( | ||
this IEnumerable<TEventItem> eventItems, | ||
IContextService contextService) | ||
where TEventItem : EventItem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where TEventItem : EventItem | |
where TEventItem : EventItem |