This repository was archived by the owner on Apr 30, 2024. It is now read-only.
Queue groups with random distribution#102
Open
tylertreat-wf wants to merge 20 commits intoWorkiva:masterfrom
Open
Queue groups with random distribution#102tylertreat-wf wants to merge 20 commits intoWorkiva:masterfrom
tylertreat-wf wants to merge 20 commits intoWorkiva:masterfrom
Conversation
This adds support for queue groups when creating an Async.
We're no longer asynchronously fetching queue statistics.
This allows us to fall back to an appropriate default queue when something goes wrong during queue selection for both Messages and Asyncs.
Temporarily cache the QueueStatistics to reduce RPCs.
Selecting an optimal queue from a group via the taskqueue API is looking like it will add a lot of code, so for now let's keep things lightweight and only use random queue selection.
There's no reason to have this extra check since it will be handled without issue.
Contributor
Author
|
This is for issue #100 |
furious/async.py
Outdated
Contributor
There was a problem hiding this comment.
This could just be if not group_queues:
furious/async.py
Outdated
Contributor
There was a problem hiding this comment.
Wonder if we should have a furious.queues module where this queue code can live?
Probably make it simpler to leverage with other queue systems as well.
The queues module will be used for any queue selection logic and dealing with queue groups.
The queue counts indicate the number of queues a queue group has. Rather than passing this information into select_queue, we will make in configurable in furious.yaml.
We are no longer passing in queue counts.
furious/queues.py
Outdated
There was a problem hiding this comment.
I would say if we're splitting this into two lines, we may as well test that queue_counts is in fact a dict as well. Otherwise this could blow up. Having a meaningful message as to why the exception is raised would be good.
…ueue-groups-random
This adds an additional check in select_queue to ensure the configured queue_counts property is a dictionary.
Piggyback off of _local.get_local_context() to store queue group caching instead of creating a separate thread local variable.
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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 adds support for queue groups from Asyncs and Messages. The initial plan was to include an option for optimal queue selection by leveraging the taskqueue API, but it was turning into more code than I had hoped. In an effort to keep things lightweight, I've opted for just using random (but equal) queue distribution, loosely based on deferred's implementation.
I may revisit optimal queue selection to try and find a good way of doing it. The main issue is handling the case where lots of Asyncs are added in a context.
@robertkluin-wf @ericolson-wf @beaulyddon-wf @tannermiller-wf @johnlockwood-wf