snehac-miner / redis

Automatically exported from code.google.com/p/redis
0 stars 0 forks source link

RANDOMKEY and KEYS don't work with keys containing whitespaces #88

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Multi-bulk command protocol allows keys to contain whitespace characters.
RANDOMKEY and KEYS can't work with such keys yet.
This is just reminder that they should be fixed before 1.1 :)

Original issue reported on code.google.com by biruk...@gmail.com on 4 Nov 2009 at 6:00

GoogleCodeExporter commented 8 years ago
Hello birukoff,

my only concern about this is how client libraries should handle this. 
Basically in 
theory multi-bulk query protocol can handle all the commands and not just MSET, 
and if everything uses multi-bulk keys start to be binary safe. But the old 
protocol is 
optimized for latency and uses less CPU time server-side, so basically it's to 
be 
preferred most of the times. So even if Redis now in theory is able to handle 
binary 
safe keys should we use such a feature? I'm not sure, at least for Redis 1.1. 
Maybe 
some client library should implement an option to send all the commands via 
multi-
bulk protocol but should not be the default.

Also note that binary safe keys are not really needed in the long run since 
Redis 1.2 
will bring us hashes that will be binary safe from the ground up.

So... I'll do this change and both RANDOMKEY and KEYS will return a multi-bulk 
reply 
in order to make , at least in theory, Redis keys binary safe, but client libs 
should 
continue to use the old protocol when possible and documentation should not 
advertise key-binary-safeness probably.

Cheers,
Salvatore

Original comment by anti...@gmail.com on 10 Nov 2009 at 9:07

GoogleCodeExporter commented 8 years ago
Hm...
Is the performance impact really so great?
To me, using whitespaces in keys is critically important, and I have even 
re-written
python client completely to use the new protocol exclusively. My application can
survive losing a tenth of millisecond or so, but having no spaces is big pain. 
Maybe,
I won't be using redis at all without this feature...

Original comment by biruk...@gmail.com on 10 Nov 2009 at 2:48

GoogleCodeExporter commented 8 years ago
I thought of sharing my Python client after it is complete; I can also add 
option to
switch protocols.

Original comment by biruk...@gmail.com on 10 Nov 2009 at 3:00

GoogleCodeExporter commented 8 years ago
birukoff: I understand your problem, this is why I think it's a good idea to 
support 
this changing both KEYS and RANDOMKEY implementation but for now don't tell 
that 
Redis keys are binary safe to the world ;)

Once all the client libraries will get multi-bulk query abilities then we can 
just say 
"it's ok if you say your client to always use multi bulk" and probably after 
some 
release and some more benchmarking if the difference is little we can reach a 
point 
where the old protocol is abolished completely.

So the steps are:

1.1 - Change KEYS and RANDOMKEY to play well with binary keys, but don't state 
in 
the docs that Redis is key-binary-safe
1.2 - Try to get all the clients able to switch protocol when the client object 
is 
created if the user wants so.
1.3 - Drop the old protocol if the new one is believed to perform more or less 
as 
well, and focus the efforts on the rewriting of the command dispatcher to 
support 
only the new protocol but with better performances.

Cheers,
Salvatore

Original comment by anti...@gmail.com on 10 Nov 2009 at 3:22

GoogleCodeExporter commented 8 years ago
Is this been fixed in 1.2.x?

Original comment by mrdu...@gmail.com on 24 Mar 2010 at 12:10

GoogleCodeExporter commented 8 years ago
Hello,

Redis doesn't return a bulk reply for RANDOMKEY. Keys are now binary-safe in 
most 
functions, and even support new lines. The problem is, RANDOMKEY still returns 
a 
single-line reply, which doesn't work with new lines.

Steps to reproduce:
1 - start with an empty database.
2 - SET "abc\r\ndef" "hello"
3 - RANDOMKEY → returns "+abc\r\ndef\r\n" which will break most client 
libraries.

Solution:
RANDOMKEY should return a bulk reply, like GET.

Original comment by n.favref...@gmail.com on 10 Apr 2010 at 11:27

GoogleCodeExporter commented 8 years ago
That's fixed now

Original comment by anti...@gmail.com on 12 May 2010 at 7:33