Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ERR MULTI calls can not be nested after upgrade to redis 4.6.0 #191

Closed
ArturT opened this issue Mar 4, 2022 · 2 comments · Fixed by #192
Closed

ERR MULTI calls can not be nested after upgrade to redis 4.6.0 #191

ArturT opened this issue Mar 4, 2022 · 2 comments · Fixed by #192

Comments

@ArturT
Copy link

ArturT commented Mar 4, 2022

I use redis-namespace 1.8.1.
I upgraded redis gem from 4.5.1 to 4.6.0 and I noticed errors from time to time in our Sidekiq workers.

Redis::CommandError: ERR MULTI calls can not be nested

I believe this is related to recent changes in redis gem regarding multi method:
https://github.com/redis/redis-rb/blob/master/CHANGELOG.md#460

Deprecate calling commands on Redis inside Redis#multi.

redis.multi do
  redis.get("key")
end

should be replaced by:

redis.multi do |transaction|
  transaction.get("key")
end

Here is how to reproduce the error.

# reproduce_bug.rb
require 'redis'
require 'redis-namespace'
key = 'a_key'
threads = []
redis = Redis.new(timeout: 5)
redis = Redis::Namespace.new('my-namespace', redis: redis)
1000.times do
  threads << Thread.new do
    redis.multi do |transaction|
      transaction.incr(key)
      transaction.expire(key, 360)
    end
  end
end

threads.each(&:join)
puts 'it works!'

Then run in bash this

while [ "$?" == 0 ]; do ruby reproduce_bug.rb; done

It will keep running the script until it fails.

bash-3.2$ while [ "$?" == 0 ]; do ruby reproduce_bug.rb; done
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
it works!
#<Thread:0x0000000155952da0 reproduce_bug.rb:8 run> terminated with exception (report_on_exception is true):
/Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:246:in `call_pipelined': ERR MULTI calls can not be nested (Redis::CommandError)
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:200:in `block in call_pipeline'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:344:in `with_reconnect'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:198:in `call_pipeline'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:221:in `block in multi'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:258:in `block in synchronize'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:258:in `synchronize'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:258:in `synchronize'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:215:in `multi'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-namespace-1.8.1/lib/redis/namespace.rb:523:in `namespaced_block'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-namespace-1.8.1/lib/redis/namespace.rb:294:in `multi'
	from reproduce_bug.rb:9:in `block (2 levels) in <main>'
/Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:246:in `call_pipelined': ERR MULTI calls can not be nested (Redis::CommandError)
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:200:in `block in call_pipeline'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:344:in `with_reconnect'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis/client.rb:198:in `call_pipeline'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:221:in `block in multi'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:258:in `block in synchronize'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:258:in `synchronize'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:258:in `synchronize'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-4.6.0/lib/redis.rb:215:in `multi'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-namespace-1.8.1/lib/redis/namespace.rb:523:in `namespaced_block'
	from /Users/artur/.rvm/gems/ruby-3.0.3/gems/redis-namespace-1.8.1/lib/redis/namespace.rb:294:in `multi'
	from reproduce_bug.rb:9:in `block (2 levels) in <main>'

Here the line in redis-namespace. Maybe it somehow wrongly use redis gem after the latest changes for multi ( redis/redis-rb#1059 )?

related issue in redis gem repo

I reported the issue also in redis gem redis/redis-rb#1069 (comment) but we found out that it happens only when using redis-namespace gem.
@ArturT ArturT mentioned this issue Mar 4, 2022
Closed
@ArturT
Copy link
Author

ArturT commented Mar 7, 2022

Here is a script that can help pinpoint the issue.
redis/redis-rb#1069 (comment)

Maybe something is wrong with the following code when running in threads and @redis is overridden.

redis-namespace/lib/redis/namespace.rb

Line 525 in 7e43f2d

original, @redis = @redis, r

casperisfine pushed a commit to casperisfine/redis-namespace that referenced this issue Mar 7, 2022
@byroot
Fix compatibility with redis-rb 4.6.0
fdb85dc 
Fix: resque#191
Fix: redis/redis-rb#1069

In redis-rb 4.6.0, the object yielded by `multi` and `pipelined`
is an unsynchronized `PipelinedConnection` object.

This changed opened a thread safety issue in Redis::Namespace:

  - T1: `redis.multi do`, sets `Namespace#redis = PipelinedConnection`
  - T2: any command called before T1 exists the `multi` block is called on the pipelined connection
casperisfine pushed a commit to casperisfine/redis-namespace that referenced this issue Mar 7, 2022
@byroot
Fix compatibility with redis-rb 4.6.0
261dfc4 
Fix: resque#191
Fix: redis/redis-rb#1088

In redis-rb 4.6.0, the object yielded by `multi` and `pipelined`
is an unsynchronized `PipelinedConnection` object.

This changed opened a thread safety issue in Redis::Namespace:

  - T1: `redis.multi do`, sets `Namespace#redis = PipelinedConnection`
  - T2: any command called before T1 exists the `multi` block is called on the pipelined connection
@casperisfine casperisfine mentioned this issue Mar 7, 2022
Merged
casperisfine pushed a commit to casperisfine/redis-namespace that referenced this issue Mar 7, 2022
@byroot
Fix compatibility with redis-rb 4.6.0
5dccf88 
Fix: resque#191
Fix: redis/redis-rb#1088

In redis-rb 4.6.0, the object yielded by `multi` and `pipelined`
is an unsynchronized `PipelinedConnection` object.

This changed opened a thread safety issue in Redis::Namespace:

  - T1: `redis.multi do`, sets `Namespace#redis = PipelinedConnection`
  - T2: any command called before T1 exists the `multi` block is called on the pipelined connection
casperisfine pushed a commit to casperisfine/redis-namespace that referenced this issue Mar 7, 2022
@byroot
Fix compatibility with redis-rb 4.6.0
b80aefa 
Fix: resque#191
Fix: redis/redis-rb#1088

In redis-rb 4.6.0, the object yielded by `multi` and `pipelined`
is an unsynchronized `PipelinedConnection` object.

This changed opened a thread safety issue in Redis::Namespace:

  - T1: `redis.multi do`, sets `Namespace#redis = PipelinedConnection`
  - T2: any command called before T1 exists the `multi` block is called on the pipelined connection
casperisfine pushed a commit to casperisfine/redis-namespace that referenced this issue Mar 7, 2022
@byroot
Fix compatibility with redis-rb 4.6.0
7f76249 
Fix: resque#191
Fix: redis/redis-rb#1088

In redis-rb 4.6.0, the object yielded by `multi` and `pipelined`
is an unsynchronized `PipelinedConnection` object.

This changed opened a thread safety issue in Redis::Namespace:

  - T1: `redis.multi do`, sets `Namespace#redis = PipelinedConnection`
  - T2: any command called before T1 exists the `multi` block is called on the pipelined connection
@ArturT
Copy link
Author

ArturT commented Mar 7, 2022

The issue has been solved by this PR: #192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix compatibility with redis-rb 4.6.0 casperisfine/redis-namespace
1 participant
@ArturT