Merge lp://staging/~thumper/juju-core/hook-execution-message into lp://staging/~juju/juju-core/trunk

Reviewers: mp+159280_code.launchpad.net,

Message:
Please take a look.

Description:
Write out the hook context id as the lock message.

It has the important parts like the unit name and the hook name.

https://code.launchpad.net/~thumper/juju-core/hook-execution-message/+merge/159280

Requires:
https://code.launchpad.net/~thumper/juju-core/fslock-message/+merge/159261

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8812044/

Affected files:
   A [revision details]
   M worker/uniter/uniter.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: worker/uniter/uniter.go
=== modified file 'worker/uniter/uniter.go'
--- worker/uniter/uniter.go 2013-04-17 00:02:28 +0000
+++ worker/uniter/uniter.go 2013-04-17 02:10:47 +0000
@@ -274,6 +274,13 @@
    }
   }
   hctxId := fmt.Sprintf("%s:%s:%d", u.unit.Name(), hookName, u.rand.Int63())
+ // Explicitly ignore any error we may get writing out the message for the
+ // lock. Since we have a lock, the only thing it could be is that there
+ // was some error actually writing the file, in which case we don't care.
+ // If there are disk full problems, something else serious will fail.
+ // This message is for post-failure debugging anyway where we can
+ // interrogate the file system.
+ _ = u.hookLock.SetMessage(hctxId)
   ctxRelations := map[int]*ContextRelation{}
   for id, r := range u.relationers {
    ctxRelations[id] = r.Context()

If this is meant to be a message to people, could we put more bit of
message so we can understand what the bits of data are without having to
read the code?

LGTM, though it might be nice to do it while taking the lock, instead of
just after.

https://codereview.appspot.com/8812044/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/8812044/diff/1/worker/uniter/uniter.go#newcode283
worker/uniter/uniter.go:283: _ = u.hookLock.SetMessage(hctxId)
As mentioned in the prereq, is it possible to compute this before we
take the lock, so we can put the data into held instead of writing a new
file?

https://codereview.appspot.com/8812044/

https://codereview.appspot.com/8812044/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/8812044/diff/1/worker/uniter/uniter.go#newcode283
worker/uniter/uniter.go:283: _ = u.hookLock.SetMessage(hctxId)
On 2013/04/17 09:56:31, jameinel wrote:
> As mentioned in the prereq, is it possible to compute this before we
take the
> lock, so we can put the data into held instead of writing a new file?

Emphatic +1 on this. And I would like it a lot if we had code and tests
for units breaking their own locks on startup.

https://codereview.appspot.com/8812044/

On 2013/04/17 09:56:31, jameinel wrote:
> If this is meant to be a message to people, could we put more bit of
message so
> we can understand what the bits of data are without having to read the
code?

> LGTM, though it might be nice to do it while taking the lock, instead
of just
> after.

> https://codereview.appspot.com/8812044/diff/1/worker/uniter/uniter.go
> File worker/uniter/uniter.go (right):

https://codereview.appspot.com/8812044/diff/1/worker/uniter/uniter.go#newcode283
> worker/uniter/uniter.go:283: _ = u.hookLock.SetMessage(hctxId)
> As mentioned in the prereq, is it possible to compute this before we
take the
> lock, so we can put the data into held instead of writing a new file?

Done, and done.

https://codereview.appspot.com/8812044/