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.

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

Error when trying to change type of key #1

Open
hltbra opened this issue Sep 14, 2018 · 1 comment
Open

Error when trying to change type of key #1

hltbra opened this issue Sep 14, 2018 · 1 comment
Labels
bug Something isn't working

Comments

@hltbra
Copy link
Contributor

hltbra commented Sep 14, 2018

There's no check at the moment of what type is the key before trying write operations such as SET/HSET/ZADD/SADD.

$ redis-cli
127.0.0.1:6379> set foo bar
OK
127.0.0.1:6379> type foo
"string"
127.0.0.1:6379> sadd foo 1
(error) ERR "Traceback (most recent call last):\n  File \"/Users/hugo/projects/dredis/dredis/server.py\", line 34, in execute_cmd\n    result = run_command(keyspace, cmd, args)\n  File \"dredis/commands.py\", line 384, in run_command\n    return REDIS_COMMANDS[cmd.upper()](keyspace, *str_args)\n  File \"dredis/commands.py\", line 34, in newfn\n    return fn(keyspace, *args, **kwargs)\n  File \"dredis/commands.py\", line 170, in cmd_sadd\n    count += keyspace.sadd(key, value)\n  File \"dredis/keyspace.py\", line 90, in sadd\n    value_path.write(value)\n  File \"dredis/path.py\", line 29, in write\n    with open(self._path, 'w') as f:\nIOError: [Errno 2] No such file or directory: 'dredis-data/0/foo/values/c4ca4238a0b923820dcc509a6f75849b'\n"
@hltbra hltbra added the bug Something isn't working label Sep 14, 2018
@hltbra
Copy link
Contributor Author

hltbra commented Jul 3, 2019

After the updates with backends and proper key codec, this problem isn't as bad because you'll end up with keys with the same name but different types:

127.0.0.1:6377> set foo bar
OK
127.0.0.1:6377> sadd foo b
(integer) 1
127.0.0.1:6377> zadd foo 123 hello
(integer) 1
127.0.0.1:6377> keys *
1) "foo"
127.0.0.1:6377> get foo
"bar"
127.0.0.1:6377> smembers foo
1) "b"
127.0.0.1:6377> zrange foo 0 -1 withscores
1) "hello"
2) "123"

hltbra added a commit that referenced this issue Jan 9, 2020
There was too much re-parsing when dealing with large inputs that required multiple reads from the network.
@jimjshields found this when sending about 10MB of data to dredis (it was hanging for a long time). That was hanging dredis because it was re-parsing the partial input every time there was new data.
For example, if a client sent the following sequence of commands to parse:
1. "*1\r\n"
2. "$4\r\n"
3. "PING\r\n"

After receiving payload #1, dredis would append it to a buffer and parse the buffer ("*1\r\n"). It is incomplete, so it moves on.
After receiving payload #2, dredis would append it to a buffer and parse the buffer (now the buffer is "*1\r\n$4\r\n"). It is incomplete, so it moves on.
After receiving payload #3, dredis would append it to a buffer and parse the buffer (nw the buffer is "*1\r\n$4\r\nPING\r\n"). It is finally complete, so it executes the command PING.
If you follow this approach for a command with 50,000 elements, it would re-parse all elements until it reaches the final 50,000th element. A much better approach would be to save every parsed element and wait until the last element to execute the command, but never re-parse valid data.

This commit changes the parser to never re-parse valid data. Using the previous example with the current implementation, after each new payload, dredis will only that and never re-parse the previous valid payloads.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant