Commit 3a8a4ced authored by Sean McGivern's avatar Sean McGivern

Don't raise CrossSlotError when no keys given

When no keys are passed to a Redis command that accepts multiple keys,
that's an error. But it's not a cross-slot violation, as there are no
slots. By raising this error we were masking the true
Redis::CommandError underneath, which is bad for two reasons:

1. It's confusing to developers.
2. It leads to a different failure in production to development and
   test.

To elaborate a bit more on the second point: the purpose of the
CrossSlotError is to only be an _additional_ error in development and
test, but it shouldn't _replace_ underlying errors as that can cause
issues like this one.
parent 8e39f635
......@@ -61,7 +61,7 @@ module Gitlab
key_slot(args.first)
end
unless key_slots.uniq.length == 1
if key_slots.uniq.many? # rubocop: disable CodeReuse/ActiveRecord
raise CrossSlotError.new("Redis command #{command_name} arguments hash to different slots. See https://docs.gitlab.com/ee/development/redis.html#multi-key-commands")
end
end
......
......@@ -53,6 +53,7 @@ RSpec.describe Gitlab::Instrumentation::RedisClusterValidator do
:del | [%w(foo bar)] | true # Arguments can be a nested array
:del | %w(foo foo) | false
:hset | %w(foo bar) | false # Not a multi-key command
:mget | [] | false # This is invalid, but not because it's a cross-slot command
end
with_them do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment