allow batch enqueueing of jobs using Redis Pipelining #154
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using Redis Pipelining is much faster than one-by-one enqueueing since there is a single Round-Trip-Time for the entire batch, instead of one RTT for each job. This improves the performance of enqueueing a large number of jobs considerably. I decided against splitting large batches into multiple chunks (as recommended by https://redis.io/topics/pipelining#redis-pipelining), but mentioned it in the JavaDoc of this method. Thus, it is up to the user of this library to decide whether he wants to consider chunking the batch.
Not once per job in the list. Thus, I refactored the validation methods to be smaller. I decided against inlining `validateArguments()` method since I am not sure whether this is in line with the design principle of the author of this library.
|
This PR supersedes #153. Sorry for the confusion. Please review and merge. Thanks for the great work! |
|
can you maybe please tell how much faster that solution is? |
|
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 :-). |
|
Thanks for the PR! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
validatemethods to be able to validate the queue name parameter independently of a job parameter.Closes #123