haproxytech / haproxy-consul-connect

HaProxy Connector for Consul Connect. Enables Service Mesh with Consul and HaProxy using TLS and Consul Discovery
Apache License 2.0
95 stars 20 forks source link

Intentions: Add cache for Consul authorization and certificate parsing #61

Closed ShimmerGlass closed 4 years ago

ShimmerGlass commented 4 years ago

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.

ShimmerGlass commented 4 years ago

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. I don't think this value should be configurable because it's pretty low level and it will be hard to explain / users to configure correctly, but if you think its tool low I can set a higher value. Note that the goal here is to cache the bulk of certificate parsing (hence lru), it's not a big issue if a small number of requests cache miss.

pierresouchay commented 4 years ago

@dcorbett-haproxy anyone on HAProxy side to review this or can we merge it?

aiharos commented 4 years ago

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.

ShimmerGlass commented 4 years ago

@aiharos that's a good point, I've added prometheus metrics to track cache hit ratios

ShimmerGlass commented 4 years ago

Thank you !