store: add statistics storage support

store: add statistics storage support

This is just the low-level infrastructure, and doesn't yet add
collecting calls or access to the collected data.

https://code.launchpad.net/~niemeyer/juju/go-store-stats/+merge/101442

(do not edit description out of merge proposal)

[niemeyer, 2012-04-10 20:17:04]

Please take a look.

[rog, 2012-04-16 16:51:30]

looks good.

https://codereview.appspot.com/6002045/diff/1/store/store.go
File store/store.go (right):

https://codereview.appspot.com/6002045/diff/1/store/store.go#newcode58
store/store.go:58: // Ignore error. It'll always fail after created.
is there no way to check that the error type is as expected?

https://codereview.appspot.com/6002045/diff/1/store/store.go#newcode100
store/store.go:100: func (s *Store) statsKey(session *storeSession, key
[]string) (string, error) {
i'd like to see some comment explaining how the stats keys work somewhere. it
looks like you're using the row id (?) of the entry in the tokens table to form
the key for that element of the key slice.

why not just use the string key directly? is it an optimisation or a limitation
of keys, or something else?

https://codereview.appspot.com/6002045/diff/1/store/store.go#newcode154
store/store.go:154: // 1 minute resolution
it took me a little while to see what was going on here.
perhaps:
// Use a maximum of one counter per minute by
// rounding down to the start of the current minute.
?

[niemeyer, 2012-04-16 19:54:24]

Please take a look.

https://codereview.appspot.com/6002045/diff/1/store/store.go
File store/store.go (right):

https://codereview.appspot.com/6002045/diff/1/store/store.go#newcode58
store/store.go:58: // Ignore error. It'll always fail after created.
On 2012/04/16 16:51:30, rog wrote:
> is there no way to check that the error type is as expected?

Not at the moment. There's a bug in mgo that will be fixed in the next release.
As you might guess, I've discovered while implementing this. :-) I've added a
TODO note.

https://codereview.appspot.com/6002045/diff/1/store/store.go#newcode100
store/store.go:100: func (s *Store) statsKey(session *storeSession, key
[]string) (string, error) {
On 2012/04/16 16:51:30, rog wrote:
> i'd like to see some comment explaining how the stats keys work somewhere.

Good point. I've used that function's comment to document it. Please see if you
have any further questions.

https://codereview.appspot.com/6002045/diff/1/store/store.go#newcode154
store/store.go:154: // 1 minute resolution
On 2012/04/16 16:51:30, rog wrote:
> it took me a little while to see what was going on here.
> perhaps:
> // Use a maximum of one counter per minute by
> // rounding down to the start of the current minute.
> ?

Sorry, I should have been more clear indeed. That said, this isn't entirely
right either (we don't have one *counter* per minute). Please see if the new
comment makes you happier.

[rog, 2012-04-17 08:24:18]

LGTM

https://codereview.appspot.com/6002045/diff/3001/store/store.go
File store/store.go (right):

https://codereview.appspot.com/6002045/diff/3001/store/store.go#newcode59
store/store.go:59: // TODO Check the error once mgo handles it to us.
s/handles/gives/

[niemeyer, 2012-04-17 12:35:25]

*** Submitted:

store: add statistics storage support

This is just the low-level infrastructure, and doesn't yet add
collecting calls or access to the collected data.

R=rog
CC=
https://codereview.appspot.com/6002045

https://codereview.appspot.com/6002045/diff/3001/store/store.go
File store/store.go (right):

https://codereview.appspot.com/6002045/diff/3001/store/store.go#newcode59
store/store.go:59: // TODO Check the error once mgo handles it to us.
On 2012/04/17 08:24:18, rog wrote:
> s/handles/gives/

Done.