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

[service-bus] Implement handling for subscribe() after closing a subscription. #9829

Closed
richardpark-msft opened this issue Jul 1, 2020 · 0 comments
Assignees
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Service Bus
Milestone

Comments

@richardpark-msft
Copy link
Member

As part of #9802 we now allow the user to close() the result of a subscribe().

This introduces a new partially-closed state that we didn't have before:

Old:

const receiver: Receiver;
receiver.subscribe();
// no way to stop the subscription except....
await receiver.close();

Now:

const receiver: Receiver;
const subscription = receiver.subscribe();

await subscription.close();     // does not close the receiver link, but does drain the receiver

// hey, can I subscribe again?
// receiver.subscribe() throws an error here about only having one active subscription at a time per receiver.

await receiver.close();   // actually closes the link

The issue that @ramya-rao-a has brought up is that the user might then be tempted to just .subscribe() again (after all, they've 'closed' the other subscription). This does not work today because our old check logic assumed that the user could only have one active subscribe at a time.

A couple of options:

  1. Eliminate the restriction on the user having only one subscribe() at a time. It follows logically that the user can just subscribe() and subscription.close() whenever they want. We can still track all outstanding links associated with the receiver and just shut them down (ie, 1 is the same as 'n' from the internals).
  2. Carve out a hole so a user can call subscribe() again so long as the old subscription was close()'d. This would still enforce the "only one active subscription at a time" restriction if we think that's necessary.

#1 is my preference - although there is some possible utility is trying to limit users to creating only a single instance each of a batching/streaming receiver there isn't any particular reason to enforce it (users can get the same behavior by just calling it once).

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 1, 2020
@richardpark-msft richardpark-msft added Client This issue points to a problem in the data-plane of the library. Service Bus labels Jul 1, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 1, 2020
@richardpark-msft richardpark-msft added this to the [2020] August milestone Jul 1, 2020
@ramya-rao-a ramya-rao-a added the blocking-release Blocks release label Jul 14, 2020
richardpark-msft added a commit that referenced this issue Jul 28, 2020
As part of the ongoing enhancements to Track 2 we've been running into issues where code is getting duplicated between session and non-session (or worse, is drifting apart). 

This PR is first in a series to unify the implementations, starting with unifying BatchingReceiver and MessageSession to the same body of code.

This is tangentially related to #9829, which will get the same treatment but for StreamingReceiver and MessageSession.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Service Bus
Projects
None yet
Development

No branches or pull requests

2 participants