Handle truncated streams when reading events for projections

[StuartFergusonVme]

Fixed: Fix projections getting stuck when reading from truncated streams

Fixes #2954

[StuartFergusonVme]

#2954

[StevenBlair123]

Any chance we could get this approved to run on the CI process?
Just need to verify tests etc are working

[hayley-jean]

Thank you for the PR @StuartFergusonVme!
However, changing the StorageReaderWorker isn't what we'd want to do in this situation.

When a stream has been truncated, the storage reader should return an empty response with NextEventNumber set to the event number of the next available event to read from and IsEndOfStream set to false. It is then up to the client to issue another read from there.

It looks like the projections aren't making use of the IsEndOfStream property of the read, and simply stopping when there are no events.
Would you mind updating the EOF logic in the following readers to use the IsEndOfStream property instead, and see if that fixes the issue?

• EventByTypeIndexEventReader
• MultiStreamEventReader
• StreamEventReader
• TransactionFileEventReader

[StuartFergusonVme]

@hayley-jean updated as requested.

I have added the changes in from #2947 with this as well as without these changes the Access Violation error is hit that has been fixes with that PR.

[StevenBlair123]

Could someone approve this so the workflow can kick off?

[hayley-jean]

@StuartFergusonVme Thanks for updating! However when I test this locally I see that this change is actually more involved than I initially thought

The projection readers assume that EOF means that there are no events, so don't process events or continue reading once it sets EOF to true - resulting in the projection still getting stuck.

For example, the StreamEventReader only publishes the events to the projections if it's not EOF, and the other readers appear to have similar issues.
Would you be willing to fix this in the readers?

[StevenBlair123]

@hayley-jean - does that mean we need to check message.events.length

So we would do:

var eof = (message.events.length == 0) && message.IsEndOfStream

[hayley-jean]

I think that could work @StevenBlair123, it would be the closest to what the readers seem to expect at the moment

[StevenBlair123]

OK, we will plug that in just now.

[StuartFergusonVme]

@hayley-jean pushed up some more updates as requested, can you approve the CI process please?

[StuartFergusonVme]

@hayley-jean just noticed from the last run that the test "EventStore.Core.Tests.ClientAPI.when_connection_drops_messages_that_have_run_out_of_retries_are_not_retried" failed only on ubuntu-18.04 with the error
Error Message:
2021-05-19T07:14:58.5890544Z Expected: No Exception to be thrown
2021-05-19T07:14:58.5892880Z But was: <System.InvalidOperationException: Subscription group 91101607-5696-46ad-80a2-82449b9fc462 on stream a213f024-620b-47a0-8dfe-3e718915473b already exists

Is this just a timing issue when running the tests as I can't see how the code changes I have made can affect this especially as it is passing on windows?

[hayley-jean]

@StuartFergusonVme Thanks! The changes you've made fixes the issue for stream, event type and $all projections 😄

The tests aren't failing because of this PR - they've been rather flaky recently. If this PR breaks anything it would only be in Projections.Core.Tests

The only one left that doesn't work is multi-stream projections, for example:

fromStreams("foo", "bar")
.when({
    $init: function() {
        return { count: 0 }
    },
    $any: function(s,e) {
        s.count++;
    }
})

There's probably something preventing the read from moving on in the case when no events are returned, if you want to have a look at that in the MultiEventReader.
Otherwise I can see if I can push a fix for the multi stream projections later today or tomorrow

[StevenBlair123]

@hayley-jean - we had actually tested this specifically on multi streams (one of the streams was the root of the problem) and it looked like this code was working.
Handle is called for each, and our check should verify if we were at the end of stream.
Clearly, there is something we have missed, but could you give us some guidance on this?

[hayley-jean]

@StevenBlair123 Sure, the test I ran was:

1. Write 2000 events to stream foo. Leave stream bar empty
2. Delete foo
3. Write another 20 events to foo
4. Create the following projection

fromStreams("foo", "bar")
.when({
    $init: function() {
        return { count: 0 }
    },
    $any: function(s,e) {
        s.count++;
    }
})

How many truncated events did you test with?
The multi-stream projection reads large batches of events at a time, so it may still have received some events which allowed it to continue running.

As for fixing it, it looks like the positions that are used in the reads are only updated when events are delivered (see here).
This means that the initial empty read won't update the positions, resulting in the projection reading from position 0 over and over again. So updating the _fromPositions on an empty non-eof read should fix this

[StevenBlair123]

@hayley-jean - thanks. we will have a look at this tomorrow.

[hayley-jean]

Thanks! This all works on my side now 👍

[YoEight]

Thanks for your contribution! It looks good to me too. Could you get rid of unnecessary changes and squash your commit too?

[StuartFergusonVme]

@YoEight I think I have done what you have asked for but the CI process is blocked now, would you be able to approve the workflow and confirm the change I have done is as you requested.

[StevenBlair123]

Could we get someone to accept workflow for CI?

[YoEight]

@StuartFergusonVme I expected you squashed your commits too 😄

You back merged on your PR branch, let me see what I can do, otherwise, I might have to create another PR to merge this.

[YoEight]

@StuartFergusonVme @StevenBlair123 I didn't manage to fix your PR so I opened a new one here: #2979