Remove thread count limitation on workers #7671
Conversation
| if (string.IsNullOrEmpty(startInfo.EnvironmentVariables.GetValue(RpcWorkerConstants.PythonThreadpoolThreadCount)) && | ||
| string.Equals(functionWorkerRuntime, RpcWorkerConstants.PythonLanguageWorkerName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| startInfo.EnvironmentVariables[RpcWorkerConstants.PythonThreadpoolThreadCount] = RpcWorkerConstants.DefaultConcurrencyLimit; |
There was a problem hiding this comment.
can these be exposed in worker.config.json instead of AppSettings?
There was a problem hiding this comment.
No, these settings are user-configurable per app. They are not new, they already exist and the language workers expect them.
There was a problem hiding this comment.
The DefaultConcurrencyLimit could be a part of worker.config, but adding them doesn't add any value as they can be overwritten by the Cx.
| @@ -53,5 +53,10 @@ public static class RpcWorkerConstants | |||
| public const string DotNetExecutableName = "dotnet"; | |||
| public const string DotNetExecutableNameWithExtension = DotNetExecutableName + ".exe"; | |||
| public const string DotNetFolderName = "dotnet"; | |||
|
|
|||
| // Language worker concurrency limits | |||
| public const string PythonThreadpoolThreadCount = "PYTHON_THREADPOOL_THREAD_COUNT"; | |||
There was a problem hiding this comment.
Could you please add the link to the external documentation of both these settings - to give future context on where these things came from.
| @@ -115,5 +117,25 @@ public void DefaultWorkerProcessFactory_SanitizeExpandedArgs(string inputString, | |||
| { | |||
| return new Dictionary<string, string>() { { "rpckey1", "rpcvalue1" }, { "rpckey2", "rpcvalue2" } }; | |||
| } | |||
|
|
|||
| [Theory] | |||
| [InlineData(RpcWorkerConstants.PythonLanguageWorkerName, RpcWorkerConstants.PythonThreadpoolThreadCount, "1", "1")] | |||
There was a problem hiding this comment.
Do we need any special test for values more than 1000?
| if (string.IsNullOrEmpty(startInfo.EnvironmentVariables.GetValue(RpcWorkerConstants.PythonThreadpoolThreadCount)) && | ||
| string.Equals(functionWorkerRuntime, RpcWorkerConstants.PythonLanguageWorkerName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| startInfo.EnvironmentVariables[RpcWorkerConstants.PythonThreadpoolThreadCount] = RpcWorkerConstants.DefaultConcurrencyLimit; |
There was a problem hiding this comment.
The DefaultConcurrencyLimit could be a part of worker.config, but adding them doesn't add any value as they can be overwritten by the Cx.
Issue describing the changes in this PR
resolves #issue_for_this_pr
Pull request checklist
release_notes.mdAdditional information
Breaking change proposal:
Azure/Azure-Functions#1962
Related python change:
Azure/azure-functions-python-worker#890