Adding a timeout to IFunctionProvider.GetFunctionMetadataAsync
#10249
Conversation
…t if a provider does not return, it will not cause a deadlock state.
kshyju
changed the title
IFunctionProvider.GetFunctionMetadataAsync
…h can be set to a different value than default, from the tests.
| @@ -17,3 +17,4 @@ | |||
| - Ordered invocations are now the default (#10201) | |||
| - Skip worker description if none of the profile conditions are met (#9932) | |||
| - Fixed incorrect function count in the log message.(#10220) | |||
| - Adding a timeout to `GetFunctionMetadataAsync` to prevent deadlocks (#10219) | |||
There was a problem hiding this comment.
Can we update the note here so it's more end user friendly (i.e., doesn't require understanding of the codebase). Something along the lines of Adding a timeout when retrieving function metadata from metadata providers
| if (getFunctionMetadataFromProviderTask.IsFaulted) | ||
| { | ||
| // Task completed but with an error | ||
| _logger.LogWarning($"Failure in retrieving metadata from '{functionProvider.GetType().FullName}': {getFunctionMetadataFromProviderTask.Exception?.Flatten().ToString()}"); |
There was a problem hiding this comment.
Pass the values as arguments into your log call:
Example:
_logger.LogWarning("Failure in retrieving metadata from '{typeName}': {exception}", functionProvider.GetType().FullName, getFunctionMetadataFromProviderTask.Exception?.Flatten());There was a problem hiding this comment.
Also, should this be an error instead?
| } | ||
| } | ||
| // Timeout case. getFunctionMetadataFromProviderTask was not the one that completed | ||
| _logger.LogWarning($"Timeout or failure in retrieving metadata from '{functionProvider.GetType().FullName}'."); |
There was a problem hiding this comment.
Same comment about logging arguments.
There was a problem hiding this comment.
Also, should this be an error?
| { | ||
| // Task completed but with an error | ||
| _logger.LogWarning($"Failure in retrieving metadata from '{functionProvider.GetType().FullName}': {getFunctionMetadataFromProviderTask.Exception?.Flatten().ToString()}"); | ||
| return ImmutableArray<FunctionMetadata>.Empty; |
There was a problem hiding this comment.
Was the behavior difference considered here? Prior to this change, this would throw an exception that would bubble up in the metadata loading path (failing initialization). This will now log and move on, initializing with an empty list of functions.
| var getFunctionMetadataFromProviderTask = functionProvider.GetFunctionMetadataAsync(); | ||
| var delayTask = Task.Delay(TimeSpan.FromSeconds(MetadataProviderTimeoutInSeconds)); | ||
|
|
||
| var completedTask = Task.WhenAny(getFunctionMetadataFromProviderTask, delayTask).ContinueWith(t => |
There was a problem hiding this comment.
It's a bit painful that this lives in WebJobs.Script (and that it targets .NET standard). WaitAsync would be a cleaner approach.
resolves #10219
The fix is to wrap the
GetFunctionMetadataAsyncwith a timeout task so that if theGetFunctionMetadataAsyncdoes not return within a specific time, we return an empty array for that provider and log a message.IMPORTANT: Currently, changes must be backported to the
in-procbranch to be included in Core Tools and non-Flex deployments.in-procbranch is not requiredrelease_notes.md