Eventstore: Same events with same EventId writes to same stream (expected not)

[iRatkin]

Describe the bug
Hello. I used node-eventstore-client (https://github.com/nicdex/node-eventstore-client) for TCP connection to EventStore server.
In my task i used md5-to-uuid for creating EventId based on event data.
Well, if i use appendToStream with ExpectedVersion.any, the same events with same EventId will record in the same stream.

If i use another ExpectedVersion (noStream, streamExists, integer etc, like in documentation below https://eventstore.org/docs/dotnet-api/optimistic-concurrency-and-idempotence/index.html ), the error WrongExpectedVersion reproduce. Picture with error below!

My code below:

To Reproduce

Steps to reproduce the behavior:

1. Make docker image eventstore/eventstore:release-4.1.2
2. Run docker image, post events with same body and EventId to the same stream

Expected behavior
Second event will not write to stream

Actual behavior
Second event will write to stream with same eventId.

The same problem with https://github.com/EventStore/EventStore/issues/1949, but issue was closed

[nicdex]

I found a way to reproduce this.
For some reason when using expected version any and using an eventId that exists in the target stream and also exists in another stream the event will be saved every time.

You can reproduce with this

[Test]
        public void Test_passing_append_same_event_with_expected_version_any() {
            var failures = 0;
            //var eventId = Guid.NewGuid();
            for (var i = 0; i < 2; i++) {
                var stream = Guid.NewGuid().ToString();
                var eventId = Guid.NewGuid();
                var eventData = new EventData(eventId, "MyEvent", true, Encoding.Default.GetBytes("{\"a\":1}"), new byte[0]);
                _sut.AppendToStreamAsync(stream, -1, eventData).Wait();
                var result = _sut.AppendToStreamAsync(stream, ExpectedVersion.Any, eventData).Result;
                if (result.NextExpectedVersion != 0) {
                    failures++;
                    Console.WriteLine("Failed #{0} {1} {2}", i, stream, eventId);
                }
            }
            Assert.AreEqual(0, failures);
        }

[Test]
        public void Test_failing_append_same_event_with_expected_version_any() {
            var failures = 0;
            var eventId = Guid.NewGuid();
            for (var i = 0; i < 2; i++) {
                var stream = Guid.NewGuid().ToString();
                //var eventId = Guid.NewGuid();
                var eventData = new EventData(eventId, "MyEvent", true, Encoding.Default.GetBytes("{\"a\":1}"), new byte[0]);
                _sut.AppendToStreamAsync(stream, -1, eventData).Wait();
                var result = _sut.AppendToStreamAsync(stream, ExpectedVersion.Any, eventData).Result;
                if (result.NextExpectedVersion != 0) {
                    failures++;
                    Console.WriteLine("Failed #{0} {1} {2}", i, stream, eventId);
                }
            }
            Assert.AreEqual(0, failures);
        }

The difference in the tests being one re-use the same eventId across mutiple streams and the other does not.

[condron]

This seems to match the expected behavior
https://eventstore.org/docs/dotnet-api/optimistic-concurrency-and-idempotence/index.html#idempotence

ExpectedVersion.Any turns off the idenpotecy checks, and the idenpotency key is EventId+Stream, so writing the same event to multiple streams is allowed.

Let me know if I am not understanding the issue correctly.

[nicdex]

Ok then I guess I'm more trying to understand why the first test (with eventid only in one stream) always pass (idempotency works even if disabled with any) and the second test (eventid repeated in multiple streams) always fails (idempotency never works) if the idempotency is "disabled"?

Also documentation ia unclear at the top it says any = disabled but when you read the section about any it insinuate that it works but it's not guaranteed which isn't the same as disabled.

[nicdex]

I re-read the documentation. "Any" disables the optimistic concurrency check not the idempotency check which is diffent. There's a note saying that idempotency is not guaranteed, but it also says that duplicate are a small chance. I have a reproducible way of creating a duplicate in a stream, when the eventid is also used in another stream, that's not a small chance.

[gregoryyoung]

There are two idempotency checks that occur.

The first is if expected version is present to actually look at the events in the stream.

The second is to check a hashtable of recent writes to see if the same one is there (stream+id iirc). Saying "if I write the same event to a different stream its not idempotent" does not even make sense. Idempotency is about repeating the same operation not some other operation. By definition this would not be "idempotent"

Just imagine the confusion if when you write to stream streamA it succeeded but never wrote because the event already existed in SomeOtherStreamB

[nicdex]

Still not answering the question.

Given these two tests:

Test 1
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 2 to stream B - Success: nextExpectedVersion = 0
Save eventId 2 to stream B - Success: nextExpectedVersion = 0 (Idempotent)
Test passed I expect both second saves on streams to succeed with nextExpectedVersion = 0

Test 2
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 1 to stream B - Success: nextExpectedVersion = 0
Save eventId 1 to stream B - Success: nextExpectedVersion = 1 (Not idempotent)
Test failed I expect both second saves on streams to succeed with nextExpectedVersion = 0

Why does test 1 pass and not test 2?

[gregoryyoung]

I am not sure what you expect to have happen here.

Certainly by changing the expected version to a different expected version
this throws away the entire term of "idempotent"?!

On Sun, Aug 4, 2019 at 10:35 PM Nicolas Dextraze [e-mail]
wrote:

Still not answering the question.

Given these two tests:

Test 1
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 2 to stream B - Success: nextExpectedVersion = 0
Save eventId 2 to stream B - Success: nextExpectedVersion = 0 (Idempotent)
Test passed I expect both second saves on streams to succeed with
nextExpectedVersion = 0

Test 2
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 1 to stream B - Success: nextExpectedVersion = 0
Save eventId 1 to stream B - Success: nextExpectedVersion = 1 (Not
idempotent)
Test failed I expect both second saves on streams to succeed with
nextExpectedVersion = 0

Why does test 1 pass and not test 2?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/EventStore/EventStore/issues/1970?email_source=notifications&email_token=AAC5CWUXO5L2KNFAHQJY7HDQC6GYXA5CNFSM4IHRIDH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QQUAQ#issuecomment-518064642,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5CWVYHA7FDI6I37DNCRLQC6GYXANCNFSM4IHRIDHQ
.

--
Studying for the Turing test

[nicdex]

Seriously this is getting ridiculous. It's like I'm speaking an alien language. I'm out. @iRatkin up to you to explain.

[gregoryyoung]

Test 2
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 1 to stream B - Success: nextExpectedVersion = 0
Save eventId 1 to stream B - Success: nextExpectedVersion = 1 (Not idempotent)
Test failed I expect both second saves on streams to succeed with nextExpectedVersion = 0

when you say "eventId" you are referring to the uuid associated on the event?

If so I think I know how this is happening. I believe it is assumed to be unique ;-)

[nicdex]

yes eventId is the uuid associated with the event. The first parameter in the new EventData constructor

[gregoryyoung]

EventId is assumed to be ... unique.

Test 1
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 2 to stream B - Success: nextExpectedVersion = 0
Save eventId 2 to stream B - Success: nextExpectedVersion = 0 (Idempotent)
Test passed I expect both second saves on streams to succeed with nextExpectedVersion = 0

Works exactly as expected... Note: two separate ids

Test 2
Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 1 to stream B - Success: nextExpectedVersion = 0
Save eventId 1 to stream B - Success: nextExpectedVersion = 1 (Not idempotent)

Again this works as I would expect due to the reuse of the id. It went and looked and saw in the cache the id. It saw it was something else and therefore said "something is already there". I could make an argument that given two events, in different streams, that the second one should "takeover" for the first but this is ... also pretty weird and could lead to the opposite odd case if implemented as well (it would reverse). Here it said "I already have one so will keep that one". There is an issue with this...

Save eventId 1 to stream A - Success: nextExpectedVersion = 0
Save eventId 1 to stream A - Success: nextExpectedVersion = 0 (Idempotent)
Save eventId 1 to stream B - Success: nextExpectedVersion = 0
Save eventId 1 to stream B - Success: nextExpectedVersion = 0 (idempotent)
Save eventId 1 to stream A - Success: nextExpectedVersion = 1 (not Idempotent)

Its also worth noting that this would work with expected version I believe if set to something as it would use the events in the stream and not the raised hash of recent ids to determine idempotency (need to double check, it may use both).

The issue here is the ids which are assumed to be unique.

I will look more into this usecase though not really sure what can be done about it aside from better documenting that event id is assumed to be unique. Lifting stream into a hash could be very expensive need to double check ... maybe a best effort as a hash of the stream (should make a few billion times or so less likely to happen :)

Related code as a note: https://github.com/EventStore/EventStore/blob/master/src/EventStore.Core/Services/Storage/ReaderIndex/IndexWriter.cs#L163

[iRatkin]

@gregoryyoung @nicdex My thanks for your attention!

Situation is:

1. Create event A with EventId uuid (based on hash(A))
2. Write (A, uuid) in Stream1 with ExpectedVersion.Any
3. Write (A, uuid) in Stream1 again with ExpectedVersion.Any
4. As a result, 2 absolutely same events in one stream.

Is it expected result?
And so how can we stop writing same events without this mechanism?

BTW, thanks for you work! EventStore and node.js client reduced launch time of the docker in 5 times compared to event stored in mongo!

[gregoryyoung]

@iRatkin What is the time period/count of other events between the writes?

When using ExpectedVersion.Any the only idempotence check is the one of recent writes which does not hold a huge number of events. I believe its 1m off the top of my head but could verify it if you want. If there is a long time period it can fall out of this window.

If using ExpectedVersion a quick test should show that idempotency is honored here.

The reason for .Any only working for a window is that it requires resources (needs to hold the ids at some point).

[iRatkin]

@gregoryyoung few minutes / hours. Thanks for telling about limited time for ExpectedVersion.Any's idempotence check, didnt know it.
If our time between 2 same events can be more than few days, what mechanism (Or version of ExpectedVersion) can you advice?

[iRatkin]

Same events, same ID's, same stream (url)

[iRatkin]

Well, if i send same events throw HTTP request, it behave expected way: only first event were writed.

[condron]

what is the use case / situation producing this resubmission?
If the application is going to run out the idenpotecy cache then one way to address it is to read the stream before writing.

[iRatkin]

Use case: save envents from smart contracts on local storage

I will test reading stream, thanks!

[iRatkin]

@condron well tested:

1. Read events from stream
2. Write duplicates to stream
3. Events were recorded to stream (expected no duplicates)

[gregoryyoung]

@condron well tested:

1. Read events from stream
2. Write duplicates to stream
3. Events were recorded to stream (expected no duplicates)

How were they written (ExpectedVersion.Any vs ExpectedVersion?) My guess is ExpectedVersion.Any. With ExpectedVersion there is an assurance.

How were they "duplicated"? EG: what was the timeframe/load? As stated there is a cache kept even for use with ExpectedVersion.Any but if this is over minutes+ with other load (or a restart and some other things)... its quite likely the overall amount is larger than this cache.

If you are using ExpectedVersion you should be able to assure no duplicates, ExpectedVersion.Any is only a best effort. The reason for this is that an assurance with ExpectedVersion.Any would be an unbounded amount of work! When using ExpectedVersion the pattern is to read out events (build your state etc) then write back using ExpectedVersion showing what you have read. This is to prevent another writer having written to the stream during this time.

[hayley-jean]

Are you possibly restarting Event Store between writing the two events, or batching the two writes together?

The cache of existing event Id's is empty on restart, and with batching, only the first ID is checked.
As Greg and Chris have stated, duplicates should be prevented by using ExpectedVersion and unique ID's

[alex-wernick-cko]

Hi,

If anyone is still interested I have found the cause of the issue.

The issue is caused by the below line

https://github.com/EventStore/EventStore/blob/master/src/EventStore.Core/Services/Storage/ReaderIndex/IndexWriter.cs#L163

Lets say we have stream A and stream B

A has just had an event written to it with eventId

If you write and event with eventId to stream B with expected version Any or StreamExists there will be eventId in the cache but associated stream A. This will result in a CommitDecision.Ok being returned on line 164.

Now we have

Stream A has event with eventId
Stream B has event with eventId

Now again if you write to a stream B with expected version Any or StreamExists there will be eventId in the cache but still associated stream A (see PutRecord function in BoundedCache.cs when throwOnDuplicate is false). This will result in a CommitDecision.Ok being returned on line 164 and hence a duplicate eventId in stream B