Skip to content
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

Add IHostedLifecycleService #87335

Merged
merged 5 commits into from
Jun 30, 2023
Merged

Add IHostedLifecycleService #87335

merged 5 commits into from
Jun 30, 2023

Conversation

steveharter
Copy link
Member

@steveharter steveharter commented Jun 9, 2023

Fixes #86511

Please see the "design notes" section in the linked issue above. Summary:

  • Exception semantics are the same:
    • Exceptions are caught, logged, and then re-thrown
  • Same guaranteed execution of callbacks
    • Callbacks will be called even if a prior phase had exceptions.
    • Exceptions from IHostApplicationLifetime are logged but do not re-throw.
  • There is a "serial+concurrent" model for both new callbacks and the existing StartAsync\StopAsync when HostOptions.ServicesStartConcurrently and\or HostOptions.ServicesStopConcurrently are enabled.
  • The new timeout for start is disabled by default.
  • The new callbacks (Started, Stopping, Stopped) come before the corresponding IHostApplicationLifetime callbacks. The corresponding IHostLifetime callbacks (WaitForStartAsync and StopAsync) always come first\last.

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Jun 9, 2023

Tagging subscribers to this area: @dotnet/area-extensions-hosting
See info in area-owners.md if you want to be subscribed.

Issue Details

WIP

Author: steveharter
Assignees: steveharter
Labels:

area-Extensions-Hosting

Milestone: -

commit d0e9189a6fe21b3e604dc3188a804d0cb252125d
Author: Steve Harter <steveharter@users.noreply.github.com>
Date:   Fri Jun 9 13:20:30 2023 -0500

    sync

commit a5e88d58c15be5f3fe63e4d3f994448054ca8dbe
Merge: 8010ffa392c 9b7db0f
Author: Steve Harter <steveharter@users.noreply.github.com>
Date:   Fri Jun 9 10:59:47 2023 -0500

    Merge branch 'main' of https://github.com/steveharter/runtime into Startup

commit 8010ffa392cc10ceb96ed84c7a1c571a7dc47aac
Author: Steve Harter <steveharter@users.noreply.github.com>
Date:   Fri May 19 10:39:31 2023 -0500

    Add IServiceStartup to enable pre-startup hooks
@steveharter steveharter added this to the 8.0.0 milestone Jun 28, 2023
@steveharter steveharter marked this pull request as ready for review June 28, 2023 20:49
@steveharter steveharter changed the title Add IStartupHostedService Add IHostedLifecycleService Jun 28, 2023
Copy link

@rafal-mz rafal-mz left a comment

Choose a reason for hiding this comment

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

cool 😎

@Tratcher
Copy link
Member

Tratcher commented Jun 28, 2023

ServicesStartConcurrently and ServicesStopConcurrently should apply uniformly across events. Anything else is going to be extremely confusing. Especially since nobody even asked for ServicesStartConcurrently.

@steveharter
Copy link
Member Author

ServicesStartConcurrently and ServicesStopConcurrently should apply uniformly across events. Anything else is going to be extremely confusing. Especially since nobody even asked for ServicesStartConcurrently.

That was the way it was prototyped originally, but later some of the scenarios were concurrent async validation - e.g. multiple validators (e.g. verifying certificates, endpoints, etc) or warm-up operations that are not related to each other which ideally run concurrently so the startup finishes sooner.

The default for ServicesStartConcurrently is false and changing to true may break backwards compat for existing implementations of StartAsync() especially for hosts that utilize StartAsync() for the long-running actual worker, so that may limit some scenarios that are controlled by a single knob.

If we go with ServicesStartConcurrently\ServicesStopConcurrently for the new events I suppose we could add knobs later (e.g. StartingIsConcurrent) to enable that per-event to enable that per event if necessary (instead of adding knobs later that would have the opposite defaults of ServicesStartConcurrently\ServicesStopConcurrently).

So I'm inclined now to change that behavior as suggested to help avoid confusion.

@steveharter
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

LGTM

@steveharter
Copy link
Member Author

Issues created for the long-running test failures (unrelated to this PR).

@steveharter steveharter merged commit abcd91c into dotnet:main Jun 30, 2023
103 of 107 checks passed
@steveharter steveharter deleted the Startup branch June 30, 2023 21:04
@ghost ghost locked as resolved and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: Add IHostedLifecycleService to support additional callbacks
6 participants