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

[FEATURE REQ] Allow access to the session in the ServiceBusSessionProcessor #18527

Closed
ErikPilsits-RJW opened this issue Feb 8, 2021 · 14 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Milestone

Comments

@ErikPilsits-RJW
Copy link

Library or service name.
Azure.Messaging.ServiceBus

Is your feature request related to a problem? Please describe.
Allow access to the session in the ServiceBusSessionProcessor, giving the user control over when to close the session.

The feature and use cases were discussed in #16773.

#16773 (comment)

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 8, 2021
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Bus labels Feb 8, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 8, 2021
@jsquire
Copy link
Member

jsquire commented Feb 8, 2021

Thank you for your feedback. Tagging and routing to the team best able to assist.

@davidallyoung
Copy link

Hey all, wanted to add a feedback point here, our team could use this one as well to be able to let a consumer of a session using the ServiceBusSessionProcessor close the session it's currently on and move on to something else, should something come up that it knows it can't process that particular one but some other handler may be able to. (Network issues, etc).

@sfrk
Copy link

sfrk commented May 18, 2021

Our team is using sessions to ensure that messages belonging to the same session are never processed in parallel.

However, 95% of these sessions comprise only 1 message, meaning that each session will effectively idle for SessionIdleTimeout before releasing the session lock. That's a lot of idling (about 5000 seconds a day).

Allowing us to 'eagerly' close the session and release the session lock after completing each message, thus preventing the processor from automatically looking for another message on the same session that will never be there, would instantly eliminate the need to idle for single-message sessions.

Is there another way for summarily locking related messages on the service bus that I'm not aware of?

@JoshLove-msft JoshLove-msft added this to the Backlog milestone May 24, 2021
@JoshLove-msft
Copy link
Member

@sfrk there isn't another way to eagerly close the session. We would need to add support for it in the library.

@JoshLove-msft JoshLove-msft modified the milestones: Backlog, [2021] July May 24, 2021
@esbenbach
Copy link

I need this feature as well and it used to be there in the Track 0 SDK (on MessageSession).
We already have Complete/Abandon on the ProcessSessionMessageEventArgs wouldnt it make sense to expose CloseSession here as well?

@esbenbach
Copy link

Since this has been added to the July/Backlog milestones, does that mean some sort of solution is being worked on?

@JoshLove-msft
Copy link
Member

JoshLove-msft commented Jul 7, 2021

Hi @esbenbach, I'd like to add the ability to close a session in our August release (possibly as a beta release). One complication is that multiple threads may be concurrently receiving from a single session, so we will need to be careful in how we handle Closing so as to not stomp on another thread's work, and also to not require each thread to call close on the same session. My thought is that in this scenario, the close call would signal that the session should be closed once all in-flight work is completed for that session.

@epilsits
Copy link

epilsits commented Jul 7, 2021

I haven't looked at the doc in a while... In the Microsoft.Azure.ServiceBus sdk the session handler was single threaded. You could handle multiple sessions but only one concurrent message per session. Are you saying the new sdk also allows multiple concurrent calls to the session handler?

@JoshLove-msft
Copy link
Member

JoshLove-msft commented Jul 7, 2021

Are you saying the new sdk also allows multiple concurrent calls to the session handler?

Yes, via the MaxConcurrentCallsPerSession option.

@JoshLove-msft
Copy link
Member

Here is what I am thinking the behavior will be:
/// Closes the session that is being processed. If there are multiple
/// threads processing the session, due to ServiceBusSessionProcessorOptions.MaxConcurrentCallsPerSession being greater than 1,
/// the session will be marked for closing, but may not be immediately closed. When this method is called, the message that was received
/// will not be automatically completed, even if ServiceBusSessionProcessorOptions.AutoCompleteMessages is set to true,
/// (or abandoned in the case of an exception). In order to settle the message, the message settlement methods
/// should be used before calling this method.

@esbenbach
Copy link

@JoshLove-msft the description sounds reasonable.
Say that I can live with the fact that the messages are not settled (completed/abandoned) until the last thread is complete, would it be auto-completed then? Or will the fact that we are explicitly closing the session require we settle messages always.

Its not really an issue, just that the description leaves me in doubt of how it works.

@epilsits
Copy link

epilsits commented Jul 7, 2021

It doesn't sound like all threads completing would have anything to do with message settlement. However it's not clear what happens in the other threads that didn't close the session. Do they also have to manually settle the message? If so then a user would need to implement some kind of sync with a session status flag maybe (?) so each thread knows how to settle the message.

What is the purpose of disabling auto settlement in this close session scenario?

@JoshLove-msft
Copy link
Member

JoshLove-msft commented Jul 7, 2021

These are great questions and perhaps the behavior becomes confusing when disabling the auto completion with MaxConcurrentCallsPerSession > 1. The scenario I was thinking about is where the user receives a message that they do not want to autocomplete and they want to close the session. I suppose this scenario isn't really specific to closing sessions. Users can always abandon the message themselves in this case.

The other thing is that the semantics are a bit awkward when the CloseSession method does not actually close the session at that time. If we wanted to still allow autocompletion, we would have to have the method (or property maybe?) act as a signal to the processor that the session is marked for closing, rather than actually close the session at the time the method (or property) is called.

@ErikPilsits-RJW
Copy link
Author

I think the property / method in a multi message session handler would be acceptable. It should prevent the message pump from getting additional messages from the session and allow in-flight handlers to complete. When they have all completed it would close the session and acquire a new one.

I don't think it should change the behavior of auto completion. As you say, abandoning the message would remain an option, and would be the typical method when auto complete is enabled anyway.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Projects
None yet
Development

No branches or pull requests

8 participants
@jsquire @esbenbach @davidallyoung @epilsits @JoshLove-msft @ErikPilsits-RJW @sfrk and others