alphazero / jredis

Java Client and Connectors for Redis
http://code.google.com/p/jredis/
Apache License 2.0
315 stars 136 forks source link

full binary support #47

Closed costin closed 13 years ago

costin commented 13 years ago

Currently JRedis provides binary support for values but not for keys. Since Redis 2.0 uses binary-safe strings, such a feature will help greatly for storing data in binary format.

alphazero commented 13 years ago

Costin,

An api change.

Can you help convince me by giving me examples of what sort of keys you can not use right now? And what sort of gymnastics (Romania :) are you willing to go through in terms of switching between semantic e.g. pure binary or classic String, interfaces to JRedis in your code?

Thank you for you feedback.

costin commented 13 years ago

A string is a byte[] array with a certain encoding. So storing data that falls outside the encoding range (think dealing with i18n and multiple encodings inside the same app and thus db), thumbnails or crypto keys simply doesn't work unless doing all sort of unneeded tricks to fit the data into the String. Change the JVM encoding and you have to start over.

Redis is binary safe and libraries such as Jedis support this natively. As for my usage, in Spring Data we give the choice to the user. There's no 'semantic' switch - Strings again are just byte[] with a range of chars (encoding). You can just as well save your objects as XML/Json or use Java Serialization just to name a few options. We always use the binary API , it's the most generic one. In case of JRedis, we go through the gymnastics of encoding the binary data all the time (base64) which is no surprise, kills performance.

alphazero commented 13 years ago

Understood about the encoding but needed use-cases: crypto-keys. convinced me.

alphazero commented 13 years ago

Redis 2.2.n client. branch: master commit: 956a8749b67c41e988648cbdd335e2743fc92ce8

costin commented 13 years ago

That was fast! Thanks - by the way, are there any nightly snapshots for JRedis?

alphazero commented 13 years ago

no plans for nightly snapshots. you guys could help out by running your binary tests against the latest. I did a very trivial test in the hello redis example. The rest of the test suite is still using String keys so do let me know if you see any issues. Specially helpful would be hash tests.

costin commented 13 years ago

Hi,

I've started testing the code and found the following issues: a. List is still used as a return type on JRedis (by keys() and keys(K keys)). b. mget uses Strings instead of byte[] c. KeyValueSet uses String as a key d. list/set/zset operations such as sdiff and sunion are still String based e. hash operations (i.e. hset) use String for a field (instead of K)

alphazero commented 13 years ago

Hi, thanks for catching that.

btw, I gather (please confirm) that at least on your end async ops are not supported?

costin commented 13 years ago

There is work to add support for delayed results. Not sure whether this is what you meant by "async ops"...

alphazero commented 13 years ago

https://github.com/alphazero/jredis/blob/master/core/api/src/main/java/org/jredis/JRedisFuture.java

alphazero commented 13 years ago

Issue GH-47 (wip) -- sunion, sunionstore, sdiff and sdiffstore (future commit de5a329143bc68ddf793

Issue GH-47 (wip) -- sinterstore (future was written) -- TESTED commit 0a0f70cfe8d178928e68

Issue GH-47 (wip) -- sinter() (future was written) TESTED. commit 079367cfd00c31427488

Issue GH-47 (wip) -- rest of the hash methods. commit 1e8e3a3c9410a73eba89

Issue GH-47 (wip) -- hset binary support. (Future was written.) commit d1a74c76ea2ccfe59e7d

Issue GH-47 (wip) -- KeyValueSet and co. (that was 'fun' ..) -- TESTED commit 8c9f4a43ee9f89d6bad0

Issue GH-47 (wip) -- keys() and mget binary support. (The future was commit cf650613c481514f1f9a