-
-
Notifications
You must be signed in to change notification settings - Fork 89
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?
Changes from 2 commits
9d79ad5
e63861f
9d4304b
f4e21fd
9b29fdc
d375a5f
26c5f1e
9f04d9c
0c9c164
36cfcc9
622d4ad
594995c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// Copyright (c) David Pine. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
namespace Microsoft.Azure.CosmosEventSourcing.Options; | ||
|
||
public class CosmosEventSourcingOptions | ||
{ | ||
public const string SectionName = "CosmosEventSourcing"; | ||
|
||
public bool IsSequenceNumberingDisabled { get; set; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,18 @@ | |
|
||
using System.Runtime.CompilerServices.Context; | ||
using Microsoft.Azure.CosmosEventSourcing.Items; | ||
using Microsoft.Azure.CosmosEventSourcing.Options; | ||
using Microsoft.Azure.CosmosRepository; | ||
using Microsoft.Extensions.Options; | ||
|
||
namespace Microsoft.Azure.CosmosEventSourcing.Stores; | ||
|
||
internal partial class DefaultEventStore<TEventItem>( | ||
IBatchRepository<TEventItem> batchRepository, | ||
IReadOnlyRepository<TEventItem> readOnlyRepository, | ||
IContextService contextService) : | ||
IContextService contextService, | ||
IOptionsMonitor<CosmosEventSourcingOptions> optionsMonitor) : | ||
IEventStore<TEventItem> where TEventItem : EventItem | ||
{ | ||
private readonly IOptionsMonitor<CosmosEventSourcingOptions> _optionsMonitor = optionsMonitor; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This field isn't needed, you can access the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahhh that makes sense now, I'll update this cheers! |
||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -22,9 +22,12 @@ public async ValueTask PersistAsync( | |||||
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 commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
throw new AtomicEventRequiredException(); | ||||||
if (eventItems.Count(x => x.EventName is nameof(AtomicEvent)) is not 1) | ||||||
{ | ||||||
throw new AtomicEventRequiredException(); | ||||||
} | ||||||
} | ||||||
|
||||||
await batchRepository.UpdateAsBatchAsync( | ||||||
|
@@ -78,7 +81,7 @@ private static IEnumerable<TEventItem> SetCorrelationId( | |||||
return items; | ||||||
} | ||||||
|
||||||
private static IEnumerable<TEventItem> BuildEvents( | ||||||
private IEnumerable<TEventItem> BuildEvents( | ||||||
IAggregateRoot aggregateRoot, | ||||||
string partitionKey) | ||||||
{ | ||||||
|
@@ -90,10 +93,13 @@ private static IEnumerable<TEventItem> BuildEvents( | |||||
partitionKey) as TEventItem) | ||||||
.ToList(); | ||||||
|
||||||
events.Add(Activator.CreateInstance( | ||||||
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 commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
events.Add(Activator.CreateInstance( | ||||||
typeof(TEventItem), | ||||||
aggregateRoot.AtomicEvent, | ||||||
partitionKey) as TEventItem); | ||||||
} | ||||||
|
||||||
return events.Any(x => x is null) | ||||||
? throw new InvalidOperationException( | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,32 @@ private TestAggregate() | |
} | ||
} | ||
|
||
private class TestAggregateNoSequencing : AggregateRoot | ||
{ | ||
public int ReplayedEvents { get; private set; } | ||
|
||
protected override void Apply(DomainEvent domainEvent) | ||
{ | ||
switch (domainEvent) | ||
{ | ||
case ReplayableEvent: | ||
ReplayedEvents++; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I'm good with it if you are, let's |
||
break; | ||
} | ||
} | ||
|
||
public static TestAggregateNoSequencing Replay(List<DomainEvent> domainEvents) | ||
{ | ||
TestAggregateNoSequencing a = new(); | ||
a.Apply(domainEvents); | ||
return a; | ||
} | ||
|
||
private TestAggregateNoSequencing() : base(isSequenceNumberDisabled: true) | ||
{ | ||
} | ||
} | ||
|
||
private class TestAggregateRootMapper : IAggregateRootMapper<TestAggregate, ReplayableEventItem> | ||
{ | ||
public IEnumerable<ReplayableEventItem> MapFrom(TestAggregate aggregateRoot) => throw new NotImplementedException(); | ||
|
@@ -71,67 +97,19 @@ public TestAggregate MapTo(IEnumerable<ReplayableEventItem> domainEvents) => | |
TestAggregate.Replay(domainEvents.Select(x => x.DomainEvent).ToList()); | ||
} | ||
|
||
private class AggregateWithNoReplayMethod : AggregateRoot | ||
{ | ||
protected override void Apply(DomainEvent domainEvent) => throw new NotImplementedException(); | ||
} | ||
|
||
[Fact] | ||
public async Task ReadAggregateAsync_AggregateWithReplayMethod_ReplaysEvents() | ||
private class TestAggregateRootNoSequencingMapper : IAggregateRootMapper<TestAggregateNoSequencing, ReplayableEventItem> | ||
{ | ||
//Arrange | ||
IEventStore<ReplayableEventItem> sut = _autoMocker.CreateInstance<DefaultEventStore<ReplayableEventItem>>(); | ||
public IEnumerable<ReplayableEventItem> MapFrom( | ||
TestAggregateNoSequencing aggregateRoot) => throw new NotImplementedException(); | ||
|
||
Mock<IReadOnlyRepository<ReplayableEventItem>> repository = _autoMocker.GetMock<IReadOnlyRepository<ReplayableEventItem>>(); | ||
|
||
ReplayableEventItem atomicEvent = new(new AtomicEvent(Guid.Empty.ToString(), "etag"), "A"); | ||
atomicEvent.SetPrivatePropertyValue(nameof(FullItem.Etag), Guid.NewGuid().ToString()); | ||
|
||
List<ReplayableEventItem> events = new() | ||
{ | ||
new ReplayableEventItem(new ReplayableEvent(), "A"), | ||
atomicEvent, | ||
}; | ||
|
||
repository | ||
.Setup(o => | ||
o.GetAsync(x => x.PartitionKey == "A", default)) | ||
.ReturnsAsync(events); | ||
|
||
//Act | ||
TestAggregate a = await sut.ReadAggregateAsync<TestAggregate>("A"); | ||
|
||
//Assert | ||
a.ReplayedEvents.Should().Be(1); | ||
public TestAggregateNoSequencing MapTo( | ||
IEnumerable<ReplayableEventItem> domainEvents) => | ||
TestAggregateNoSequencing.Replay(domainEvents.Select(x => x.DomainEvent).ToList()); | ||
} | ||
|
||
[Fact] | ||
public async Task ReadAggregateAsync_AggregateMapper_MapsAggregateCorrectly() | ||
private class AggregateWithNoReplayMethod : AggregateRoot | ||
{ | ||
//Arrange | ||
IEventStore<ReplayableEventItem> sut = _autoMocker.CreateInstance<DefaultEventStore<ReplayableEventItem>>(); | ||
|
||
Mock<IReadOnlyRepository<ReplayableEventItem>> repository = _autoMocker.GetMock<IReadOnlyRepository<ReplayableEventItem>>(); | ||
|
||
ReplayableEventItem atomicEvent = new(new AtomicEvent(Guid.Empty.ToString(), "etag"), "A"); | ||
atomicEvent.SetPrivatePropertyValue(nameof(FullItem.Etag), Guid.NewGuid().ToString()); | ||
|
||
List<ReplayableEventItem> events = new() | ||
{ | ||
new ReplayableEventItem(new ReplayableEvent(), "A"), | ||
atomicEvent | ||
}; | ||
|
||
repository | ||
.Setup(o => | ||
o.GetAsync(x => x.PartitionKey == "A", default)) | ||
.ReturnsAsync(events); | ||
|
||
//Act | ||
TestAggregate a = await sut.ReadAggregateAsync("A", new TestAggregateRootMapper()); | ||
|
||
//Assert | ||
a.ReplayedEvents.Should().Be(1); | ||
protected override void Apply(DomainEvent domainEvent) => throw new NotImplementedException(); | ||
} | ||
|
||
[Fact] | ||
|
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.