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

Ordered queue handling by workers #665

Merged
merged 6 commits into from
Jul 12, 2022

Conversation

jrochkind
Copy link
Contributor

@jrochkind jrochkind commented Jul 12, 2022

Just an experiment, to have some concrete code to talk about, and to give me an exersize.

See: #624

Although I'm interested in it, I also understand if you ultimately think this feature is not necessary and would like to avoid it bloating the codebase. Abstractly, you can do the same thing with priorities, although it can be less convenient to do so (in some cases extremely cumbersome), and priorities don't allow you to re-shuffle how an existing queue of jobs is handled like ordered queue workers do.

I updated the benchmarking with more clear combinations of different sorting. Using the ordered queue feature does have some slowdown; I suspect it won't matter for good_job-scale loads, and also any performance hit from this feature will only be if you use the feature, it shouldn't effect you if you don't.

If I run the benchmark multiple times, sometimes I get some differences between examples that other times same-ish for. Here are two examples.

Comparison:
sort by priority only:      337.0 i/s
       without sorts:      327.9 i/s - same-ish: difference falls within error
sort by creation only:      312.0 i/s - same-ish: difference falls within error
sort by priority and creation:      303.1 i/s - same-ish: difference falls within error
sort by ordered queues and creation:      269.9 i/s - same-ish: difference falls within error
sort by ordered queues only:      268.1 i/s - same-ish: difference falls within error
sort by ordered queues, priority, and creation:      209.1 i/s - 1.61x  (± 0.00) slower
Comparison:
sort by creation only:      386.7 i/s
sort by priority only:      364.7 i/s - same-ish: difference falls within error
       without sorts:      352.3 i/s - same-ish: difference falls within error
sort by priority and creation:      293.8 i/s - same-ish: difference falls within error
sort by ordered queues only:      284.3 i/s - 1.36x  (± 0.00) slower
sort by ordered queues and creation:      276.8 i/s - 1.40x  (± 0.00) slower
sort by ordered queues, priority, and creation:      226.2 i/s - 1.71x  (± 0.00) slower

@jrochkind jrochkind changed the title Ordered queue handling Ordered queue handling by workers Jul 12, 2022
Copy link
Owner

@bensheldon bensheldon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks great. I'm curious what happens with some random strings like +*, but I'm happy with this. 👍🏻

I made a few suggestions/fixed-typos in the docs. I'll merge those and release this.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@bensheldon bensheldon merged commit 950ef93 into bensheldon:main Jul 12, 2022
@bensheldon
Copy link
Owner

Thank you! Released in v3.2.0

@jrochkind
Copy link
Contributor Author

Thanks @bensheldon! And thanks for the doc clarifications.

I suppose +* would end up telling the pool only to pull from the queue named "*". Which, if you don't have such a queue name (and it would be strongly discouraged for you to!), will end up pulling no jobs. 🤷

Compare to if you specified -* as a queue. (I guess that would end up being the same as just *, so long as you have no queue named *!). + is being handled exactly like -, as far as parsing.

@bensheldon
Copy link
Owner

bensheldon commented Jul 13, 2022

@jrochkind agreed. I think we can ignore the edgecase semantics until someone notices 😄

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

Successfully merging this pull request may close these issues.

2 participants