socketry / async-redis

MIT License
83 stars 18 forks source link

LRANGE crashes with a few thousand of elements #5

Closed davidor closed 5 years ago

davidor commented 5 years ago

LRANGE crashes when I try to get a few thousand elements.

Here's a minimal program to reproduce the error:

require 'async/redis'

endpoint = Async::Redis.local_endpoint
client = Async::Redis::Client.new(endpoint)

list = 'my_list'

Async.run do
  client.del(list)
end

Async.run do
  10000.times do
    client.lpush(list, 1)
  end
end

Async.run do
  client.lrange(list, 0, 4000)
end

This is the error raised when calling .lrange:

Traceback (most recent call last):
        10: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-1.15.1/lib/async/task.rb:199:in `block in make_fiber'
         9: from /tmp/tmp_1.rb:19:in `block in <main>'
         8: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/methods/lists.rb:76:in `lrange'
         7: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/client.rb:100:in `call'
         6: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/pool.rb:52:in `acquire'
         5: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/client.rb:102:in `block in call'
         4: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/protocol/resp.rb:104:in `read_object'
         3: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/protocol/resp.rb:104:in `new'
         2: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/protocol/resp.rb:104:in `initialize'
         1: from /home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/protocol/resp.rb:104:in `block in read_object'
/home/david/.rvm/gems/ruby-2.6.0/gems/async-redis-0.3.2/lib/async/redis/protocol/resp.rb:119:in `read_object': Implementation for token 1 missing (NotImplementedError)

I get the same error if I use client.call('LRANGE', ....) instead of client.lrange

davidor commented 5 years ago

I've been investigating a bit and found out that if I increase the BLOCK_SIZE in https://github.com/socketry/async-io/blob/bb6f3240dcebab448eb2cc2ff1893ad714e8116e/lib/async/io/stream.rb#L29 the problem disappears.

ioquatix commented 5 years ago

It looks like there might be a bug in the parser? Implementation for token 1 missing (NotImplementedError) is an error from the redis protocol parser.

ioquatix commented 5 years ago

Can you make a failing spec?

davidor commented 5 years ago

@ioquatix Thanks for the quick response. I added a failing test that shows the problem: #6

ioquatix commented 5 years ago

Thanks so much for reporting this. The issue should be fixed in the latest version of async-io v1.18.3.