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

[core-amqp] Make CbsClient.negotiateClaim cancellable #9988

Closed
chradek opened this issue Jul 9, 2020 · 1 comment · Fixed by #13835
Closed

[core-amqp] Make CbsClient.negotiateClaim cancellable #9988

chradek opened this issue Jul 9, 2020 · 1 comment · Fixed by #13835
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@chradek
Copy link
Contributor

chradek commented Jul 9, 2020

The CbsClient.negotiateClaim method in core-amqp is async, but doesn't currently accept an abortSignal. This means that if a connection is closed while the claim is being negotiated, the process won't exit until the operation timeout (default 60 seconds) has been reached.

This behavior can be reproduced using the sample provided in #9615.

In the sample, we close the EventHubConsumerClient, but then the process waits ~60 seconds before exiting while it waits for the timeout from the negotiateClaim call to be reached.

@chradek chradek added Client This issue points to a problem in the data-plane of the library. Azure.Core labels Jul 9, 2020
@chradek chradek added this to the Backlog milestone Jul 9, 2020
@ramya-rao-a ramya-rao-a modified the milestones: Backlog, [2020] August Jul 10, 2020
@ramya-rao-a
Copy link
Contributor

Looks like we have RequestResponseLink.sendRequest() respecting the abort signal passed to it.
What we need to do is

  • Ensure CbsClient.negotiateClaim() in core-amqp takes the abortSignal and passes it to sendRequest()
  • Update Service Bus & Event Hubs, to pass the abortSignal to CbsClient.negotiateClaim()

Then there is the matter of CbsClient.init() not being cancellable. Not sure if we want to track that here or in #4422

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants