Opened 4 years ago

Last modified 4 years ago

#6813 enhancement new

Improved service support

Reported by: Dustin J. Mitchell Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: lvh, Jean-Paul Calderone Branch:
Author:

Description (last modified by Glyph)

Buildbot is a major consumer of services. A big service tree is created at startup, services come and go during operation, and the service tree gets a major pruning during reconfigs.

All of this turns up some design deficiencies:

  • services do not start asynchronously, leaving the caller of startService uncertain as to when the service is actually fully started
  • the startService and stopService methods do not check whether the service is already running. Neither does MultiService check whether its child services are running before calling their start/stop methods. Related: #4366
  • MultiService handles deferreds from stopService incorrectly (this is a straight-up bug, #6790)

There was some discussion of this in #4366, but it appears that the issues cannot be fixed within the current implementation. Maybe they can, or maybe we need a new, alternative implementation.

Change History (3)

comment:1 Changed 4 years ago by lvh

For what it's worth, I've been bitten by most of these, for example, I've written plenty of code where either startService returned a Deferred, or (when I felt less like breaking the API) had some total wart of a method like notifyStarted or something that returned a Deferred to the same effect.

I've only been bitten by multiple starts once, but then again, I'm only one user :)

comment:2 Changed 4 years ago by Glyph

Description: modified (diff)

comment:3 Changed 4 years ago by Glyph

I'm inclined to say that this ticket is far too broad to be actionable. Dustin, can you come up with a more tightly-scoped summary? As it stands I have no idea how this could be considered "done".

However, I do have one comment here: one of the recurring themes here is that services ought to "asynchronously" start up, and by that, most people seem to mean "startService should return a Deferred, and then ... something ... should happen, which involves the service being in some third "starting" state".

I started off sort of -0 on that idea and as time has gone on, I'm becoming much more strongly -1.

The IService and IServiceCollection interfaces are primitives. They're not intended to encapsulate all of the possible patterns and states that all possible services could be in, and they shouldn't be. Services ought to have only two externally visible states: stopped (their initial state), and started (someone called startService). This information is exposed as the 'running' flag.

As soon as you call stopService, the service is "stopped". Someone could call startService on it again, and that would be valid. Then they could call stopService again.

But, please keep in mind, IService is an interface for containers to communicate with application code about its lifecycle, and IServiceCollection is an interface for an application to integrate objects from multiple libraries into a single service. This information: "is it started", I hold is sufficient for those use-cases. Adding more states just adds a potentially bigger bug surface to the container (e.g. twistd) and doesn't provide any information it needs to do its job better. There's a phase of an application's life-cycle it somewhat arbitrarily decides to call "startup" which it might want to provide to its container, but there's nothing that the container can do with this information other than juggle multiple Deferreds and make an annoyed admin have to sit at their terminal hitting C-backslash instead of C-c because it adds a whole new dimension of "this service won't stop when I asked it to stop".

What this ticket is talking about is a more expressive interface for applications to express dependencies on different bits of not-synchronously-accesssible state or behavior during their initialization. In other words, it's super hard to correctly write a service which has a complex multi-party conversation in its initialization code to the IService interface, because stopService potentially needs to keep track of any number of Deferreds in flight, cancel them if stopService is called before they've all fired.

So, by all means, we should add some objects which provide IService that you can wrap around some other interface, which lets you return a Deferred from some "start" method (not startService) and implements startService to do something more reasonable: raise an exception, log a message, or something like that, if you call it twice; implements stopService to know about that Deferred, to cancel it if it starts-while-stopping, and so on. Implementing this should be straightforward.

And of course the documentation should be improved to better cover existing tricks for using these interfaces (like the trick where you use setServiceParent to start up more services as you go along if you have new things that will need stopping in your MultiService).

I completely agree that Twisted-as-a-system has some design deficiencies, in that these use-cases are important and should be addressed. But it doesn't necessarily follow that IService's design, for what it does, is deficient; merely that the broader service architecture is missing some pieces which can be added on top.

Note: See TracTickets for help on using tickets.