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
Stream consumers on lost clients #1669
Stream consumers on lost clients #1669
Conversation
@@ -501,7 +501,7 @@ internal void TryForwardRequest(Message message, ActivationAddress oldAddress, A | |||
} | |||
catch (Exception ex) | |||
{ | |||
if (!(ex.GetBaseException() is KeyNotFoundException)) | |||
if (!(ex.GetBaseException() is KeyNotFoundException) && !(ex.GetBaseException() is ClientNotAvailableException)) |
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.
lets use bool doNotLog
instead, for readability.
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.
ShouldLogError? to avoid double negative?
Jason, I thought more about it during the week, and it does not make sense to me that Concurrent Dictionary will throw modification exception if mutated while iteration. It is supposed to allow concurrent modification from another thread, how come it will disallow when it comes from the same thread? |
Tested repeatedly. Seems ok without the ToList(). Also, reading up on concurrent dictionary seems to confirm your expectations. |
ac1626a
to
b2c2e33
Compare
Re concurrent dictionary: great, that simplifies a lot! |
b2c2e33
to
b923d49
Compare
@dotnet-bot test this please |
Added basic tests demonstrating issues with losing clients that are consuming from streams.
Fixed sms and azure queue and event hub stream providers to handle delivery failures to loss clients.
Azure queue test is disabled as it does not work well yet due to issues unrelated to this PR or client stream consumers.