Inconsistent cache miss behaviour in MultiStore
In MultiStore
, reads fallback to the secondary store if the primary store value is a falsey. See https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/redis/multi_store.rb#L244
While drafting a failing spec to cover for sscan_each
, I noticed that nil
is only returned in 2 scenarios:
- Redis error raised. This applies to all commands.
- Command returns
nil
or falsey as an empty response. However, redis-rb client returns several forms of "empty" responses:
-
nil
for operations likeget
. This is a falsey -
true/false
for operations likesismember
andexists
. The value directly translates to truthy/falsey. -
0
for count-based lookups likesmembers
,hlen
or-2
forttl
. This is a truthy. -
[nil, nil...]
for operations likemget
. This is a truthy. -
[]
for operations likesmembers
. This is a truthy. -
{}
for operations likehgetall
. This is a truthy. -
[0, []
for operations likehscan
,sscan
. This is a truthy.
Here is a snippet to verify locally using an existing MultiStore
SidekiqStatus
(primary: shared state + secondary: sidekiq)
# Redis details
pry(main)> Gitlab::Redis::SharedState.params
=> {:instrumentation_class=>Gitlab::Instrumentation::Redis::SharedState, :path=>"/Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket", :db=>0}
pry(main)> Gitlab::Redis::Queues.params
=> {:instrumentation_class=>Gitlab::Instrumentation::Redis::Queues, :path=>"/Users/sylvesterchin/work/gitlab-development-kit/redis/redis.socket", :db=>1}
# setup
pry(main)> Feature.disable(:use_primary_and_secondary_stores_for_sidekiq_status)
pry(main)> Gitlab::Redis::SidekiqStatus.with { |c| c.hset('hash', 'a', 1) }
=> 1
pry(main)> Gitlab::Redis::SidekiqStatus.with { |c| c.hgetall('hash') }
=> {"a"=>"1"}
pry(main)> Gitlab::Redis::SidekiqStatus.with { |c| c.set('sskey', 1234) }
pry(main)> Gitlab::Redis::SidekiqStatus.with { |c| c.get('sskey') }
=> "1234"
# turn on feature flag
pry(main)> Feature.enable(:use_primary_and_secondary_stores_for_sidekiq_status)
# empty hash returned even though `hash` is still in redis-sidekiq
pry(main)> Gitlab::Redis::SidekiqStatus.with { |c| c.hgetall('hash') }
=> {}
# fallback read occurs
pry(main)> Gitlab::Redis::SidekiqStatus.with { |c| c.get('sskey') }
=> "1234"
In the docs it says If we fail to fetch data from the new instance, we will fallback and read from the old Redis instance.
. Would cache misses be considered failing
? If so, there is an inconsistent response for cache misses since only cache misses returning nil
would trigger a fallback read.
Could I check with you what was the intended behaviour on cache misses? cc @alejandro @nmilojevic1
This issue originated from #386745 (closed) but got moved into its own since the scope is slightly different.