Skip to content
This repository has been archived by the owner on Jul 19, 2024. It is now read-only.

Graceful shutdown (message draining) #102

Merged
merged 13 commits into from
Sep 22, 2020
Merged

Graceful shutdown (message draining) #102

merged 13 commits into from
Sep 22, 2020

Conversation

sidkri
Copy link
Member

@sidkri sidkri commented Aug 11, 2020

Azure functions will support draining in-flight messages before shutdown of the runtime. The current extension implementation closes the connection to Service Bus during shutdown before allowing executing function invocations to complete processing and prevents these messages from being completed or abandoned. Additionally, new messages start processing up until the connection to Service Bus is closed.

This PR implements graceful shutdown in the extension by leveraging a new set of "Unregister" methods in the Service Bus SDK : Azure/azure-sdk-for-net#14021 and published in 4.2.0 and 5.0.0 of the Service Bus SDK used by this library.

This extension supports single message as well as batch processing and the message draining implementation is different for each path.

Batch processing:
The ServiceBusListener when created in batch mode, polls Service Bus for messages in a loop while checking to ensure that it is still in started state and no cancellation has been requested. The new Functions runtime "/drain" API will be invoked when the runtime needs to shutdown and sufficient time will be given for any functions that have already been invoked to complete. The invocation of "/drain" calls StopAsync() on all listeners and with this change, the connections to Service Bus will remain open until the runtime is shutdown and Dispose() is called.

Single message processing:
The new QueueClient.UnregisterSessionHandlerAsync, SubscriptionClient.UnregisterSessionHandlerAsync and MessageReceiver.UnregisterMessageHandlerAsync methods are called when the extension is stopped. These calls block until all in-flight messages complete processing (drain) and new messages no longer get dispatched. As the amount of time given to functions to execute depends on the SKU, a MaxValue timespan is specified when draining as the host will shut down regardless after the max function execution time.

Other changes:

  1. Addressed Connection is closed immediately when StopAsync is called on IListener  #95
  2. Support for logging to test output via ITestOutputHelper in both Integration test classes.
  3. All test settings are exported to environment variables as WebJobs relies solely on environment variables. This obviates the need to set environment variables at the operating system level when debugging tests locally in Visual Studio.
  4. Stopping and disposing the host object in each test consistently.

@sidkri sidkri requested a review from brettsam August 11, 2020 09:19
@sidkri sidkri marked this pull request as ready for review August 21, 2020 00:22
@sidkri sidkri linked an issue Aug 21, 2020 that may be closed by this pull request
@sidkri sidkri changed the title Graceful shutdown (message draining) for batch triggers Graceful shutdown (message draining) Sep 15, 2020
@sidkri sidkri requested review from alrod and soninaren September 17, 2020 21:50
Copy link
Member

@soninaren soninaren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit comment. Looks good otherwise.

if (_clientEntity != null)
{
if (_clientEntity is QueueClient queueClient)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: last 2 if statements can be merged

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean lines 140 and 142? If so, I don't want lines 148 and 149 executed on a null _clientEntity so can't check in a single if statement. Would definitely like to compact this if chain!

Copy link
Member

@soninaren soninaren Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. You probably can make it easier to read by checking for negation instead. Something like and so on.

if (!_singleDispatch)
{
 return:
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have to set the _started flag to false in all cases. Would have to duplicate that logic in two places with the short circuit of (!_singleDispatch) so not sure if that's a better option as more common steps may be added at a later time and those would have to be duplicated.

@sidkri sidkri added this to the Functions Sprint 85 milestone Sep 19, 2020
@sidkri sidkri merged commit 6da7fe7 into dev Sep 22, 2020
@sidkri sidkri deleted the sidkri/drain-mode branch September 23, 2020 03:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants