Azure / azure-webjobs-sdk

Azure WebJobs SDK
MIT License
737 stars 358 forks source link

ServiceBusListener is returning immediately when StopAsync is called. #892

Open Thnslj opened 7 years ago

Thnslj commented 7 years ago

I have implemented a draining flag to allow current Function runs to complete before stopping the process. When the flag is set i call the cancellationTokenSource.Cancel() and then calls jobHost.Stop().

jobHost.StartAsync(cancellationTokenSource.Token).GetAwaiter().GetResult();                
while (!settingsService.GetSettings().DisableWebJobs)
{
   Thread.Sleep(TimeSpan.FromSeconds(4));
}
cancellationTokenSource.Cancel();
jobHost.Stop();

Expected behavior

jobHost.Stop(); Should wait for functions started by ServiceBusListener to complete. Just like TaskSeriesTimer is implemented.

From TaskSeriesTimer.cs

   private async Task StopAsyncCore(CancellationToken cancellationToken)
        {
            await Task.Delay(0);
            TaskCompletionSource<object> cancellationTaskSource = new TaskCompletionSource<object>();

            using (cancellationToken.Register(() => cancellationTaskSource.SetCanceled()))
            {
                // Wait for all pending command tasks to complete (or cancellation of the token) before returning.
                await Task.WhenAny(_run, cancellationTaskSource.Task);
            }

            _stopped = true;
        }

Actual behavior

jobHost.Stop(); Returns immediately

Related information

Package version: <package id="Microsoft.Azure.WebJobs" version="2.0.0-beta1-10456" targetFramework="net461" />

brettsam commented 7 years ago

It looks like our Listeners are inconsistent wrt whether they allow in-process invocations to complete when StopAsync() is called. Some examples:

@MikeStall / @mathewc / @davidebbo -- is there a 'best practice' for how Listeners should behave in this situation? It seems like waiting for current executions to complete is desirable. Should we try to get all Listeners to behave that way?

The drawback is that it's possible that a call to StopAsync() never returns -- or takes a very long time.

MikeStall commented 7 years ago

It'd be good for listeners to behave consistently. Even better if we can structure the extensibility model such that consistency is enforced. There's a few things to consider:

  1. Once StopAsync() is called. When is it's task signaled?
  2. allowing already-running functions to run to completion. Nice for short functions.
  3. soft exit: firing a cancellation token to give a long-running functions a chance to softly abort
  4. hard exit: forcibly killing long-running functions. In some cases, once the listener (or other "keep alive" tasks stops), there's no point in the function continuing to run since it may not be able to save its results.