From 326f8f2c88adfa9034cc6ebcced8798ceb2decbf Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Sat, 26 Oct 2019 12:19:07 -0700 Subject: [PATCH] Add an expiring cache for the caching token authenticator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit And maybe the webhook authorizer cache. This cache has two primary advantages over the LRU cache used currently: - Cache hits don't acquire an exclusive lock. - More importantly, performance doesn't fallover when the access pattern scans a key space larger than an arbitrary size (e.g. the LRU capacity). The downside of using an expiring cache here is that it doesn't have a maximum size so it's suspectible to DoS when the input is user controlled. This is not the case for successful authentications, and successful authentications have a natural expiry so it might be a good fit here. It has some a few differences compared to: https://github.com/kubernetes/kubernetes/blob/3d7318f29d9f56810efd3d690811cdea730b5317/staging/src/k8s.io/client-go/tools/cache/expiration_cache.go - Expiration is not entirely lazy so keys that are never accessed again are still released from the cache. - It does not acquire an exclusive lock on cache hits. - It supports per entry ttls specified on Set. The expiring cache (without striping) does somewhere in between the simple cache and striped cache in the very contrived contention test where every iteration acquires a write lock: ``` $ benchstat simple.log expiring.log name old time/op new time/op delta Cache-12 2.74µs ± 2% 2.02µs ± 3% -26.37% (p=0.000 n=9+9) name old alloc/op new alloc/op delta Cache-12 182B ± 0% 107B ± 4% -41.21% (p=0.000 n=8+9) name old allocs/op new allocs/op delta Cache-12 5.00 ± 0% 2.00 ± 0% -60.00% (p=0.000 n=10+10) $ benchstat striped.log expiring.log name old time/op new time/op delta Cache-12 1.58µs ± 5% 2.02µs ± 3% +27.34% (p=0.000 n=10+9) name old alloc/op new alloc/op delta Cache-12 288B ± 0% 107B ± 4% -62.85% (p=0.000 n=10+9) name old allocs/op new allocs/op delta Cache-12 9.00 ± 0% 2.00 ± 0% -77.78% (p=0.000 n=10+10) $ benchstat simple.log striped.log expiring.log name \ time/op simple.log striped.log expiring.log Cache-12 2.74µs ± 2% 1.58µs ± 5% 2.02µs ± 3% name \ alloc/op simple.log striped.log expiring.log Cache-12 182B ± 0% 288B ± 0% 107B ± 4% name \ allocs/op simple.log striped.log expiring.log Cache-12 5.00 ± 0% 9.00 ± 0% 2.00 ± 0% ``` I also naively replacemed the LRU cache with the expiring cache in the more realisitc CachedTokenAuthenticator benchmarks: https://gist.github.com/mikedanese/41192b6eb62106c0758a4f4885bdad53 For token counts that fit in the LRU, expiring cache does better because it does not require acquiring an exclusive lock for cache hits. For token counts that exceed the size of the LRU, the LRU has a massive performance drop off. The LRU cache is around 5x slower (with lookups taking 1 milisecond and throttled to max 40 lookups in flight). ``` $ benchstat before.log after.log name old time/op new time/op delta CachedTokenAuthenticator/tokens=100_threads=256-12 3.60µs ±22% 1.08µs ± 4% -69.91% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=500_threads=256-12 3.94µs ±19% 1.20µs ± 3% -69.57% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=2500_threads=256-12 3.07µs ± 6% 1.17µs ± 1% -61.87% (p=0.000 n=9+10) CachedTokenAuthenticator/tokens=12500_threads=256-12 3.16µs ±17% 1.38µs ± 1% -56.23% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=62500_threads=256-12 15.0µs ± 1% 2.9µs ± 3% -80.71% (p=0.000 n=10+10) name old alloc/op new alloc/op delta CachedTokenAuthenticator/tokens=100_threads=256-12 337B ± 1% 300B ± 0% -11.06% (p=0.000 n=10+8) CachedTokenAuthenticator/tokens=500_threads=256-12 307B ± 1% 304B ± 0% -0.96% (p=0.000 n=9+10) CachedTokenAuthenticator/tokens=2500_threads=256-12 337B ± 1% 304B ± 0% -9.79% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=12500_threads=256-12 343B ± 1% 276B ± 0% -19.58% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=62500_threads=256-12 493B ± 0% 334B ± 0% -32.12% (p=0.000 n=10+10) name old allocs/op new allocs/op delta CachedTokenAuthenticator/tokens=100_threads=256-12 13.0 ± 0% 11.0 ± 0% -15.38% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=500_threads=256-12 12.0 ± 0% 11.0 ± 0% -8.33% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=2500_threads=256-12 13.0 ± 0% 11.0 ± 0% -15.38% (p=0.000 n=10+10) CachedTokenAuthenticator/tokens=12500_threads=256-12 13.0 ± 0% 10.0 ± 0% -23.08% (p=0.000 n=9+10) CachedTokenAuthenticator/tokens=62500_threads=256-12 17.0 ± 0% 12.0 ± 0% -29.41% (p=0.000 n=10+10) ``` Benchmarked with changes in #84423 Bugs: #83259 #83375 Kubernetes-commit: 9167711fd18511ffc9c90ee306c462be9fc7999b --- go.mod | 21 +++++++++++---------- go.sum | 5 +---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index cae55245..61c508f5 100644 --- a/go.mod +++ b/go.mod @@ -5,18 +5,19 @@ module k8s.io/sample-controller go 1.12 require ( - k8s.io/api v0.0.0-20191114215726-6cd33094d465 - k8s.io/apimachinery v0.0.0-20191114215425-67a48e0c9266 - k8s.io/client-go v0.0.0-20191114220110-6f03b71b98e6 - k8s.io/code-generator v0.0.0-20191114215150-2a85f169f05f + k8s.io/api v0.0.0 + k8s.io/apimachinery v0.0.0 + k8s.io/client-go v0.0.0 + k8s.io/code-generator v0.0.0 k8s.io/klog v1.0.0 ) replace ( - golang.org/x/sys => golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a // pinned to release-branch.go1.13 - golang.org/x/tools => golang.org/x/tools v0.0.0-20190821162956-65e3620a7ae7 // pinned to release-branch.go1.13 - k8s.io/api => k8s.io/api v0.0.0-20191114215726-6cd33094d465 - k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20191114215425-67a48e0c9266 - k8s.io/client-go => k8s.io/client-go v0.0.0-20191114220110-6f03b71b98e6 - k8s.io/code-generator => k8s.io/code-generator v0.0.0-20191114215150-2a85f169f05f + golang.org/x/sys => golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a + golang.org/x/tools => golang.org/x/tools v0.0.0-20190821162956-65e3620a7ae7 + k8s.io/api => ../api + k8s.io/apimachinery => ../apimachinery + k8s.io/client-go => ../client-go + k8s.io/code-generator => ../code-generator + k8s.io/sample-controller => ../sample-controller ) diff --git a/go.sum b/go.sum index 5599e799..e93eaec7 100644 --- a/go.sum +++ b/go.sum @@ -71,6 +71,7 @@ github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= +github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d h1:7XGaL1e6bYS1yIonGp9761ExpPPV1ui0SAC59Yube9k= @@ -213,10 +214,6 @@ gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -k8s.io/api v0.0.0-20191114215726-6cd33094d465/go.mod h1:TnwOuwwOmtUq5BythlhmbJs+lwQGEsSJ6N4S4ODl2m8= -k8s.io/apimachinery v0.0.0-20191114215425-67a48e0c9266/go.mod h1:dXFS2zaQR8fyzuvRdJDHw2Aerij/yVGJSre0bZQSVJA= -k8s.io/client-go v0.0.0-20191114220110-6f03b71b98e6/go.mod h1:Hykx8A+UlxwDre3qJcqZPgCAZGjl9vNp6yc7SmfMQ1k= -k8s.io/code-generator v0.0.0-20191114215150-2a85f169f05f/go.mod h1:Vh0irzg7dL9pFS4c8hFsali5txtbmse3MFS4zEH7Thg= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6 h1:4s3/R4+OYYYUKptXPhZKjQ04WJ6EhQQVFdjOFvCazDk= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20190822140433-26a664648505 h1:ZY6yclUKVbZ+SdWnkfY+Je5vrMpKOxmGeKRbsXVmqYM=