taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.15k stars 131 forks source link

Listeners & Pub/Sub example does not work #116

Closed ozjongwon closed 9 years ago

ozjongwon commented 9 years ago

The example handlers in the doc do not get called. Is this known issue? (if yes, sorry please close this)

ptaoussanis commented 9 years ago

Hi, sorry - I'm not sure I understand? The examples in the README should work, what problem are you having?

ozjongwon commented 9 years ago

(I guess that means a new problem. More details:

Redis server v=2.8.4 org.clojure/clojure "1.6.0" com.taoensso/carmine "2.8.0" )

When I try below code, I do not see any printouts from handlers. I.e., I expect to see "Channel match..." and "Pattern match..." whenever I publish matching key value pairs. But no printouts.

(ns my-app (:require [taoensso.carmine :as car :refer (wcar)]))

(def server1-conn {:pool {}
                   :spec {:host "localhost" :port 6379 :timeout 4000
                          }})

(def listener
  (car/with-new-pubsub-listener (:spec server1-conn)
    {"foobar" (fn f1 [msg] (println "Channel match: " msg))
     "foo*"   (fn f2 [msg] (println "Pattern match: " msg))}
   (car/subscribe  "foobar" "foobaz")
   (car/psubscribe "foo*")))

;;; Confirmed subscribe through 'redis-cli monitor' at this point

(defmacro wcar* [& body] `(car/wcar server1-conn ~@body))

(wcar* (car/publish "foobar" "Hello to foobar!"))

;;; Confirmed publish through 'redis-cli monitor' at this point
ptaoussanis commented 9 years ago

Have you confirmed that your println calls are showing up where you expect them to when on another thread?

Do you see (future (println "this is a test"))printing? You might need to check your cider REPL, etc.

ozjongwon commented 9 years ago
(ns my-app (:require [taoensso.carmine :as car :refer (wcar)]))

(def server1-conn {:pool {}             ;{:max-active 8}
                   :spec {:host "localhost" :port 6379 :timeout 4000 ;;:password ""
                          }})

(def foobar (atom 0))
(def foo (atom 0))

(def listener
  (car/with-new-pubsub-listener (:spec server1-conn)
    {"foobar" (fn f1 [msg] (swap! foobar inc) (println "Channel match: " msg))
     "foo*"   (fn f2 [msg] (swap! foo inc) (println "Pattern match: " msg))}
   (car/subscribe  "foobar" "foobaz")
   (car/psubscribe "foo*")))

1. At this point both foo and foobar should be still zero, shouldn't they? But I got 1

(defmacro wcar* [& body] `(car/wcar server1-conn ~@body))

(wcar* (car/publish "foobar" "Hello to foobar!"))

2. Now, foo and foobar have to be increased by 1. But they are not.

ptaoussanis commented 9 years ago

Can you try that again without the :timeout?

ozjongwon commented 9 years ago

Removing :timeout fixes 2 (and I do not know what it means, honestly) and still I don't understand 1 (I've changed previous comment to make it clear)

ptaoussanis commented 9 years ago

For 1: Both @foobar and @foo will start with a value of 0 in your code there. If you're seeing anything other than 0, try restart Redis &/or your REPL to clear any lingering state.

For 2: the :timeout option was causing your Pub/Sub connection to close after 4000 milliseconds (4 seconds). You probably don't want a :timeout when using Pub/Sub.

Does that help?

ozjongwon commented 9 years ago

No luck for 1. But if the above example code works for you, never mind. Maybe I missed something. Thx!

ptaoussanis commented 9 years ago

No luck for 1.

Sorry - that's my mistake: @foobar and @foo should both have value 1 here, not zero. This is because each is getting one message. If you watch the println output you'll see:

Channel match:  [subscribe foobar 1]
Pattern match:  [psubscribe foo* 3]

So that's getting triggered because of the car/subscribe and car/psubscribe calls. Does that make sense?

ozjongwon commented 9 years ago

To me it is a surprise that subscribes can trigger handlers. On 25/11/2014 11:22 PM, "Peter Taoussanis" notifications@github.com wrote:

Reopened #116 https://github.com/ptaoussanis/carmine/issues/116.

— Reply to this email directly or view it on GitHub https://github.com/ptaoussanis/carmine/issues/116#event-198234378.

ptaoussanis commented 9 years ago

To me it is a surprise that subscribes can trigger handlers.

