Add support for return/ThrowBehavior of services
#148
Conversation
FranzBusch
force-pushed
the
fb-return-behaviors
branch
from
64858c1 to
38f62c8
Compare
FranzBusch
force-pushed
the
fb-return-behaviors
branch
from
38f62c8 to
757b3fb
Compare
| let gracefulShutdownSignals = await UnixSignalsSequence(trapping: self.gracefulShutdownSignals) | ||
| let cancellationSignals = await UnixSignalsSequence(trapping: self.cancellationSignals) |
There was a problem hiding this comment.
Might be simpler to just create one sequence because both tasks for these are exactly the same...
There was a problem hiding this comment.
I tried that first but it makes things a bit more complicated because right now I return from the child task on the first signal; however, we have to keep listening to cancellation signals once we get a graceful shutdown signal.
| public var returnBehaviour: TerminationBehavior | ||
| /// The behavior when the service throws from its `run()` method. | ||
| public var throwBehaviour: TerminationBehavior |
There was a problem hiding this comment.
You need to use the US spelling for the properties here.
There was a problem hiding this comment.
The naming here doesn't quite sit right with me; it's very strongly tied to language keywords rather than the behaviour of the service runner.
Not sure exactly what to suggest but the idea of a 'clean exit' vs. 'unclean exit' might be worth exploring.
There was a problem hiding this comment.
Adopted the US spelling. I am really struggling to come up with something better.
There was a problem hiding this comment.
Moved to use successfulTerminationBehavior and failureTerminationBehavior
FranzBusch
force-pushed
the
fb-return-behaviors
branch
from
757b3fb to
822f4f6
Compare
# Motivation Currently the `ServiceGroup` cancels the whole group when a service returns or throws. This is fine for most server applications but it doesn't work for CLI tools since they mostly want to use the group to orchestrate the services as long as the user command is handled and then return cleanly from the group. Furthermore, even on server setups one might want to customize the behavior of what happens when a service returns/throws e.g. one might want to shutdown the group after their HTTP server throws so that a telemetry service flushes out all the remaining data. # Modification This PR does a few things: 1. It creates a new `ServiceConfiguration` and `TerminationBehavior` in the `ServiceGroupConfiguration`. Those can be used to declare the services that are run and what happens when the service returns or throws 2. Adds a new `cancellationSignals` to to the `ServiceGroupConfiguration` which allows cancellation to trigger based on a signal 3. Makes sure that any given service is only retained as long as necessary i.e. when a service returns the group is not retaining it anymore. This allows freeing of resources as early as possible 4. Breaking: Removes the `Hashable` conformance on the configuration structs. This was wrong in the first place and something we should just avoid doing in general. # Result We now give the user even more control about the group's behaviors.
FranzBusch
force-pushed
the
fb-return-behaviors
branch
from
822f4f6 to
de13697
Compare
| @@ -80,7 +81,8 @@ extension UnixSignalsSequence { | |||
| signal(sig.rawValue, SIG_IGN) | |||
| #endif | |||
| return .init( | |||
| dispatchSource: DispatchSource.makeSignalSource(signal: sig.rawValue, queue: UnixSignalsSequence.queue), | |||
| // This force-unwrap is safe since Dispatch always returns a `DispatchSource` | |||
There was a problem hiding this comment.
This is true under the assumption that DispatchSource is currently the only type conforming to DispatchSourceSignal, right?
There was a problem hiding this comment.
Yeah and if we start to crash here then we will notice it and have to see what we get returned.
Sources/ServiceLifecycle/Docs.docc/How to adopt ServiceLifecycle in applications.md
Outdated
Show resolved
Hide resolved
Sources/ServiceLifecycle/Docs.docc/How to adopt ServiceLifecycle in applications.md
Outdated
Show resolved
Hide resolved
Sources/ServiceLifecycle/Docs.docc/How to adopt ServiceLifecycle in libraries.md
Outdated
Show resolved
Hide resolved
| await XCTAsyncAssertEqual(await eventIterator1.next(), .runPing) | ||
|
|
||
| // The first service should now receive the signal | ||
| await XCTAsyncAssertEqual(await eventIterator1.next(), .shutdownGracefully) |
There was a problem hiding this comment.
Shouldn't we also be getting a runCancelled event after the second service exits?
There was a problem hiding this comment.
Good one but no we do not since a service returning when we are gracefully shutting down is expected.
| self.cancellationSignals = cancellationSignals | ||
| } | ||
|
|
||
| @available(*, deprecated) |
There was a problem hiding this comment.
Will this be removed before 2.0?
There was a problem hiding this comment.
We probably should.
Motivation
Currently the
ServiceGroupcancels the whole group when a service returns or throws. This is fine for most server applications but it doesn't work for CLI tools since they mostly want to use the group to orchestrate the services as long as the user command is handled and then return cleanly from the group. Furthermore, even on server setups one might want to customize the behavior of what happens when a service returns/throws e.g. one might want to shutdown the group after their HTTP server throws so that a telemetry service flushes out all the remaining data.Modification
This PR does a few things:
ServiceConfigurationandTerminationBehaviorin theServiceGroupConfiguration. Those can be used to declare the services that are run and what happens when the service returns or throwscancellationSignalsto to theServiceGroupConfigurationwhich allows cancellation to trigger based on a signalHashableconformance on the configuration structs. This was wrong in the first place and something we should just avoid doing in general.Result
We now give the user even more control about the group's behaviors.