search

e-mzungu / rjc

Redis Java Client
Other
26 stars 5 forks source link

pubsub threading issues #8

Closed costin closed 13 years ago

costin commented 13 years ago

Since Redis subscriptions are blocking, currently rjc creates a thread that waits for messages to arrive. However this has several issues:

  1. there is no control over what thread does the waiting. In managed environments (app servers) a thread pool could be used but currently that's impossible since each call spawns a new thread.
  2. connection problems Seems to be some problems with the same underlying connection being used (with pooling). In my tests from time to time I get the following error: Exception in thread "Thread-3" java.lang.ClassCastException: [B cannot be cast to java.util.List at org.idevlab.rjc.ds.RedisConnectionImpl.getObjectMultiBulkReply(RedisConnectionImpl.java:203) at org.idevlab.rjc.ds.PoolableRedisConnection.getObjectMultiBulkReply(PoolableRedisConnection.java:139) at org.idevlab.rjc.Client.getObjectMultiBulkReply(Client.java:708) at org.idevlab.rjc.message.RedisNodeSubscriber$ResponseThread.run(RedisNodeSubscriber.java:134)
  3. message delays/drops We got into an interesting problem in the test suite where the first messages where lost. The cause seems to be the new thread that needs some time to start waiting for messages - however since subscribe returned immediately one assumes the subscription is set up when that's not the case. Even worse, there's no way to know when that happens.

A simple solution to this problems would be to just avoid the extra thread and simple use the thread that invokes subscribe. This way the user is in full control and can add whatever monitors/strategy she wants.

costin commented 13 years ago

Turning off pooling seems to solve 1" (the CCE problems)

P. S. Seems to be a problem with Github comments since my edits do not show up

e-mzungu commented 13 years ago

Looks like I have reproduced second problem and I will look into it. If subscribe use the same thread for message listening it will block that thread. I'm not sure that client will expect that behavior.

costin commented 13 years ago

Actually if you're coming from a Redis background, that's what you expect. Also other libraries (like Jedis) use the same approach. At the end of the day, both approaches (simple but blocking and non-blocking with an additional thread) could be offered.

e-mzungu commented 13 years ago

Looks like you right... Ok, I'll simplify pubsub functionality and remove threading stuff.

e-mzungu commented 13 years ago

RedisNodeSubscriber was completely reworked and simplified. Update is available in integration branch and will be available in 0.6.3 release

costin commented 13 years ago

Great - any ETA for 0.6.3? For us, the sooner the better since I'd like the next release of Spring Data to feature RJC as well.

e-mzungu commented 13 years ago

If it is critical for you I can release next version on Monday or Tuesday evening, maybe with some minor fixes. Is it Ok for you?

costin commented 13 years ago

That would be great! Thanks!