Intentions: Add cache for Consul authorization and certificate parsing#61
Intentions: Add cache for Consul authorization and certificate parsing#61
Conversation
pierresouchay
left a comment
pierresouchay
left a comment
There was a problem hiding this comment.
LGTM, but wonder if we should allow to tune the 2018/1min cache configuration
| cfg: cfg, | ||
| c: c, | ||
| cfg: cfg, | ||
| certCache: ttlru.New(2048, ttlru.WithTTL(time.Minute)), |
There was a problem hiding this comment.
this is limited to 2048 entries then?
|
The size of the cache is based on how many service instances will call this one instance in one minute. Additionally, this cache contains pointers that the GC will track and since there is already quite a bit of time spent in the GC I was reluctant having a cache too big. |
|
@dcorbett-haproxy anyone on HAProxy side to review this or can we merge it? |
|
Overall this looks ok to merge, but I'm thinking whether it would be useful to have some feedback about the cache hit rate or eviction rate, perhaps at shutdown? If you guys think it's not necessary, I'll merge it as is. |
With intentions enabled, a lot of time is spent parsing the client certificate and calling consul. This adds a cache for both, taking care of in flight requests resulting in much better performance.
|
@aiharos that's a good point, I've added prometheus metrics to track cache hit ratios |
|
Thank you ! |
3 participants
With intentions enabled, a lot of time is spent parsing the client
certificate and calling consul.
This adds a cache for both, taking care of in flight requests resulting
in much better performance.