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

[azservicebus] Removing the idle tracking code. #19581

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

richardpark-msft
Copy link
Member

@richardpark-msft richardpark-msft commented Nov 16, 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).

@richardpark-msft
Copy link
Member Author

/azp run go - azservicebus

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…imeout - together they're creating a perfect storm for customers and causing hangs.

I also left in the removal of the defaultDrainTimeout private field - it's not used at all.
@richardpark-msft
Copy link
Member Author

/azp run go - azservicebus

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richardpark-msft richardpark-msft merged commit f4d16eb into Azure:main Nov 17, 2022
@richardpark-msft richardpark-msft deleted the sb-rollback-keepalive branch November 17, 2022 00:39
@yulin-li
Copy link

I want to say the 1.1.2 makes our service down because this issue. Besides this fix, could you try to avoid using a path release for these big changes?

@richardpark-msft
Copy link
Member Author

I want to say the 1.1.2 makes our service down because this issue. Besides this fix, could you try to avoid using a path release for these big changes?

Hi @yulin-li, you've got a good point about this change, although it was intended to simply complement the existing recovery code (but revealed a weak point). The next time I release this it'll go out as a minor rev bump.

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.

3 participants