Determine fallback behaviour of `*scan_each` Redis commands in MultiStore

Background

Gitlab::Redis::MultiStore or MultiStore helps GitLab rails migrate keys between 2 Redis instances smoothly without downtime.

We have recently expanded the list of supported commands to include sscan_each in repository_set_cache.rb. However, redis-rb *scan_each commands does not interact well with the current implementation of MultiStore.

Problem 1: inconstent fallback behaviour

tldr; when *scan_each should fallback, it doesn’t. And when *scan_each should not fallback, it does

*scan_each commands cannot be integrated into MultiStore under WRITE_COMMANDS or READ_COMMANDS.

*scan_each (hscan, sscan, scan, zscan) Redis commands are read commands which behaves differently from typical read commands

  1. Without a block: an Enumerator is returned.
  2. With a block: the block is performed via keys.each(&block) and a nil is returned.

Without a block

An empty set returns an Enumerator means fallback_read is never performed.

[1] pry(main)> Gitlab::Redis::SharedState.with { |c| c.sadd?('set', ['a', 'b']) }
=> true

# non-empty set returns an Enumerator
[2] pry(main)> Gitlab::Redis::SharedState.with { |c| c.sscan_each('set') }
=> #<Enumerator: ...>
[3] pry(main)> Gitlab::Redis::SharedState.with { |c| c.sscan_each('set') }.to_a
=> ["a", "b"]

# empty set returns an Enumerator too
[4] pry(main)> Gitlab::Redis::SharedState.with { |c| c.sscan_each('set2') }
=> #<Enumerator: ...>
[5] pry(main)> Gitlab::Redis::SharedState.with { |c| c.sscan_each('set2') }.to_a
=> []

With a block

*scan_each is performed on both stores if use_primary_and_secondary_stores_for_{store name} is enabled. This variable value line will be nil and fallback_read will be called regardless.

[6] pry(main)> Gitlab::Redis::SharedState.with do |redis|
[6] pry(main)*   redis.sscan_each('set') do |value|
[6] pry(main)*     puts value
[6] pry(main)*   end
[6] pry(main)* end
a
b
=> nil

See original discussion here !107429 (comment 1222403492)

Problem 2: Inconsistent block execution behaviour

Using sscan_each as our example (this applies to hscan/zscan/scan_each too) and assuming use_primary_and_secondary_stores_for_ feature flag is enabled:

redis.sscan_each('set') do |k|
  redis.del(k)
end

This snippet passes the block into sscan_each and del is performed for the instance where the key is found in. E.g. if set in primary and secondary instances differ, the deletions on each instances would differ too. This is because we assign @instance and use it when performing subsequent writes/reads within the scope (see here for an example).

Note that taking problem 1 into consideration, sscan_each and del would occur for both primary and secondary instances.

redis.sscan_each('set').each do |k|
  redis.del(k)
end

This snippet works on the Enumerator via .each(&block). del is performed on both primary and secondary instances. If set in primary and secondary instances differ, the deletions would be performed using the keys in the primary instance's set unless a fallback read is performed. Note that taking problem 1 into consideration, sscan_each would be run on primary only while del would run on both primary and secondary instances.

Note that block does not necessarily perform redis operations. The current (and only) instance of sscan_each just reads keys from the set (https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/finders/repositories/branch_names_finder.rb#L15)

Expected behaviour

In both cases (with and w/o blocks), fallback_read should only be triggered if nothing is found (empty enumerator).

Edited by Sylvester Chin