[azservicebus] Idle timer #19465
Merged
[azservicebus] Idle timer #19465
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
October 31, 2022 15:57
…guard against conditions where our link has expired but our client doesn't know about it. We can't repro this condition locally but it might just be that we're missing the Detach frame from the service side due to network conditions.
… transparently restart the link.
|
/azp run go - azservicebus |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jhendrixMSFT
approved these changes
Nov 1, 2022
richardpark-msft
added a commit
that referenced
this pull request
Nov 8, 2022
…alls (#19506) I added in a simple idle timer in #19465, which would expire the link if our internal message receive went longer than 5 minutes. This extends that to track it across multiple consecutive calls as well, in case the user calls and cancels multiple times in a row, eating up 5 minutes of wall-clock time. This is actually pretty similar to the workaround applied by the customer here in #18517 but tries to take into account multiple calls and also recovers the link without exiting ReceiveMessages().
richardpark-msft
added a commit
that referenced
this pull request
Nov 17, 2022
Client-side idle tracking was added in v1.1.2, but it seems be causing issues for customers and is actually making their receiving _less_ reliable. I had previously removed it for just sessions, but I'm now also removing it for non-session receivers as well. I'm leaving the idle check code, just not hooked up for now, because it still seems like a good defensive feature but it requires more testing and stressing to be fit for production. This rolls back the changes in #19465 (simple idle timer), #19492 (ignore ctx on link close) and #19506 (track idle across multiple receives).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We seem to have some customers that can reproduce an issue where the receiver is "active" but no longer receiving any messages.
Currently, our keep-alive check is only checking things at the connection level, including our keep-alive. However, if our link were to be disconnected on the service side, and we somehow missed the detach notification, there'd be no confirmation on our end that the link is actually alive since we don't re-issue credits unless something changes (and nothing will).
This change makes it so we give it a max 5 minutes of pure-idle time on the client. If we don't receive anything for that time we'll just restart the link, which is a pretty cheap operation and it will guarantee that the link is active. I've also added in some error messages specifically around this behavior, which should give us something to narrow down on in our stress testing as well.