Code review comment for lp:~themue/juju-core/go-state-lifecycle-relation

https://codereview.appspot.com/6455065/diff/17001/state/lifecycle.go
File state/lifecycle.go (right):

https://codereview.appspot.com/6455065/diff/17001/state/lifecycle.go#newcode45
state/lifecycle.go:45: // init initializes the lifecycle with the state
Alive .
On 2012/08/14 12:36:06, TheMue wrote:
> On 2012/08/13 15:48:49, niemeyer wrote:
> > Why do we need this? We should be creating the actual nodes with
life set to
> > Alive in the first place, rather than changing them to Alive after
the fact
> via
> > this method.

> Maybe this comment is misleading. init() is to write the initial Alive
into the
> config node. init() is called immediately after the creation of the
instance, so
> the config node should be empty so far. But I preferred to take care
that it's
> really so.

The comment isn't misleading. The logic itself is wrong. You can't
create a node that has no lifecycle state and then initialize it after
the fact. Meanwhile, someone else will pick it up, and will see wrong
state.

This, specifically, is a relevant race condition:

98 r := newRelation(st, key)
99 if err := r.lifecycle.init(); err != nil {

We've been going back and forth on this, and I'm feeling unproductive
reviewing this over and over about the same problems.

I suggest we stop work on state, put that aside, and focus entirely on
mstate. Let's implement watches and presence, and move forward.

https://codereview.appspot.com/6455065/