Skip to content

Gracefully shutdown worker on WorkerTerminate request #631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 9, 2022

Conversation

shreyas-gopalakrishna
Copy link
Member

@shreyas-gopalakrishna shreyas-gopalakrishna commented Aug 18, 2022

Issue describing the changes in this PR

resolves #630 Azure/azure-functions-host#2308 Azure/azure-functions-host#3296

attached design doc: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs?path=/TeamDocs/FunctionTeamDocs/Design/OOPWorkers/GracefulShutdownOfWorker.md&_a=preview&anchor=out-of-proc-support-for-cancellation-tokens

The flow would be the following:

  • If the worker does not support termination notification (based on capabilities RpcWorkerConstants.HandlesWorkerTerminateMessage), no change in the behavior.
  • Otherwise; the host will send a termination message and wait for the process to exit up to the timeout for process termination.
  • The worker's responsibility would be to handle that message and exit the process within the allotted time. In the case of .NET, application lifetime services will be notified and if customers are subscribing to shutdown events and tokens, they would also receive the notification. This would also terminate hosted services.
  • If the timeout is hit, the process would be killed.

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
  • I have added all required tests (Unit tests, E2E tests)

@kaibocai
Copy link
Member

Hi @liliankasem , just to confirm that host will restart after it send out WorkerTerminate request to language worker right. Do we long how many time we got between WorkerTerminate request send out and host restart? Thanks.

@liliankasem
Copy link
Member

Hi @liliankasem , just to confirm that host will restart after it send out WorkerTerminate request to language worker right. Do we long how many time we got between WorkerTerminate request send out and host restart? Thanks.

Grace period is set here, which I believe is currently 5 seconds @surgupta-msft can confirm

Copy link
Member

@kaibocai kaibocai left a comment

Choose a reason for hiding this comment

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

add some minor comments.

kaibocai
kaibocai previously approved these changes Sep 7, 2022
Copy link
Member

@kaibocai kaibocai left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@kaibocai kaibocai left a comment

Choose a reason for hiding this comment

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

👍

@shreyas-gopalakrishna shreyas-gopalakrishna merged commit 59d7dd1 into dev Sep 9, 2022
@shreyas-gopalakrishna shreyas-gopalakrishna deleted the shreyasg/worker-terminate branch September 9, 2022 15:57
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.

Allow java workers to terminate gracefully when WorkerTerminate message received
3 participants