Startup improvements for http function invocations

mathewc · mathewc · commit 1e87ddd49a19 · 2016-11-07T16:20:14.000-08:00
diff --git a/src/WebJobs.Script.WebHost/App_Start/WebApiConfig.cs b/src/WebJobs.Script.WebHost/App_Start/WebApiConfig.cs
@@ -2,9 +2,7 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.IO;
-using System.Threading.Tasks;
 using System.Web;
 using System.Web.Http;
 using Autofac;
diff --git a/src/WebJobs.Script.WebHost/Controllers/HomeController.cs b/src/WebJobs.Script.WebHost/Controllers/HomeController.cs
@@ -4,7 +4,6 @@
 using System;
 using System.Net;
 using System.Net.Http;
-using System.Net.Http.Formatting;
 using System.Text;
 using System.Web.Http;
 using Microsoft.Azure.WebJobs.Script.WebHost.Properties;
diff --git a/src/WebJobs.Script.WebHost/Handlers/WebScriptHostHandler.cs b/src/WebJobs.Script.WebHost/Handlers/WebScriptHostHandler.cs
@@ -49,15 +49,16 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                 // initialize. This might happen if http requests come in while the
                 // host is starting up for the first time, or if it is restarting.
                 TimeSpan timeWaited = TimeSpan.Zero;
-                while (!scriptHostManager.IsRunning && scriptHostManager.LastError == null && (timeWaited < _hostTimeoutSeconds))
+                while (!scriptHostManager.CanInvoke() && 
+                        scriptHostManager.State != ScriptHostState.Error && 
+                        (timeWaited < _hostTimeoutSeconds))
                 {
                     await Task.Delay(_hostRunningPollIntervalMs);
                     timeWaited += TimeSpan.FromMilliseconds(_hostRunningPollIntervalMs);
                 }
 
