server/auth: Add in-process LRU-based cache.

server/auth: Add in-process LRU-based cache.

Add an in-process LRU-based GlobalCache implementation. Use it for unit
tests instead of a mocked cache.

BUG=chromium:682465
TEST=unit

Review-Url: https://codereview.chromium.org/2646733008
Committed: https://github.com/luci/luci-go/commit/879672eba7a6a9dd58326c99269cf50ea456115a

[dnj, 2017-01-20 23:27:56]

Patchset #1 (id:1) has been deleted

[dnj, 2017-01-20 23:40:25]

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

[dnj, 2017-01-20 23:40:26]

PTAL

[Vadim Sh., 2017-01-20 23:53:09]

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go
File server/auth/cache.go (right):

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go#ne...
server/auth/cache.go:171: func ProcGlobalCache(size int) GlobalCache {
put a big fat warning that it is not really global :(

Actually, this kind of makes me cautious. It is possible in the future global
(cross-machine) properties of GlobalCache will actually be required.
ProcGlobalCache will stop working.

Or consider renaming GlobalCache to just MemoryCache, commenting that some
implementations may not be global.

[iannucci, 2017-01-21 00:34:56]

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go
File server/auth/cache.go (right):

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go#ne...
server/auth/cache.go:177: func (agc procGlobalCache) Get(c context.Context, key
string) ([]byte, error) {
agc?

[dnj, 2017-01-21 01:07:30]

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go
File server/auth/cache.go (right):

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go#ne...
server/auth/cache.go:171: func ProcGlobalCache(size int) GlobalCache {
On 2017/01/20 23:53:08, Vadim Sh. wrote:
> put a big fat warning that it is not really global :(
> 
> Actually, this kind of makes me cautious. It is possible in the future global
> (cross-machine) properties of GlobalCache will actually be required.
> ProcGlobalCache will stop working.

Hmm - well, I don't want to set future iterations up for failure. Should I back
away from implementing this?

> Or consider renaming GlobalCache to just MemoryCache, commenting that some
> implementations may not be global.

This sounds fine, if moving forward is also fine.

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go#ne...
server/auth/cache.go:177: func (agc procGlobalCache) Get(c context.Context, key
string) ([]byte, error) {
On 2017/01/21 00:34:56, iannucci wrote:
> agc?

oops, used to be called "authGlobalCache" :P

[Vadim Sh., 2017-01-21 01:10:21]

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go
File server/auth/cache.go (right):

https://codereview.chromium.org/2646733008/diff/10005/server/auth/cache.go#ne...
server/auth/cache.go:171: func ProcGlobalCache(size int) GlobalCache {
On 2017/01/21 01:07:30, dnj wrote:
> On 2017/01/20 23:53:08, Vadim Sh. wrote:
> > put a big fat warning that it is not really global :(
> > 
> > Actually, this kind of makes me cautious. It is possible in the future
global
> > (cross-machine) properties of GlobalCache will actually be required.
> > ProcGlobalCache will stop working.
> 
> Hmm - well, I don't want to set future iterations up for failure. Should I
back
> away from implementing this?
> 
> > Or consider renaming GlobalCache to just MemoryCache, commenting that some
> > implementations may not be global.
> 
> This sounds fine, if moving forward is also fine.

Yes, please rename auth.GlobalCache to auth.MemoryCache and add a note that it
may be per-process cache in some implementations.

[dnj, 2017-01-21 02:07:34]

Done, PTAL

[Vadim Sh., 2017-01-21 02:28:16]

lgtm

[dnj, 2017-01-21 03:22:48]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2017-01-21 03:22:53]

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-21 03:38:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484968968664440,
"parent_rev": "9b5c2ac8e891eedde109b1c9ec77735d77a567ae", "commit_rev":
"879672eba7a6a9dd58326c99269cf50ea456115a"}

[commit-bot: I haz the power, 2017-01-21 03:38:12]

Description was changed from

==========
server/auth: Add in-process LRU-based cache.

Add an in-process LRU-based GlobalCache implementation. Use it for unit
tests instead of a mocked cache.

BUG=chromium:682465
TEST=unit
==========

to

==========
server/auth: Add in-process LRU-based cache.

Add an in-process LRU-based GlobalCache implementation. Use it for unit
tests instead of a mocked cache.

BUG=chromium:682465
TEST=unit

Review-Url: https://codereview.chromium.org/2646733008
Committed:
https://github.com/luci/luci-go/commit/879672eba7a6a9dd58326c99269cf50ea456115a
==========

[commit-bot: I haz the power, 2017-01-21 03:38:13]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-go/commit/879672eba7a6a9dd58326c99269cf50ea456115a