Fix feature flag race condition with stale reads [RUN ALL RSPEC] [RUN AS-IF-FOSS]
On GitLab.com we sometimes observe that a feature flag has been changed - and we have the response in chatops to demonstrate that - but the behaviour of the application doesn't reflect that. While we cannot prove that this is the case, we believe that this is due to a combination of these factors:
- We have some feature flags (Gitaly feature flags in particular) that are read very frequently. All Gitaly feature flags are read on every Gitaly call.
- We use database load balancing, so a read from a secondary may be slightly behind the primary.
- The feature flag library we use (Flipper) deletes the feature flag
from the cache when it's changed, and relies on the next read to
re-populate the cache. As these reads use what is essentially
Rails.cache.fetch
, the last writer 'wins'.
That could lead to this situation:
- Client A is using the primary. It performs
fetch
, finds nothing, executes its block to get the value. - Client B does the same, but using a secondary and missing out the write performed by client A.
- Client A finishes its block and writes to Redis.
- Client B does the same, clobbering client A's write.
The solution in this change is simple: instead of deleting the feature
flag from the cache when updating it, we always immediately write the
new value back. That way any clients using Rails.cache.fetch
won't
attempt to write until the cache has expired after an hour, by which
point the secondaries would have caught up anyway.
This is behind a feature flag because it is a somewhat risky change, but using a feature flag is safe here as this only controls how we handle feature flag writes.
For #325452 (closed).