cmd/jujud: avoid using allFatal

cmd/jujud: avoid using allFatal

When a worker gets an error, this prevents it
taking down other workers unless the connection
has actually gone down.

https://code.launchpad.net/~rogpeppe/juju-core/384-avoid-allfatal/+merge/184743

(do not edit description out of merge proposal)

[rog, 2013-09-10 09:53:00]

Please take a look.

[rog, 2013-09-10 10:49:01]

Please take a look.

[natefinch, 2013-09-10 14:19:56]

On 2013/09/10 10:49:01, rog wrote:
> Please take a look.

LGTM modulo a couple minor things.

[natefinch, 2013-09-10 14:50:12]

I always forget that I have to do this publish and mail thing.

https://codereview.appspot.com/13640043/diff/6001/testing/mgo.go
File testing/mgo.go (right):

https://codereview.appspot.com/13640043/diff/6001/testing/mgo.go#newcode94
testing/mgo.go:94: "--sslPEMKeyFile", filepath.Join(mgoDir, "server.pem"),
I don't really like that we're recreating the path to server.pem here.

https://codereview.appspot.com/13640043/diff/6001/testing/mgo.go#newcode143
testing/mgo.go:143: MgoAddr, mgoDir = "", ""
Why are these on the same line?

https://codereview.appspot.com/13640043/diff/6001/testing/mgo.go#newcode152
testing/mgo.go:152: panic(err)
Does this really need to be a panic?

[thumper, 2013-09-10 22:38:35]

I don't think that this actually fixes the problem at all.

The issue is that the api opening fails itself, not the api workers that are
created after the connection has been established.

Since it is the creation of the main api worker itself that is erroring, this
still takes down everything.

I do still like that this change means that not everything is fatal, but it
doesn't address the underlying problem.

Perhaps John's suggestion of retrying the open api several times with a small
delay would be sufficient.

[fwereade, 2013-09-11 07:36:36]

On 2013/09/10 22:38:35, thumper wrote:
> I don't think that this actually fixes the problem at all.
> 
> The issue is that the api opening fails itself, not the api workers that are
> created after the connection has been established.
> 
> Since it is the creation of the main api worker itself that is erroring, this
> still takes down everything.
> 
> I do still like that this change means that not everything is fatal, but it
> doesn't address the underlying problem.

Well, it does address one real problem that I've been banging on about since
before the code was written -- the original deal was to *avoid* the issue in the
new work -- so I'm very happy to see this. I agree it's not finished though ;).

> Perhaps John's suggestion of retrying the open api several times with a small
> delay would be sufficient.

This is, I think, also necessary, and should apply to state connections as well.
My original instructions, FWIW, were to implement the API worker and state
workers *independently*, such that an API worker would come up on its own and
peacefully wait for the appropriate state worker to start serving the API,
rather than tangling the two up with that horrid ensureStateWorker business.

Is there any actual reason to keep the two loops coupled like this? or have the
consequences spread far enough that it's practically inoperable at this stage?

[jameinel, 2013-09-11 09:24:21]

...

> > Perhaps John's suggestion of retrying the open api several times with a
small
> > delay would be sufficient.
> 
> This is, I think, also necessary, and should apply to state connections as
well.
> My original instructions, FWIW, were to implement the API worker and state
> workers *independently*, such that an API worker would come up on its own and
> peacefully wait for the appropriate state worker to start serving the API,
> rather than tangling the two up with that horrid ensureStateWorker business.
> 
> Is there any actual reason to keep the two loops coupled like this? or have
the
> consequences spread far enough that it's practically inoperable at this stage?

ensureStateWorker is based on the premise that all agents but machine-0 will
connect to the API and find out what jobs that agent needs to do, which might
include serving the API. Once we get Provisioner and Uniter such that they don't
need direct db access, then we'll actually be at a point where we don't start
StateWorker at all on agents that aren't API servers (and firewaller, etc, but
all those are part of the state server nodes).

[rog, 2013-09-12 14:53:47]

Please take a look.

[fwereade, 2013-09-12 15:00:11]

LGTM, thanks.

[rog, 2013-09-12 16:18:45]

Please take a look.