server/config: Generic caching backend.

server/config: Generic caching backend.

Add a generic caching backend. The backend works by reducing config
service operations to unique cache keys and supporting various utility
serialization/deserialization methods.

The cache package is currently implemented for "proccache", which uses
the process cache to store configurations. Other cache implementations
will also use this package.

BUG=chromium:674378
TEST=unit

Review-Url: https://codereview.chromium.org/2573403002
Committed: https://github.com/luci/luci-go/commit/ae167b8559d9a2e0de3210b4dea1c665ae25d6d4

[dnj, 2016-12-15 05:11:24]

dnj@chromium.org changed reviewers:
+ iannucci@chromium.org

[dnj, 2016-12-15 05:11:25]

PTAL. Full stack is:
https://codereview.chromium.org/2580713002 (base)
https://codereview.chromium.org/2578893002 (textproto)
https://codereview.chromium.org/2576993002 (access)
https://codereview.chromium.org/2576343002 (erroring)
https://codereview.chromium.org/2582433002 (test)
https://codereview.chromium.org/2573403002 <-- this
https://codereview.chromium.org/2575383002 (gaeconfig)
https://codereview.chromium.org/2576923003 (datastore cache)

[iannucci, 2017-01-07 20:53:17]

lgtm

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/codec.go
File server/config/caching/codec.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/codec...
server/config/caching/codec.go:20: func Encode(v interface{}) ([]byte, error) {
maybe EncodeJSONZlib / DecodeJSONZlib?

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/codec...
server/config/caching/codec.go:51: // unique hash key for that series.
I would document that the output has a fixed size of 32 bytes.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
File server/config/caching/config.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:89: // ValueItem is a cache-optimized
config.Item projection.
I would explain that this means that it stores the result of being transformed
by the format operation, with a link to that chunk of docs.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:100: FormatData []byte `json:"fd,omitempty"`
why is formatdata needed again? couldn't it just be stored in Content with
cmpbin?

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:190: // Encode encodes this Valu.
Value

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:220: HardFailure bool
I would explain that you'd set this to true if the 'fallback' is unacceptably
slow for your application.

I would also maybe call it FailFast or something like that.
https://en.wikipedia.org/wiki/Fail-fast

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:224: // the supplied Loaer.
Loader

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:358: // If an existing cache value is known, it
should be supplied as v. Otherwise,
It would be good to explain how the provided v will be used... is it going to be
saved to the cache? Or something else?

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/doc.go
File server/config/caching/doc.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/doc.g...
server/config/caching/doc.go:6: // store its configuration values.
can we be more specific on what 'caching layer' means? maybe just list the known
compatible caching packages?

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/procc...
File server/config/caching/proccache.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/procc...
server/config/caching/proccache.go:17: // ProcCache returns a config.Backend
that caches results in proccache.
results? resolves?

[dnj, 2017-01-10 03:29:17]

Updated, PTAL

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/codec.go
File server/config/caching/codec.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/codec...
server/config/caching/codec.go:20: func Encode(v interface{}) ([]byte, error) {
On 2017/01/07 20:53:16, iannucci wrote:
> maybe EncodeJSONZlib / DecodeJSONZlib?

i was thinking I'd leave it ambiguous in function name in case we want to change
it in the future. The contract is that Decode works on Encode :)

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/codec...
server/config/caching/codec.go:51: // unique hash key for that series.
On 2017/01/07 20:53:16, iannucci wrote:
> I would document that the output has a fixed size of 32 bytes.

Done.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
File server/config/caching/config.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:89: // ValueItem is a cache-optimized
config.Item projection.
On 2017/01/07 20:53:17, iannucci wrote:
> I would explain that this means that it stores the result of being transformed
> by the format operation, with a link to that chunk of docs.

Done.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:100: FormatData []byte `json:"fd,omitempty"`
On 2017/01/07 20:53:16, iannucci wrote:
> why is formatdata needed again? couldn't it just be stored in Content with
> cmpbin?

b/c we have a single registered Formatter instance (TextProto) that then has to
know *which* textproto to unmarshal into. Could combine, but I think this is
simpler. FormatData is welcome to be structured, but doesn't need to be ATM with
this scheme.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:190: // Encode encodes this Valu.
On 2017/01/07 20:53:16, iannucci wrote:
> Value

Cool kids say "valu".

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:220: HardFailure bool
On 2017/01/07 20:53:16, iannucci wrote:
> I would explain that you'd set this to true if the 'fallback' is unacceptably
> slow for your application.
> 
> I would also maybe call it FailFast or something like that.
> https://en.wikipedia.org/wiki/Fail-fast

I'll rename to FailOnError. "Fail fast" means something different here, I think.
Namely, FailOnError says, "expect to work and die if you don't", and "fail fast"
is done with an intention of not propagating failures downstream, which isn't
what would happen here.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/confi...
server/config/caching/config.go:224: // the supplied Loaer.
On 2017/01/07 20:53:16, iannucci wrote:
> Loader

Done.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/doc.go
File server/config/caching/doc.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/doc.g...
server/config/caching/doc.go:6: // store its configuration values.
On 2017/01/07 20:53:17, iannucci wrote:
> can we be more specific on what 'caching layer' means? maybe just list the
known
> compatible caching packages?

Done.

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/procc...
File server/config/caching/proccache.go (right):

https://codereview.chromium.org/2573403002/diff/1/server/config/caching/procc...
server/config/caching/proccache.go:17: // ProcCache returns a config.Backend
that caches results in proccache.
On 2017/01/07 20:53:17, iannucci wrote:
> results? resolves?

Done.

[iannucci, 2017-01-11 02:56:24]

lgtm

[dnj, 2017-01-11 20:47:07]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2017-01-11 20:47:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 20:53:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484167627466970,
"parent_rev": "bb51c387acaf3309d1ae17d64dbb1e467f158497", "commit_rev":
"ae167b8559d9a2e0de3210b4dea1c665ae25d6d4"}

[commit-bot: I haz the power, 2017-01-11 20:53:33]

Description was changed from

==========
server/config: Generic caching backend.

Add a generic caching backend. The backend works by reducing config
service operations to unique cache keys and supporting various utility
serialization/deserialization methods.

The cache package is currently implemented for "proccache", which uses
the process cache to store configurations. Other cache implementations
will also use this package.

BUG=chromium:674378
TEST=unit
==========

to

==========
server/config: Generic caching backend.

Add a generic caching backend. The backend works by reducing config
service operations to unique cache keys and supporting various utility
serialization/deserialization methods.

The cache package is currently implemented for "proccache", which uses
the process cache to store configurations. Other cache implementations
will also use this package.

BUG=chromium:674378
TEST=unit

Review-Url: https://codereview.chromium.org/2573403002
Committed:
https://github.com/luci/luci-go/commit/ae167b8559d9a2e0de3210b4dea1c665ae25d6d4
==========

[commit-bot: I haz the power, 2017-01-11 20:53:34]

Committed patchset #2 (id:20001) as
https://github.com/luci/luci-go/commit/ae167b8559d9a2e0de3210b4dea1c665ae25d6d4