That's an important capability to have - you may want to perform a setup/teardown procedure when subscriptions come and go.

The message format is described under "Format of pushed messages": http://redis.io/topics/pubsub.

ozjongwon commented 9 years ago

It was my ignorance on Redis, thanks for the explanation.

For 1 What about alternative design which shows hints about messages? I.e., different handlers for each type.

(def listener
  (car/with-new-pubsub-listener (:spec server1-conn)
    {["foobar" "subscribe"]     subscribe-handler
     ["foobar" "unsubscribe"]   unsubscribe-handler
     ["foobar" "message"]       message-handler

     ;; OR
     "foobar"                   message-handler
     "foo*"                     foo*-message-handler}
   (car/subscribe  "foobar" "foobaz")
   (car/psubscribe "foo*")))

For 2 In the case of timeout, probably connection should be re-opened automatically for subscribe listeners. Now I understand what's going on but when I first hit this problem, it was so strange because I didn't have any issues with the connection spec until that moment.

To me, the current pub/sub machinery of Carmine has a mismatch problem between 'how it looks' and 'how it works'. So I have to dig into source code to figure out how to use. (maybe it is just me?) (I saw some tickets on pub/sub, so probably you may have some improvement ideas in the future)

Thank for the kind explanation!

ptaoussanis commented 9 years ago

What about alternative design which shows hints about messages?

The current design provides messages of the form: [<source> <channel-id> <message-content>].

You can check and/or route by the <source>, which will be "message" or "pmessage" for "publish" command messages. The Redis docs go into detail about this format.

So if you want to ignore subscribe/unsubscribe information, just ignore messages whose <source> is "subscribe" or "unsubscribe". Does that make sense?

In the case of timeout, probably connection should be re-opened automatically for subscribe listeners.

There are use-cases for a Pub/Sub listener + timeout with the current behaviour. If you don't want the Pub/Sub connection to close, you can leave off the :timeout option. I'll mod the Pub/Sub docstrings to make it clearer that you probably don't want a :timeout option there - appreciate the suggestion :-)

Does that help?

Cheers!

ptaoussanis commented 9 years ago

Updated docstrings here: https://github.com/ptaoussanis/carmine/commit/66fc1bcae02f41b2d3fdf0ca673563e7c238b9bd

ozjongwon commented 9 years ago

On 26/11/14 17:01, Peter Taoussanis wrote:

What about alternative design which shows hints about messages?

The current design provides messages of the form: |[

]|. You can check and/or route by the ||, which will be "message" or "pmessage" for "publish" command messages. The Redis docs http://redis.io/topics/pubsub go into detail about this format. So if you want to ignore subscribe/unsubscribe information, just ignore messages whose || is "subscribe" or "unsubscribe". Does that make sense?

I had read Redis docs after your explanation (which Carmine does not work the way I expected), and I knew it. My suggestion is providing a clear declarative style handler construction can be better than providing documentation (in this case Redis doc to use Carmine).

In the case of timeout, probably connection should be re-opened
automatically for subscribe listeners.

There are use-cases for a Pub/Sub listener + timeout with the current behaviour. If you don't want the Pub/Sub connection to close, you can leave off the :timeout option. I'll mod the Pub/Sub docstrings to make it clearer that you probably don't want a :timeout option there - appreciate the suggestion :-)

Does that help?

That raises a curiosity; Without timeout option, if there is an exception (like one from a bogus message) does the listener still works for later valid 'publish' messages?

I guess that's all my concern. Thx!

ptaoussanis commented 9 years ago

That raises a curiosity; Without timeout option, if there is an exception (like one from a bogus message) does the listener still works for later valid 'publish' messages?

Sure, the listener traps + logs exceptions, so the listener will continue to work.

ozjongwon commented 9 years ago

On 26/11/14 22:30, Peter Taoussanis wrote:

That raises a curiosity; Without timeout option, if there is an
exception (like one from a bogus message) does the listener still
works
for later valid 'publish' messages?

Sure, the listener traps + logs exceptions, so the listener will continue to work.

— Reply to this email directly or view it on GitHub https://github.com/ptaoussanis/carmine/issues/116#issuecomment-64570082.

Right. I thought the timeout and some error are at the same level and could stop sub/pub machinery.

Thanks for all the answers!

ptaoussanis commented 9 years ago

No problem, happy if I was of some assistance. Cheers! :-)