Handling env reload response from native placeholder for failure case. by kshyju · Pull Request #9602 · Azure/azure-functions-host

kshyju · 2023-10-10T20:58:40Z

If customer payload for dotnet isolated app is using an older version of worker package (which does not have the support to handle environment reload request via native placeholder, currently the request will crash. We recently updated the NativePlaceholder to handle this case and return a response for the env reload request. That version of Native placeholder is included in this PR. We also updated the code to handle the new response. If the failure is caused due to customer payload having an older version of package, then we will execute our fallback logic, which is to shutdown the placeholder channel and start a new channel where we will load the customer assembly directly using dotnet.exe.

resolves #9606

Pull request checklist

My changes do not require documentation changes
- Otherwise: Documentation issue linked to PR
My changes should not be added to the release notes for the next release
- Otherwise: I've added my notes to release_notes.md
My changes do not need to be backported to a previous version
- Otherwise: Backport tracked by issue/PR #issue_or_pr
My changes do not require diagnostic events changes
- Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

…etCore package.

#9602) * Handling env reload response from native placeholder for failure case. * Almost working except one test needs cleanup * Cleanup * Cleanup * Fixing Worker.Extensions.Http package version to align with Http.AspNetCore package. * Logging error as it is received from the worker. * DotNetIsolatedNativeHost package to 1.0.2 * Inlining a variable (Reverting to previous version)

safihamid · 2023-10-12T21:39:29Z

    private bool UsePlaceholderChannel(IRpcWorkerChannel channel)

@kshyju remind me again, do we need to shutdown the channel immediately or can wait like 5 sec? trying to see if this will have negative cold start impact on existing apps once we switch to opt out only

Refers to: src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs:160 in 42e4df3. [](commit_id = 42e4df3, deletion_comment = False)

* Updating patch version * Sending command line args with functions- prefix to prevent conflicts (#9514) * Sending a second copy of command line args with "FUNCTIONS_" prefix * Cleanup * Sending a second copy of command line args with "FUNCTIONS_" prefix * switched to functions- prefix * Removed duplicate port from functions_uri * Switching to kebab case for the new args * Update test * Update native placeholder package to handle new args * Update release notes. * Limit dotnet-isolated specialization to 64 bit host process (#9553) * Skipping native placeholder specialization if host is not 64 bit process * formatting linting fix * Stylecop fix. * fix indentation * Adding E2E test. * Improving log message. * Switch from LogDebug to LogInformation * Fix tests to reflect logging changes * Logging with EventId * Logging using an event name * Updated release notes after rebase merge * Handling env reload response from native placeholder for failure case. (#9602) * Handling env reload response from native placeholder for failure case. * Almost working except one test needs cleanup * Cleanup * Cleanup * Fixing Worker.Extensions.Http package version to align with Http.AspNetCore package. * Logging error as it is received from the worker. * DotNetIsolatedNativeHost package to 1.0.2 * Inlining a variable (Reverting to previous version) * Updating release notes for 4.27.5 * Update PowerShell Language Workers 7.0 (to version 4.0.2973), 7.2 (to version 4.0.2974), and 7.4 (to version 4.0.2975) (#9528) * Upgrade PowerShell language worker 7.4 to 4.0.2975 * Upgrade PowerShell language worker 7.2 to 4.0.2974 * Upgrade PowerShell language worker 7.0 to 4.0.2973 * Update release notes * Update Java Worker Version to 2.13.0 (#9544) * Update Java Worker Version to 2.13.0 * Update release_notes.md for Java worker --------- Co-authored-by: AzureFunctionsJava <funcdisc@microsoft.com> --------- Co-authored-by: azfuncgh <azfuncgh@github.com> Co-authored-by: Shyju Krishnankutty <connectshyju@gmail.com> Co-authored-by: Francisco Gamino <Francisco-Gamino@users.noreply.github.com> Co-authored-by: Shreyas Gopalakrishna <11889130+shreyas-gopalakrishna@users.noreply.github.com> Co-authored-by: AzureFunctionsJava <funcdisc@microsoft.com>

kshyju · 2023-10-13T19:32:37Z

    private bool UsePlaceholderChannel(IRpcWorkerChannel channel)
@kshyju remind me again, do we need to shutdown the channel immediately or can wait like 5 sec? trying to see if this will have negative cold start impact on existing apps once we switch to opt out only

Refers to: src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs:160 in 42e4df3. [](commit_id = 42e4df3, deletion_comment = False)

Opened #9615 to investigate that.

* Updating patch version * Sending command line args with functions- prefix to prevent conflicts (#9514) * Sending a second copy of command line args with "FUNCTIONS_" prefix * Cleanup * Sending a second copy of command line args with "FUNCTIONS_" prefix * switched to functions- prefix * Removed duplicate port from functions_uri * Switching to kebab case for the new args * Update test * Update native placeholder package to handle new args * Update release notes. * Limit dotnet-isolated specialization to 64 bit host process (#9553) * Skipping native placeholder specialization if host is not 64 bit process * formatting linting fix * Stylecop fix. * fix indentation * Adding E2E test. * Improving log message. * Switch from LogDebug to LogInformation * Fix tests to reflect logging changes * Logging with EventId * Logging using an event name * Updated release notes after rebase merge * Handling env reload response from native placeholder for failure case. (#9602) * Handling env reload response from native placeholder for failure case. * Almost working except one test needs cleanup * Cleanup * Cleanup * Fixing Worker.Extensions.Http package version to align with Http.AspNetCore package. * Logging error as it is received from the worker. * DotNetIsolatedNativeHost package to 1.0.2 * Inlining a variable (Reverting to previous version) * Updating release notes for 4.27.5 * Update PowerShell Language Workers 7.0 (to version 4.0.2973), 7.2 (to version 4.0.2974), and 7.4 (to version 4.0.2975) (#9528) * Upgrade PowerShell language worker 7.4 to 4.0.2975 * Upgrade PowerShell language worker 7.2 to 4.0.2974 * Upgrade PowerShell language worker 7.0 to 4.0.2973 * Update release notes * Update Java Worker Version to 2.13.0 (#9544) * Update Java Worker Version to 2.13.0 * Update release_notes.md for Java worker --------- Co-authored-by: AzureFunctionsJava <funcdisc@microsoft.com> --------- Co-authored-by: azfuncgh <azfuncgh@github.com> Co-authored-by: Shyju Krishnankutty <connectshyju@gmail.com> Co-authored-by: Francisco Gamino <Francisco-Gamino@users.noreply.github.com> Co-authored-by: Shreyas Gopalakrishna <11889130+shreyas-gopalakrishna@users.noreply.github.com> Co-authored-by: AzureFunctionsJava <funcdisc@microsoft.com>

Handling env reload response from native placeholder for failure case.

f32160a

kshyju mentioned this pull request Oct 10, 2023

Improved handling of env reload request - handles older version of worker sdk Azure/azure-functions-dotnet-worker#1955

Merged

7 tasks

mattchenderson added perf perf: cold-start labels Oct 11, 2023

fabiocav linked an issue Oct 11, 2023 that may be closed by this pull request

Prevent placeholder attempts with older versions of the SDK #9606

Closed

kshyju added 3 commits October 11, 2023 17:06

Almost working except one test needs cleanup

22c2cdb

Cleanup

f924ebc

Cleanup

22ab9d1

kshyju marked this pull request as ready for review October 12, 2023 00:37

kshyju requested a review from a team as a code owner October 12, 2023 00:37

kshyju requested review from brettsam, fabiocav and soninaren October 12, 2023 00:38

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread test/DotNetIsolated60/DotNetIsolated60.csproj

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread test/DotNetIsolated60/Program.cs

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread test/DotNetIsolated60/DotNetIsolated60.csproj

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread test/DotNetIsolatedUnsupportedWorker/DotNetIsolatedUnsupportedWorker.csproj

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread test/EmptyScriptRoot/host.json

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs Outdated

Fixing Worker.Extensions.Http package version to align with Http.AspN…

2d06452

…etCore package.

fabiocav approved these changes Oct 12, 2023

View reviewed changes

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs Outdated

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs Outdated

brettsam reviewed Oct 12, 2023

View reviewed changes

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs Outdated

kshyju added 2 commits October 12, 2023 10:27

Logging error as it is received from the worker.

3c0c784

DotNetIsolatedNativeHost package to 1.0.2

5eb863e

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs

kshyju requested review from brettsam and fabiocav October 12, 2023 17:32

brettsam approved these changes Oct 12, 2023

View reviewed changes

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs Outdated

kshyju commented Oct 12, 2023

View reviewed changes

Comment thread src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs

Inlining a variable (Reverting to previous version)

42e4df3

fabiocav approved these changes Oct 12, 2023

View reviewed changes

soninaren approved these changes Oct 12, 2023

View reviewed changes

kshyju merged commit d6a572b into dev Oct 12, 2023

kshyju deleted the shkr/check_dni_worker_supports_specialization branch October 12, 2023 18:46

VpOfEngineering mentioned this pull request Oct 12, 2023

4.27.5 Hotfix #9607

Closed

8 tasks

VpOfEngineering mentioned this pull request Oct 12, 2023

4.27.5 hotfix #9609

Merged

8 tasks

kshyju mentioned this pull request Oct 13, 2023

Delay ShutdownChannelIfExistsAsync until cold start #9615

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling env reload response from native placeholder for failure case.#9602

Handling env reload response from native placeholder for failure case.#9602
kshyju merged 8 commits intodevfrom
shkr/check_dni_worker_supports_specialization

kshyju commented Oct 10, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

safihamid commented Oct 12, 2023

Uh oh!

kshyju commented Oct 13, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

kshyju commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Additional information

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

safihamid commented Oct 12, 2023

Uh oh!

kshyju commented Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kshyju commented Oct 10, 2023 •

edited

Loading

kshyju commented Oct 13, 2023 •

edited

Loading