cbd / edis

An Erlang implementation of Redis
http://inaka.github.com/edis/
Apache License 2.0
469 stars 37 forks source link

None/nil answer from SADD command? #46

Open ov7a opened 9 years ago

ov7a commented 9 years ago

I am using redis-py client. It expects an int answer from SADD command, but

  File "worker.py", line 305, in update_list
    rserver.sadd(key, *members)
  File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 1477, in sadd
    return self.execute_command('SADD', name, *values)
  File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 565, in execute_command
    return self.parse_response(connection, command_name, **options)
  File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 579, in parse_response
    return self.response_callbacks[command_name](response, **options)
Thread-2    <type 'exceptions.TypeError'>: int() argument must be a string or a number, not 'NoneType'

Same thing happens with LLEN. What does it mean? According to Redis protocol, this shouldn't happen.

Also, I get sometimes records like this in logs:

00:52:49.556 [info] Unexpected info: {#Ref<0.0.0.38537>,{ok,<<MY_DATA_HERE>>}}

It has nothing to do with None answer, happens rarely, but can be related somehow. I had same issues with building as in #44, maybe this is also can be related.

elbrujohalcon commented 9 years ago

@ov7a what version of the redis protocol is that client using? Remember that edis is compatible with Redis 2.4

ov7a commented 9 years ago

As far as I understand, it implements v2.10.3, but has backwards compability. I was using it with redis 2.4.* having no throubles at all about year ago or so.

But nevertheless, both sadd and llen should always return int in all versions. http://redis.io/commands/sadd#history, http://redis.io/commands/llen#history

jfacorro commented 9 years ago

@ov7a I've been checking the response values from edis using redis-cli (2.8.17) and both SADD and LLEN seem to be returning integers as their response.

screen shot 2015-02-18 at 2 54 33 pm

I will try using redis-py, what version of that client are you currently using? Could you also share the commands you are running to get this responses? Thanks!

jfacorro commented 9 years ago

@ov7a This is weird I didn't get any error with the redis-py client either.

screen shot 2015-02-18 at 2 56 02 pm

ov7a commented 9 years ago

Oops, forgot to say. I'm so terribly sorry about that. It occurs only under high load. commands which I use: brpoplpush, llen, sadd, lpush, lrem, setnx redis-py version 2.10.3/

jfacorro commented 9 years ago

@ov7a I can understand now why we weren't seeing that behavior :smile:. Is there any possibility you could set up a repo where we can reproduce the problem?

ov7a commented 9 years ago

yep, my bad :( I'll try to create minimal code to reproduce the problem.

ov7a commented 9 years ago
import redis
import random
import threading

rserver = redis.Redis(host="127.0.0.1")
def make_load():
    for _ in range(1000000):
        rserver.sadd("test", [random.randint(0,100000) for _ in range(1000)])

for _ in range(100):
    t = threading.Thread(target=make_load)
    t.start()
jfacorro commented 9 years ago

@ov7a I'm now seeing the 16:49:12.168 [info] Unexpected info: {#Ref<0.0.0.2706>,{ok,1}} messages and the TypeError exceptions. I'm pretty sure these two are related, we will look into it. Thanks for reporting this.

ov7a commented 9 years ago

One more thing: I was able to get nil response even for official client.

I've wrapped my code with retry on error. This caused a lot of "Unexpected info" messages. I tried official client and got this:

 127.0.0.1:6379> llen works
 (nil)
 (5.05s)
 127.0.0.1:6379> llen works
 (nil)
 (5.00s)

After I stopped the app, server was still logging these for a while. It seems like some buffer overflow: server can't handle so much messages and throw away new ones.

elbrujohalcon commented 9 years ago

@ov7a what backend are you using? leveldb? Also, can you tell us the values on your configuration file, specifically client_timeout and backend?

ov7a commented 9 years ago

I'm using default values: {client_timeout, 35000} and {backend, {edis_eleveldb_backend, [{create_if_missing, true}]}}]}

elbrujohalcon commented 9 years ago

Well, @ov7a… this is the point when I have to say:

edis is still not ready for heavy use. It's still, as it was conceived originally, a toy project. A nice idea to implement, but definitely never scale tested.

I'm sorry about that, but we will most likely never get to actually compete with redis in terms of performance and reliability. We also found, after a lot of trying, that it's really not easy to create a proper multi-master version of redis, which was one of the original ideas behind this project.

Also, we're not investing much time on this project since like an year ago or more. We may eventually address (and probably fix) this issue in the future, but I recommend you not to wait on us.

Sorry about that.

ov7a commented 9 years ago

@elbrujohalcon, thanks. I've also tried ardb, which just hanged, and SSDB, which was not fully compatible with Redis protocol and my needs. Your implementation was the closest. Seems like I'll go back to RDMS.