Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Task channels #62

Merged
merged 1 commit into from Apr 29, 2012

Conversation

Projects
None yet
2 participants
Contributor

mpenet commented Apr 27, 2012

Hello Zach,

In short, this allows to create channels for emitting and receiving redis tasks. Nothing revolutionary by itself but it makes the channels functions very useful to handle forwarding, broadcasting & more.

The receiver-channel will propagate a close command to its client connection, I don't think there is another way to interrupt the pending brpop command (since we dont want a message to be received later on a closed channel), let me know if you know a better way to handle this case.

Max

Owner

ztellman commented Apr 28, 2012

I'm tempted to say that these should be spliced together into a single channel that can send and receive tasks, though if you were to do the siphon echo server thing it would cause an infinite loop with Redis. Thoughts?

Contributor

mpenet commented Apr 28, 2012

I had the same idea, that makes sense (a channel should be able to receive and emit after all), but most of the time you only emit or receive data from a single code "unit" (or even host), and I figured you are only a couple of lamina calls away if you want to do something like that.
Also this would require the creation of 2 clients even though you could be only interested in a single aspect of this channel.

All the above made me think it is probably better to let the user decide how he wants to compose with these and provide enough to make it easy if needed.

Contributor

mpenet commented Apr 28, 2012

I changed my mind, it could be useful, but I think it should be an addition, not a replacement of the emitter/receiver channel. I ll give it a stab this afternoon (fun!)

Owner

ztellman commented Apr 28, 2012

I think I may lean towards not having an emitter channel, since you
can just do a (sink ...) or (receive-all ...) and directly call the
enqueue-task function. The receiver channel strikes me as something
which is much more useful to have in the core library.

That being said, don't let me stop you from implementing it, I just
don't think I'll include it in aleph.redis.

On Sat, Apr 28, 2012 at 4:08 PM, Max Penet
reply@reply.github.com
wrote:

I changed my mind, it could be useful, but I think it should be an addition, not a replacement of the emitter/receiver channel. I ll give it a stab this afternoon (fun!)


Reply to this email directly or view it on GitHub:
#62 (comment)

Contributor

mpenet commented Apr 28, 2012

True, I forgot about sink, it makes the task emitter really trivial to create. I can modify the pull request so that it only includes task-receiver-channel if that is ok with you then?

Contributor

mpenet commented Apr 28, 2012

Ok done. I went back and modified the pull request to only include task-receiver-channel and the corresponding test

the task-channel you were talking about would end up being something like this (so simple):

(defn task-channel
  [emitter-client receiver-client queue-name]
  (splice (task-receiver-channel receiver-client queue-name)
          (sink (partial enqueue-task emitter-client queue-name))))

@ztellman ztellman merged commit 49116f8 into ztellman:perf Apr 29, 2012

Owner

ztellman commented Apr 29, 2012

Hey, I made it so that the client is implicit in this case, since you can't use it for anything else if it's constantly in the middle of BLPOPs. If anything seems off to you, let me know.

Contributor

mpenet commented Apr 29, 2012

That is much better, also having the closed? check right before the command seems safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment