gresrun / jesque

An implementation of Resque in Java.
http://gresrun.github.io/jesque
Apache License 2.0
628 stars 131 forks source link

allow batch enqueueing of jobs using Redis Pipelining #154

Closed jack-kerouac closed 5 years ago

jack-kerouac commented 5 years ago

This change allows enqueueing a list of jobs with higher performance than one-by-one invoking the existing client.enqueue() method. It therefore uses Redis Pipelining: https://redis.io/topics/pipelining.

I decided against supporting splitting the list of jobs into chunks of e.g. 10,000 jobs, which is recommended by Redis ('Important Note' at https://redis.io/topics/pipelining#redis-pipelining). I mentioned this in the JavaDoc and left it to the client of this method to keep the library simple. Some clients might want to ignore this and I wanted to leave it up to the user.

This change also includes a bit of refactoring of the validate methods to be able to validate the queue name parameter independently of a job parameter.

Closes https://github.com/gresrun/jesque/issues/123

jack-kerouac commented 5 years ago

This PR supersedes https://github.com/gresrun/jesque/pull/153. Sorry for the confusion.

Please review and merge. Thanks for the great work!

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.09%) to 70.365% when pulling 02e3af9f98e0d02c284a9d6b8c97daae5ba1cb16 on tadodotcom:batch-enqueue-using-pipelining into 88bab76606af3270db630eb38c34d95bf018976f on gresrun:master.

bp-FLN commented 5 years ago

can you maybe please tell how much faster that solution is?

jack-kerouac commented 5 years ago

This mainly depends on the RTT to Redis (and of course the number of jobs).

If the RTT to Redis is 1ms, enqueueing 1000 jobs takes 1s, if done one by one. If done using the new method, it is 1ms (or a little more since Redis still needs to append to the list, but this is very fast, compared to network latency). So the speedup factor is basically equal to the number of jobs to enqueue :-).

gresrun commented 5 years ago

Thanks for the PR!