Redis cache impl

[markphelps]

Fixes: #633 (kind of)

Major Changes

• Changes caching to be done at the request/response layer instead of the storage layer (using GRPC interceptors/middleware)
• Adds a Redis cache implementation
• Deprecates cache.memory.enabled and cache.memory.expiration
• Display warnings/deprecations in console at startup
• Ping database on startup to check if it's alive
• Default cache TTL is 1m. Previously there was no TTL for the in memory cache
• Support for pretty printing JSON responses from API (via ?pretty=true or setting Accept: application/json+pretty header)
• New cache config
• RequestID setting/Response time setting now happens in middleware for evaluation

Caching Strategy

• Still using a read-through caching strategy
• For the most part we now just rely on TTL to auto evict stale cache entries
• Cache keys are based on the request, see here for the logic

Flags

1. Cache after first flag get request for a specific flag key
2. Evict from cache on flag updates/deletes
3. Evict flag from cache on any variant additions/updates/deletes

Evaluation

1. Cache after evaluation
2. Eviction ONLY occurs because of TTL since its too complex to try and compute the graph for what would constitute manually eviction evalution entries because of other changes (ie segment update, distribution deletion, etc)

Files to 👀

These are the main files that have major changes that I would love some eyes on in this review

• cmd/flipt/main.go
• config/config.go
• server/cache/redis/cache.go
• server/middleware.go especially this function which is where all the caching logic goes

[codecov-commenter]

Codecov Report

Merging #954 (eb1af1e) into main (d507232) will decrease coverage by 1.65%.
The diff coverage is 62.20%.

@@            Coverage Diff             @@
##             main     #954      +/-   ##
==========================================
- Coverage   81.18%   79.53%   -1.66%     
==========================================
  Files          19       17       -2     
  Lines        1802     1798       -4     
==========================================
- Hits         1463     1430      -33     
- Misses        268      296      +28     
- Partials       71       72       +1     

Impacted Files	Coverage Δ
server/server.go	0.00% <0.00%> (-71.43%)	⬇️
server/middleware.go	41.81% <41.81%> (ø)
server/cache/redis/cache.go	75.60% <75.60%> (ø)
config/config.go	89.68% <94.64%> (+1.81%)	⬆️
server/cache/memory/cache.go	100.00% <100.00%> (ø)
server/evaluator.go	91.69% <100.00%> (+0.49%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

[tsickert]

LGTM! Left a few questions, found a typo or 2, but the core functionality looks great--this is an awesome new feature!

I don't have a lot of familiarity with the flag evaluation/middleware parts of the code yet, but looked good from my understanding.

🚀

[tsickert]

Clarification: TTL replaces expiration here and expiration + evicition interval are a gocache paradigm? The Redis cache accepts TTL and uses it to refresh the cache when there's a read + TTL expiration?

[markphelps]

TTL replaces expiration here and expiration

Yeah so TTL = expiration from go-cache terminology. Previously the default config used -1 for expiration which meant items never expired automatically. Since now we are more heavily reliant on TTL since we're moving the caching logic up a layer, I figured it made sense to set a 'sane' 1m default instead across the board.

evicition interval are a gocache paradigm

Yeah. go-cache lets you set how often the 'cleanup' routine runs to deleted expired items from the cache, unlike redis (see below) which seems to manage this process it self

Redis cache accepts TTL and uses it to refresh the cache when there's a read + TTL expiration

Yeah so when we go to read from Redis and it sees that the TTL has expired it acts as a 'miss' and so we go read from the DB and then repopulate the cache item. Redis randomly also deletes expired items from the redis db to clear space, but it doesnt auto-repopulate the cache itself: https://redis.io/commands/expire/#how-redis-expires-keys

[tsickert]

Do you know if redis allows negative TTLs like in-memory cache?

Question behind the question: do you generally want to defer to the underlying library for configuration validation?

[markphelps]

Good question. looking at the lib it seems to return 0 if the TTL is < 0

In general I think we do do more validation on the config after parsing. There is currently minimal validation happening on the config after parsing. IIRC its mostly to catch issues that the libraries used do not like, like passing an empty DB url to the underlying stdlib DB adapters.

We should prob come up with some kind of 'rule' to figure out what we validate and what we dont