-                // if the host is not running after our wait time has expired
-                // return a 503
-                if (!scriptHostManager.IsRunning)
+                // if the host is not ready after our wait time has expired return a 503
+                if (!scriptHostManager.CanInvoke())
                 {
                     return new HttpResponseMessage(HttpStatusCode.ServiceUnavailable)
                     {
diff --git a/src/WebJobs.Script.WebHost/WebScriptHostManager.cs b/src/WebJobs.Script.WebHost/WebScriptHostManager.cs
@@ -392,16 +392,21 @@ protected override void OnInitializeConfig(ScriptHostConfiguration config)
             hostConfig.DashboardConnectionString = null; // disable slow logging
         }
 
-        protected override void OnHostStarted()
+        protected override void OnHostCreated()
         {
-            base.OnHostStarted();
-
             // whenever the host is created (or recreated) we build a cache map of
             // all http function routes
             InitializeHttpFunctions(Instance.Functions);
 
+            base.OnHostCreated();
+        }
+
+        protected override void OnHostStarted()
+        {
             // Purge any old Function secrets
             _secretManager.PurgeOldFiles(Instance.ScriptConfig.RootScriptPath, Instance.TraceWriter);
+
+            base.OnHostStarted();
         }
 
         internal void InitializeHttpFunctions(IEnumerable<FunctionDescriptor> functions)
diff --git a/src/WebJobs.Script/Host/ScriptHostManager.cs b/src/WebJobs.Script/Host/ScriptHostManager.cs
@@ -50,12 +50,6 @@ public ScriptHostManager(ScriptHostConfiguration config, ScriptSettingsManager s
             _scriptHostFactory = scriptHostFactory;
         }
 
-        /// <summary>
-        /// Returns true if the <see cref="ScriptHost"/> is up and running and ready to
-        /// process requests.
-        /// </summary>
-        public virtual bool IsRunning { get; private set; }
-
         public virtual ScriptHost Instance
         {
             get
@@ -65,10 +59,28 @@ public virtual ScriptHost Instance
         }
 
         /// <summary>
-        /// Gets the last host <see cref="Exception"/> that has occurred.
+        /// Gets or sets the current state of the host.
+        /// </summary>
+        public virtual ScriptHostState State { get; private set; }
+
+        /// <summary>
+        /// Gets or sets the last host <see cref="Exception"/> that has occurred.
         /// </summary>
         public virtual Exception LastError { get; private set; }
 
+        /// <summary>
+        /// Returns a value indicating whether the host can accept function invoke requests.
+        /// </summary>
+        /// <remarks>
+        /// The host doesn't have to be fully started for it to allow direct function invocations
+        /// to be processed.
+        /// </remarks>
+        /// <returns>True if the host can accept invoke requests, false otherwise.</returns>
+        public bool CanInvoke()
+        {
+            return State == ScriptHostState.Created || State == ScriptHostState.Running;
+        }
+
         public void RunAndBlock(CancellationToken cancellationToken = default(CancellationToken))
         {
             // Start the host and restart it if requested. Host Restarts will happen when
@@ -78,7 +90,12 @@ public virtual ScriptHost Instance
                 ScriptHost newInstance = null;
                 try
                 {
-                    IsRunning = false;
+                    // if we were in an error state retain that,
+                    // otherwise move to default
+                    if (State != ScriptHostState.Error)
+                    {
+                        State = ScriptHostState.Default;
+                    }
 
                     // Create a new host config, but keep the host id from existing one
                     _config.HostConfig = new JobHostConfiguration
@@ -87,9 +104,16 @@ public virtual ScriptHost Instance
                     };
                     OnInitializeConfig(_config);
                     newInstance = _scriptHostFactory.Create(_settingsManager, _config);
-
                     _traceWriter = newInstance.TraceWriter;
 
+                    _currentInstance = newInstance;
+                    lock (_liveInstances)
+                    {
+                        _liveInstances.Add(newInstance);
+                    }
+
+                    OnHostCreated();
+
                     if (_traceWriter != null)
                     {
                         string message = string.Format("Starting Host (HostId={0}, Version={1}, ProcessId={2}, Debug={3})",
@@ -101,15 +125,11 @@ public virtual ScriptHost Instance
                     // log any function initialization errors
                     LogErrors(newInstance);
 
-                    lock (_liveInstances)
-                    {
-                        _liveInstances.Add(newInstance);
-                    }
-                    _currentInstance = newInstance;
                     OnHostStarted();
 
-                    // only after ALL initialization is complete do we set this flag
-                    IsRunning = true;
+                    // only after ALL initialization is complete do we set the
+                    // state to Running
+                    State = ScriptHostState.Running;
                     LastError = null;
 
                     // Wait for a restart signal. This event will automatically reset.
@@ -131,7 +151,7 @@ public virtual ScriptHost Instance
                 }
                 catch (Exception ex)
                 {
-                    IsRunning = false;
+                    State = ScriptHostState.Error;
                     LastError = ex;
 
                     // We need to keep the host running, so we catch and log any errors
@@ -221,15 +241,10 @@ public async Task StopAsync()
                 _stopEvent.Set();
                 ScriptHost[] instances = GetLiveInstancesAndClear();
 
-                Task[] tasksStop = Array.ConvertAll(instances, instance => instance.StopAsync());
+                Task[] tasksStop = Array.ConvertAll(instances, p => StopAndDisposeAsync(p));
                 await Task.WhenAll(tasksStop);
 
-                foreach (var instance in instances)
-                {
-                    instance.Dispose();
-                }
-
-                IsRunning = false;
+                State = ScriptHostState.Default;
             }
             catch
             {
@@ -242,6 +257,22 @@ public void Stop()
             StopAsync().GetAwaiter().GetResult();
         }
 
+        private static async Task StopAndDisposeAsync(ScriptHost instance)
+        {
+            try
+            {
+                await instance.StopAsync();
+            }
+            catch
+            {
+                // best effort
+            }
+            finally
+            {
+                instance.Dispose();
+            }
+        }
+
         private ScriptHost[] GetLiveInstancesAndClear()
         {
             ScriptHost[] instances;
@@ -258,6 +289,11 @@ protected virtual void OnInitializeConfig(ScriptHostConfiguration config)
         {
         }
 
+        protected virtual void OnHostCreated()
+        {
+            State = ScriptHostState.Created;
+        }
+
         protected virtual void OnHostStarted()
         {
             var metricsLogger = _config.HostConfig.GetService<IMetricsLogger>();
diff --git a/src/WebJobs.Script/Host/ScriptHostState.cs b/src/WebJobs.Script/Host/ScriptHostState.cs
@@ -0,0 +1,26 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+namespace Microsoft.Azure.WebJobs.Script
+{
+    public enum ScriptHostState
+    {
+        /// <summary>
+        /// The host has not yet been created
+        /// </summary>
+        Default,
+        /// <summary>
+        /// The host has been created and can accept direct function
+        /// invocations, but listeners are not yet started.
+        /// </summary>
+        Created,
+        /// <summary>
+        /// The host is fully running.
+        /// </summary>
+        Running,
+        /// <summary>
+        /// The host is in an error state
+        /// </summary>
+        Error
+    }
+}
diff --git a/src/WebJobs.Script/WebJobs.Script.csproj b/src/WebJobs.Script/WebJobs.Script.csproj
@@ -430,6 +430,7 @@
     <Compile Include="DictionaryJsonConverter.cs" />
     <Compile Include="Extensions\ActionExtensions.cs" />
     <Compile Include="Diagnostics\FileTraceWriter.cs" />
+    <Compile Include="Host\ScriptHostState.cs" />
     <Compile Include="HttpMethodJsonConverter.cs" />
     <Compile Include="Host\IScriptHostFactory.cs" />
     <Compile Include="Diagnostics\NullTraceWriter.cs" />
diff --git a/test/WebJobs.Script.Tests/EndToEndTimeoutTests.cs b/test/WebJobs.Script.Tests/EndToEndTimeoutTests.cs
@@ -159,7 +159,7 @@ private async Task<MockScriptHostManager> CreateAndStartScriptHostManager(string
 
             var scriptHostManager = new MockScriptHostManager(config);
             ThreadPool.QueueUserWorkItem((s) => scriptHostManager.RunAndBlock());
-            await TestHelpers.Await(() => scriptHostManager.IsRunning);
+            await TestHelpers.Await(() => scriptHostManager.State == ScriptHostState.Running);
 
             return scriptHostManager;
         }
diff --git a/test/WebJobs.Script.Tests/FunctionGeneratorTests.cs b/test/WebJobs.Script.Tests/FunctionGeneratorTests.cs
@@ -241,7 +241,7 @@ internal async Task GeneratedMethods_WithOutParams_DoNotCauseDeadlocks(string fi
 
                 await TestHelpers.Await(() =>
                 {
-                    return manager.IsRunning;
+                    return manager.State == ScriptHostState.Running;
                 });
 
                 var request = new HttpRequestMessage(HttpMethod.Get, String.Format("http://localhost/api/httptrigger-{0}", fixture));
diff --git a/test/WebJobs.Script.Tests/ScriptHostManagerTests.cs b/test/WebJobs.Script.Tests/ScriptHostManagerTests.cs
@@ -46,7 +46,7 @@ public async Task UpdateFileAndRestart()
                 Thread t = new Thread(_ =>
                    {
                        // don't start until the manager is running
-                       TestHelpers.Await(() => manager.IsRunning).Wait();
+                       TestHelpers.Await(() => manager.State == ScriptHostState.Running).Wait();
 
                        try
                        {
@@ -128,7 +128,8 @@ public async Task RunAndBlock_SetsLastError_WhenExceptionIsThrown()
             // we expect a host exception immediately
             await Task.Delay(2000);
 
-            Assert.False(hostManager.IsRunning);
+            Assert.Equal(ScriptHostState.Error, hostManager.State);
+            Assert.False(hostManager.CanInvoke());
             Assert.NotNull(hostManager.LastError);
             Assert.Equal("Kaboom!", hostManager.LastError.Message);
 
@@ -137,10 +138,12 @@ public async Task RunAndBlock_SetsLastError_WhenExceptionIsThrown()
             scriptHostFactory.Throw = false;
             await TestHelpers.Await(() =>
             {
-                return hostManager.IsRunning;
+                return hostManager.State == ScriptHostState.Running;
             });
 
             Assert.Null(hostManager.LastError);
+            Assert.True(hostManager.CanInvoke());
+            Assert.Equal(ScriptHostState.Running, hostManager.State);
         }
 
         [Fact]
@@ -168,10 +171,10 @@ public async Task EmptyHost_StartsSuccessfully()
 
             Task runTask = Task.Run(() => hostManager.RunAndBlock());
 
-            await TestHelpers.Await(() => hostManager.IsRunning, timeout: 10000);
+            await TestHelpers.Await(() => hostManager.State == ScriptHostState.Running, timeout: 10000);
 
             hostManager.Stop();
-            Assert.False(hostManager.IsRunning);
+            Assert.Equal(ScriptHostState.Default, hostManager.State);
 
             await Task.Delay(FileTraceWriter.LogFlushIntervalMs);
 
diff --git a/test/WebJobs.Script.Tests/WebScriptHostHandlerTests.cs b/test/WebJobs.Script.Tests/WebScriptHostHandlerTests.cs
@@ -42,7 +42,7 @@ public WebScriptHostHandlerTests()
         [Fact]
         public async Task SendAsync_HostNotRunning_Returns503()
         {
-            _managerMock.SetupGet(p => p.IsRunning).Returns(false);
+            _managerMock.SetupGet(p => p.State).Returns(ScriptHostState.Default);
             _managerMock.SetupGet(p => p.LastError).Returns((Exception)null);
 
             HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://functions.test.com/api/test");
@@ -53,7 +53,7 @@ public async Task SendAsync_HostNotRunning_Returns503()
         [Fact]
         public async Task SendAsync_HostRunning_ReturnsOk()
         {
-            _managerMock.SetupGet(p => p.IsRunning).Returns(true);
+            _managerMock.SetupGet(p => p.State).Returns(ScriptHostState.Running);
 
             HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://functions.test.com/api/test");
             HttpResponseMessage response = await _invoker.SendAsync(request, CancellationToken.None);
@@ -63,13 +63,13 @@ public async Task SendAsync_HostRunning_ReturnsOk()
         [Fact]
         public async Task SendAsync_HostInErrorState_Returns503Immediately()
         {
-            _managerMock.SetupGet(p => p.IsRunning).Returns(false);
+            _managerMock.SetupGet(p => p.State).Returns(ScriptHostState.Error);
             _managerMock.SetupGet(p => p.LastError).Returns(new Exception());
 
             HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://functions.test.com/api/test");
             HttpResponseMessage response = await _invoker.SendAsync(request, CancellationToken.None);
             Assert.Equal(HttpStatusCode.ServiceUnavailable, response.StatusCode);
-            _managerMock.VerifyGet(p => p.IsRunning, Times.Exactly(2));
+            _managerMock.VerifyGet(p => p.State, Times.Exactly(5));
         }
 
         public class TestHandler : DelegatingHandler
diff --git a/test/WebJobs.Script.Tests/WebScriptHostManagerTests.cs b/test/WebJobs.Script.Tests/WebScriptHostManagerTests.cs

Original file line number	Diff line number	Diff line change
`@@ -49,15 +49,16 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage`
`49`	`49`	`// initialize. This might happen if http requests come in while the`
`50`	`50`	`// host is starting up for the first time, or if it is restarting.`
`51`	`51`	`TimeSpan timeWaited = TimeSpan.Zero;`
`52`		`- while (!scriptHostManager.IsRunning && scriptHostManager.LastError == null && (timeWaited < _hostTimeoutSeconds))`
	`52`	`+ while (!scriptHostManager.CanInvoke() &&`
	`53`	`+ scriptHostManager.State != ScriptHostState.Error &&`
	`54`	`+ (timeWaited < _hostTimeoutSeconds))`
`53`	`55`	`{`
`54`	`56`	`await Task.Delay(_hostRunningPollIntervalMs);`
`55`	`57`	`timeWaited += TimeSpan.FromMilliseconds(_hostRunningPollIntervalMs);`
`56`	`58`	`}`
`57`	`59`
`58`		`- // if the host is not running after our wait time has expired`
`59`		`- // return a 503`
`60`		`- if (!scriptHostManager.IsRunning)`
	`60`	`+ // if the host is not ready after our wait time has expired return a 503`
	`61`	`+ if (!scriptHostManager.CanInvoke())`
`61`	`62`	`{`
`62`	`63`	`return new HttpResponseMessage(HttpStatusCode.ServiceUnavailable)`
`63`	`64`	`{`
Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ private async Task<MockScriptHostManager> CreateAndStartScriptHostManager(string`
`159`	`159`
`160`	`160`	`var scriptHostManager = new MockScriptHostManager(config);`
`161`	`161`	`ThreadPool.QueueUserWorkItem((s) => scriptHostManager.RunAndBlock());`
`162`		`- await TestHelpers.Await(() => scriptHostManager.IsRunning);`
	`162`	`+ await TestHelpers.Await(() => scriptHostManager.State == ScriptHostState.Running);`
`163`	`163`
`164`	`164`	`return scriptHostManager;`
`165`	`165`	`}`
Original file line number	Diff line number	Diff line change
`@@ -241,7 +241,7 @@ internal async Task GeneratedMethods_WithOutParams_DoNotCauseDeadlocks(string fi`
`241`	`241`
`242`	`242`	`await TestHelpers.Await(() =>`
`243`	`243`	`{`
`244`		`- return manager.IsRunning;`
	`244`	`+ return manager.State == ScriptHostState.Running;`
`245`	`245`	`});`
`246`	`246`
`247`	`247`	`var request = new HttpRequestMessage(HttpMethod.Get, String.Format("http://localhost/api/httptrigger-{0}", fixture));`