canonical / charms.reactive

Framework for developing charms and relations using the reactive pattern
Apache License 2.0
21 stars 34 forks source link

Transaction nature of reactive framework needs to be explained better. #137

Open merlijn-sebrechts opened 6 years ago

merlijn-sebrechts commented 6 years ago

From email thread: https://lists.ubuntu.com/archives/juju/2017-October/009548.html

The reactive framework resets state changes when a handler fails. This surprises charm authors. States are often used to ensure idempotency such as in the following example.

@when_not("initialized")
def init():
    must_be_called_exactly_once()
    set_state("initialized")

@when("initialized")
@when_not("ready")
def get_ready():
    this_call_fails()
    set_state("ready")

After this_call_fails fails, the initialized state gets removed again, causing the init function to be called again during the next run. As a result of this behavior, all handlers must be idempotent and states cannot be used to achieve that.

The reason the charm state needs to be rolled back on hook failure is that the changes made to the Juju environment get rolled back on hook failure. However, since hooks are hidden from the charm author it is not clear when a hook starts and when it finishes, and this transactional nature isn't explained in the docs nor are the names of the commands self-explanatory.

ajkavanagh commented 6 years ago

I tend to agree. There are other ways to achieve a true "run this only once" (e.g. use your own sentinel file). But there is a basic issue with the imperative nature of writing code in charms being in 'conflict' with juju's approach of retrying failed hooks.

Your example shows how very, very subtle bugs can be introduced into a charm. Perhaps, in the docs, a more 'declarative' / 'goal-seeking' approach should be considered, where every shown function is written in a way that results in the same state regardless of how many times it is called (successfully)

merlijn-sebrechts commented 6 years ago

I've created patches explaining this behavior for both the Juju docs and the charms.reactive docs. However, writing these patches, some things started to bother me about this behavior..

  1. It makes sense to reset the flags when a hook crashes because Juju resets all actions. However, does it actually make sense to reset the flags during a debug-hooks session? Juju doesn't do this, Juju only discards changes when a debug-hooks session itself exits with an error. Is it possible for us to mimic this behavior in charms.reactive?
  2. Can we dump the state of the reactive framework when crashing? Preferably in a format that we can manually persist (part of) the state if we want to? Just simply dumping the state will help debugging a lot, I think.
  3. Can we tell the user the state has been reset when a handler crashes?