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

Make EventStoreCatchUpSubscription Async #837

Merged

Conversation

samhjohnson
Copy link
Contributor

EventStoreCatchUpSubscriptions blocked when calling ReadAllEventsForwardAsync on subscribing with catch up subscriptions. This allows for asynchronous initialization of catch up subscriptions preventing blocking calls and excessive thread usage.

In addition to this it also fixes a bug when disconnects occur where events can be delivered out of order on resubscribe.

@samhjohnson
Copy link
Contributor Author

Looks like the Travis build is not working correctly.

@gregoryyoung
Copy link
Contributor

I believe its timing out on tests. @hayleyjean is looking at it.

It will take us a bit to review this though it looks reasonable on first
quick look through.

Cheers,

Greg

On Tue, Mar 8, 2016 at 12:55 PM, samhjohnson notifications@github.com
wrote:

Looks like the Travis build is not working correctly.


Reply to this email directly or view it on GitHub
#837 (comment)
.

Studying for the Turing test

@samhjohnson
Copy link
Contributor Author

Ok my tests failing now will fix up.

@pgermishuys
Copy link
Contributor

This looks good to me. @samhjohnson mind squashing those commits?

@samhjohnson
Copy link
Contributor Author

BTW have squashed commits.

@gregoryyoung
Copy link
Contributor

👍 looks reasonable and we will schedule some stress testing on it before release

gregoryyoung added a commit that referenced this pull request Mar 17, 2016
Make EventStoreCatchUpSubscription Async
@gregoryyoung gregoryyoung merged commit 39d7d26 into EventStore:release-v3.6.0 Mar 17, 2016
@gregoryyoung
Copy link
Contributor

So we will be doing some stress testing on this. I was going through the code today and wondered what will happen if eventappeared (_,x) => Thread.Sleep(5000)

In the old version of the code, this was handled here: https://github.com/EventStore/EventStore/blob/oss-v3.5.0/src/EventStore.ClientAPI/EventStoreCatchUpSubscription.cs#L272 all processing by client was specifically dispatched to another thread so a bad client couldn't screw up internal processing of the client.

hayley-jean added a commit that referenced this pull request May 20, 2016
Revert of Make EventStoreCatchUpSubscription Async #837
bartelink pushed a commit to bartelink/EventStore that referenced this pull request May 29, 2019
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.

None yet

3 participants