Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Named queues (configurable redis key prefix) #274

Closed
wants to merge 3 commits into from
Closed

Named queues (configurable redis key prefix) #274

wants to merge 3 commits into from

Conversation

seriallos
Copy link

Modified the Redis keys that Kue uses to make the prefix configurable. My two use cases:

  • Run functional tests of my application which uses Kue in isolation
  • Allow multiple applications that use Kue to use the same Redis instance without polluting each other.

redis.queueName ends up behaving similarly to redis.createClient - override it once for your application and everything should just work.

I built this off the develop branch as I needed some of the functionality that's been cropping up there.

Also, thanks for keeping this moving, behrad! Good to see changes coming along!

Dave Hendler added 3 commits January 15, 2014 23:22
Experimental change to allow an application using Kue to only process jobs with
the configured queue name.

This change was prompted by a couple needs:

* Use a single Redis instance for multiple applications (for test environments)
* Run automated job test code using the same Redis as a running application
  without interfering with the active app.
@behrad
Copy link
Collaborator

behrad commented Jan 25, 2014

Nice idea, but keeping queueName in redis module doesn't feel good to me. (As of 0.7.0 createQueue accepts parameters)
Why don't use different redis databases for this? more manageable? no key space pollution? huh...!?

@seriallos
Copy link
Author

I'll take a look at the updated createQueue and see what I can do. I initially tried to keep track of this on the main Queue object but at the time it wasn't available in all of the places where redis calls were made, IIRC.

In looking around, redis databases are basically deprecated and shouldn't be depended upon for the future.

Running multiple redis instances on different ports is possible but annoying to manage, especially for work where we have dozens of dev VMs and a bunch of different environments.

The named queues are super nice for having isolated tests. Every functional test we write uses a random named queue (test24345, test28954, etc) which is great because if a test or test cleanup fails, subsequent tests are using a completely new queue without any previous jobs. We're using this to ensure that Kue jobs can properly create subsequent jobs and verify that they complete/fail as expected.

@behrad
Copy link
Collaborator

behrad commented Jan 25, 2014

Queue isolation is something super nice for sure. I didn't know about
deprecation of redis databases (valid link?)
On name prefixes, I recommend adding another level to keys showing queue
name, something like q:test24345:jobs so that there always exists a q:
global prefix for Kue. This sounds better to me.

@behrad
Copy link
Collaborator

behrad commented Jan 25, 2014

and what about the UI? Have u looked at the web code to find it is simple
enough to add support for showing different queue namespaces?

@seriallos
Copy link
Author

Hard to find the original link, but here are some discussions about how redis cluster won't support multiple databases at all and that multiple databases for a single redis instance is not a good choice:

https://groups.google.com/forum/#!topic/redis-db/x3-6GByC3xE

http://stackoverflow.com/questions/16221563/whats-the-point-of-multiple-redis-databases

https://code.google.com/p/redis/issues/detail?id=662

@seriallos
Copy link
Author

Different queues need to run the express app on a different port. It's not intended to have one instance of Kue with multiple named queues, is to have multiple applications be able to use the same Redis instance.

Should probably change the name to "Redis Key Prefix" instead of "Named Queues" to make it less ambiguous as to what the main purpose is.

@seriallos
Copy link
Author

Looking at the code, I don't think there's an easy way to have kue.js track the redis prefix. queue/job.js requires redis but doesn't require kue.js or have an instance of Kue passed in (looks like this problem has a TODO attached to the disableSearch flag as well). Same thing is true of queue/events.js.

Thinking about this features as "what redis key prefix should I use", it doesn't smell too bad to me to have it hang off of redis.js the way the code is currently structured. In the same way that an application can define "how do I connect to Redis" they can define "what key prefix do I want to use for redis".

@behrad
Copy link
Collaborator

behrad commented Jan 25, 2014

That way of defining "how do I connect to Redis" is actually prohibited. "Global Singletons" are bad and inflexible! As you may noticed I tried to deprecate that by adding redis options to Queue.createQueue. So I don't think thats a good idea to add another point of the same kind.

@seriallos
Copy link
Author

Totally agree, I hadn't seen the updated redis configuration - definitely much better!

Do you have plans on passing the active Queue into workers and jobs or allowing them to access data from the Queue? And/or cleaning up how the various objects communicate with Redis instead of using the global redis export?

Until that's done, I think I would have do perform the same ugly stuff gong on with disableSearch like this:
if( !require('../kue').singleton.disableSearch ){

@seriallos
Copy link
Author

Closing this out. I have another pull request with a slightly better implementation that I'll submit sometime this week.

@seriallos seriallos closed this Apr 15, 2014
@0xgeert
Copy link

0xgeert commented May 20, 2014

Are named queues implemented now?

@seriallos
Copy link
Author

#325 is an updated version of this pull request. Note that it only handles a single named queue / redis key prefix per process. Don't think anyone has looked at multiple queues in one process.

@0xgeert
Copy link

0xgeert commented May 20, 2014

Note that it only handles a single named queue / redis key prefix per process.

What does this mean? Can a single producer only put jobs on 1 queue, or can a subscriber only listen to 1 queue?

@seriallos
Copy link
Author

It's not really named queues which is why the other pull request has a different name. The primary intent was twofold: 1) allow multiple applications to use one Redis instance without conflicts (before my PR, Kue always used the q: key prefix so two apps using different jobs would see the other app's jobs) and 2) allow one process to rapidly create/destroy singletons with different redis key prefixes for testing isolation.

@behrad
Copy link
Collaborator

behrad commented May 21, 2014

Are named queues implemented now?

Not yet, this is queued to be added after fixing some global initializations done regarding to createQueue

@behrad
Copy link
Collaborator

behrad commented Jul 13, 2014

This should be working from 0.8.1 and later....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants