hseeberger / constructr

Coordinated (etcd, ...) cluster construction for dynamic (cloud, containers) environments
Apache License 2.0
212 stars 37 forks source link

Propagate ConstructR failures to user-space #171

Open nick-nachos opened 6 years ago

nick-nachos commented 6 years ago

The Constructr supervisor actor has a logic according to which:

  1. Leaving the cluster also leads to actor-system termination
  2. Failure of the underlying FSM must eventually lead to actor-system termination

About the first point, it seems redundant and restrictive at the same time to take that decision at the Constructr level: MemberRemoved is an event that can be easily handled by the user-space (as it is akka public API), and there may be multiple cases (like our own) that the actor-system must resume operations even after leaving the cluster. Generally, shutting down the actor-system just seems to be out of the scope of Constructr as an extension.

About the second point, for reasons similar to the above, what would seem more appropriate would be for Constructr to notify the user-space about its terminated state, and let it decide how to handle it (e.g. continue operation without clustering, or shutdown but perform some sort of cleanup first, etc)

The suggested approach here would be to propagate the failure related decision making to the user-space by elevating the ConstructrExtension class; its functionality can be extended so that consumers may register event handlers for Constructr failure, in the same fashion as the akka "native" extensions (e.g. the 'registerOn...handlers of theCluster` extension).

If backwards compatibility is required for these behaviors (i.e. terminate actor system on failures), we can add some configuration setting that will switch them on/off.

Also, given that the ConstructrExtension will be used to provide new functionality, it will be also a good chance to make it abide by the latest akka extension interface, as per akka 2.5.0 the ExtensionKey class has been deprecated. This will have the extra benefit of course that the ConstructrExtension will be initializable both from configuration and source code as well.

hseeberger commented 6 years ago

Good thoughts! This will become a hell of a rewrite ;-)

BTW, backwards compatibility is not required, because we're still in 0.x land. Let's strive for optimal simplicity instead of adding complexity.

nick-nachos commented 6 years ago

Excellent! I'm right on it then :D