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] Validate that credits are within limits (not over or under) #19992

Merged
merged 11 commits into from
Feb 14, 2023

Conversation

richardpark-msft
Copy link
Member

Fixing issues where we could over-request credit (#19965) or allow for negative/zero credits (#19743), both of which could cause issues with go-amqp..

Fixes #19965
Fixes #19743

Richard Park added 4 commits February 9, 2023 16:15
- <= 0 is bad since it can hang and we never intend to subtract credit.
- Don't allow asking for more than the actual credit ceiling of the link, which is set, statically, when the link is created.
…hims of the go-scheduler and the speed of our test.
@richardpark-msft richardpark-msft marked this pull request as ready for review February 10, 2023 00:33
@richardpark-msft
Copy link
Member Author

/azp run go - azservicebus

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Richard Park added 2 commits February 9, 2023 16:45
@richardpark-msft
Copy link
Member Author

/azp run go - azservicebus

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@richardpark-msft
Copy link
Member Author

/azp run go - azservicebus

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Richard Park added 2 commits February 9, 2023 19:09
…rable) to be able to get logs multiple times, siphoning off whatever is in the channel at that point.

This also eliminates another silly problem I have sometimes where the channel is closed early (because I extract logs) but the receiver or sender is still doing some background work which generates log information.
@richardpark-msft
Copy link
Member Author

/azp run go - azservicebus

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jhendrixMSFT
Copy link
Member

jhendrixMSFT commented Feb 14, 2023

Once we get the underlying issue in go-amqp fixed we should be able to back this out.

@richardpark-msft
Copy link
Member Author

richardpark-msft commented Feb 14, 2023

Once we get the underlying issue in go-amqp fixed we should be able to back this out.

Yeah, infinite credits and infinite sinks will be a nice touch. :)
(caveat that we'll obviously still keep the negative/0 credit checking as we don't support clawback of credits)

@richardpark-msft richardpark-msft merged commit 5523511 into Azure:main Feb 14, 2023
@richardpark-msft richardpark-msft deleted the sb-overissue-credit branch February 14, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants