e-mzungu / rjc

Redis Java Client
Other
26 stars 5 forks source link

add binary API #5

Closed costin closed 13 years ago

costin commented 13 years ago

Hi,

I'm looking at integrating rjc into Spring Data Key Value but found that there is no binary API (only Strings can be used currently). This prevents non-common data (binary or simply data outside the encoding charset) to be encoded before being used with this client. Having access to byte[] to both keys and values gives full access to the underlying storage and opens up more possibilities.

e-mzungu commented 13 years ago

Do you think that someone will use byte[] as a key? And moreover Redis protocol based on the strings... Anyway, do you want to see something like this: public interface RedisOperations { String set(String key, String value); byte[] set(byte[] key, byte[] value); String get(String key); byte[] get(byte[] key); ... }

Or it will be enough for you propose to migrate to binary API only in org.idevlab.rjc.RedisCommands? I mean: public interface RedisCommands extends MultiExecCommands { void set(byte[] key, byte[] value); void get(byte[] key); byte[] getBulkReply(); ... }

costin commented 13 years ago

Of course - we do. Consider Java serialization, String with various encodings or snippets of files. The latest set/getbit also take advantage of the internal binary data. While the majority might use String, there will be plenty of cases where access to the raw data is preferred. Both solutions work as far as I'm concerned - the only request is to make sure the same underlying client/protocol underneath.

P.S. I've finished integrating RJC into Spring Data today and it worked pretty nicely. I'm currently tracking an issue with pubsub (will keep you posted). Good work!

costin commented 13 years ago

Hey,

Any update on this issue? With the current state of things we have to encode/decode all data going back and forth to Redis which kills performance. This problem is literally a blocker in production envs.

e-mzungu commented 13 years ago

I hope it will be done until May

costin commented 13 years ago

Hi,

Any update on this one? I'm preparing the next release and wanted to know where we at to document the functionality accordingly. Thanks

e-mzungu commented 13 years ago

I'm planing to finish with it during next weekend

e-mzungu commented 13 years ago

So, I would suggest you to use new class org.idevlab.rjc.RedisClientImpl (available since 0.7) if you need to get binary API. And more over, I think this class can be used in all other cases in you wrapper.

costin commented 13 years ago

Thanks for the update - will try the new release and report back (shouldn't take more then 1-2 days).

Cheers!

costin commented 13 years ago

Hi,

I've tried integrating 0.7 into Spring Data but reverted to the 0.6.x version. 0.7 basically provides a binary connection but not a binary API which is what I'm looking for. Unfortunately by looking at the example it seems the multi/pipeline uses the same approach which forces the user to literally copy RedisSession. Consider what it takes to do a brpop:

  client.setTimeoutInfinite();
  client.getBinaryMultiBulkReply(RedisCommand.BRPOP, args)
  client.rollback();

Basically the user needs to know the protocol usage and semantics for each command. Compare this to Session.brpop() - so much cleaner and easier. Same goes for transaction and pipeline.

Can you please add a binary equivalent of Session (BinarySession ?) or potentially something similar for pipeline/multi - a Session-like interface that returns void for example? Or that simply returns null which avoids the need of introducing two new interfaces. Of course, the BinarySession/Session interfaces could be combined so one can use either String or byte[] on the same object.

Unfortunately, as it stands right now it's simply easier to use the String Session API and do encoding/decoding...

Thanks.