Skip to content

Limit dotnet-isolated specialization to 64 bit host process#9553

Merged
kshyju merged 11 commits intodevfrom
shkr/short_citcuit_specialization
Sep 29, 2023
Merged

Limit dotnet-isolated specialization to 64 bit host process#9553
kshyju merged 11 commits intodevfrom
shkr/short_citcuit_specialization

Conversation

@kshyju
Copy link
Copy Markdown
Member

@kshyju kshyju commented Sep 20, 2023

Fixes #9548

With this change, specialization for dotnet isolated using native placeholder will happen only if the host process is 64 bit.
Our Linux distros are 64 bit and customers cannot change that via portal.

https://learn.microsoft.com/en-us/azure/azure-functions/dotnet-isolated-process-guide#placeholders-preview

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)

@kshyju kshyju self-assigned this Sep 20, 2023
@kshyju kshyju marked this pull request as ready for review September 20, 2023 14:18
@kshyju kshyju requested a review from a team as a code owner September 20, 2023 14:18
@kshyju kshyju requested a review from safihamid September 22, 2023 15:04
Comment thread src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs Outdated
Comment thread src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs Outdated
Comment thread release_notes.md Outdated
@safihamid
Copy link
Copy Markdown
Contributor

safihamid commented Sep 22, 2023

            if (UsePlaceholderChannel(rpcWorkerChannel))

if 'if statement' returns true, maybe better to add a log here that verifies we are using native placeholder and log the PID and process name like mawsfn.... this is to cross check with what DWAS reports as exact match.


Refers to: src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs:126 in 4b480db. [](commit_id = 4b480db3f4c9c58e5207dc2c65bdcb03bf9fa601, deletion_comment = False)

@kshyju
Copy link
Copy Markdown
Member Author

kshyju commented Sep 26, 2023

            if (UsePlaceholderChannel(rpcWorkerChannel))

if 'if statement' returns true, maybe better to add a log here that verifies we are using native placeholder and log the PID and process name like mawsfn.... this is to cross check with what DWAS reports as exact match.

Refers to: src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs:126 in 4b480db. [](commit_id = 4b480db, deletion_comment = False)

We already log the process id inside the method called inside if (UsePlaceholderChannel(rpcWorkerChannel))
https://github.com/Azure/azure-functions-host/blob/v4.27.1/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs#L554

Comment thread src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs Outdated
@safihamid
Copy link
Copy Markdown
Contributor

Can we add process name as well?


In reply to: 1735813698

@kshyju
Copy link
Copy Markdown
Member Author

kshyju commented Sep 26, 2023

if 'if statement' returns true, maybe better to add a log here that verifies we are using native placeholder and log the PID and process name like mawsfn.... this is to cross check with what DWAS reports as exact match.

Discussed offline. The process name here is "FunctionsNetHost". the data which starts with "mawsfn" is the runtime sitename which we log in the "RuntimeSiteName" for every log entry.

Comment thread src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs Outdated
@kshyju kshyju force-pushed the shkr/short_citcuit_specialization branch from 3caef21 to 7bb3a5b Compare September 27, 2023 21:32
@kshyju kshyju requested a review from brettsam September 27, 2023 21:54
Copy link
Copy Markdown
Contributor

@safihamid safihamid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Copy link
Copy Markdown
Member

@FinVamp1 FinVamp1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, thanks for adding the Event Name.

@kshyju kshyju merged commit 13cf289 into dev Sep 29, 2023
@kshyju kshyju deleted the shkr/short_citcuit_specialization branch September 29, 2023 15:46
@kshyju kshyju mentioned this pull request Oct 12, 2023
8 tasks
VpOfEngineering pushed a commit that referenced this pull request Oct 12, 2023
* 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
@VpOfEngineering VpOfEngineering mentioned this pull request Oct 12, 2023
8 tasks
VpOfEngineering added a commit that referenced this pull request Oct 12, 2023
* 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>
VpOfEngineering added a commit that referenced this pull request Nov 16, 2023
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Skip specialization using native placeholder if platform setting is not 64 bit

5 participants