https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst File source/drafts/stopping-units.rst (right): https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode4 source/drafts/stopping-units.rst:4: ------------- On 2012/03/21 20:07:53, niemeyer wrote: > Sorry for nitpicking, but can we please stick to a consistent convention for > headers? I believe that's what we have today: > Top level: === > Inner level: --- done. https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode30 source/drafts/stopping-units.rst:30: while still allowing for termination of errant children. On 2012/03/21 20:07:53, niemeyer wrote: > This is a very dense paragraph. Can it be simplified to something like: > """ > Juju records the desired state in the topology, and preserves that state while > the actions to accomplish it are still taking place. > """ sounds good. https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode32 source/drafts/stopping-units.rst:32: How it works, what follows is a a worked example going through a unit On 2012/03/21 20:07:53, niemeyer wrote: > "How it works, what follows"? removed https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode58 source/drafts/stopping-units.rst:58: actions are recorded in the topology as a key 'action' value 'destroy' On 2012/03/21 20:07:53, niemeyer wrote: > The "action: destroy" terminology feels a bit out of place here. This is pretty > much a procedure call, but the topology holds state instead. Something like > "destroyed: true" would be more appropriate. It remains true even after the > action has taken place. action here was a really a name for a coordinated state transition, ie. an implied final state. recording the state instead of the activity sounds good. i have a minor preference for a generic key over per state keys, as i can't think of any multi-valued concurrent activities/states here, but not guessing via constraints in the face of ambiguity does seem to be the wiser choice. https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode69 source/drafts/stopping-units.rst:69: - executing stop hook. On 2012/03/21 20:07:53, niemeyer wrote: > This should be done after departing from relations. definitely https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode72 source/drafts/stopping-units.rst:72: - unit agent updates /units/unit-x/stop to reflect the action has been performed. On 2012/03/21 20:07:53, niemeyer wrote: > That step doesn't seem necessary. We can reuse the existing liveness mechanism > that is already supported by the agent. When the agent dies, the machine will > know it. > It also seems like a nice idea to have the stop ZooKeeper node existing for as > long as that's the intended state of the unit. We have to need that node on > startup for watching it anyway. i wanted to avoid presence node usage, due its known problems with session expiration. ie if an agent has a restart in the middle of the stop process, the machine agent watch may erroneously think the agent is up. The stop node explicitness feels more robust in this context, and as you've noted its nesc for the coordination aspect anyways. Its also nice to clean up the local coordination structure when the coordination is done. https://codereview.appspot.com/5847053/diff/5001/source/drafts/stopping-units.rst#newcode94 source/drafts/stopping-units.rst:94: and garbage collects their topology footprint and zk state. On 2012/03/21 20:07:53, niemeyer wrote: > What about garbage collecting on the next action after the decision that it's > fine to GC it? I don't think we need a specific process that has the task of > GCing the topology. > In both cases, though, we have an issue: when is it ok to GC it? This proposal > is not providing any means for deciding on that, which means the data becomes > eternal. That hole indicates postponing that side of the problem may not be a > good idea. We can do something simple, but it'd be good implement cleaning up at > the same time. There is some cost associated to doing gc every user action, its n roundtrips for the client to check. I agree it is simpler for user-gc, but there are other forms of garbage that could be opportunistically collected with an agent (ie. old /relations, non topology referenced service nodes) Its not clear that garbage will be a problem in practice though outside of the topology, so i'm fine with the user initiated gc confined to topology intents. https://codereview.appspot.com/5847053/