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

Allow redefining internally used background queues #11022

Closed
kkthxbye-code opened this issue Nov 24, 2022 · 5 comments
Closed

Allow redefining internally used background queues #11022

kkthxbye-code opened this issue Nov 24, 2022 · 5 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@kkthxbye-code
Copy link
Contributor

kkthxbye-code commented Nov 24, 2022

NetBox version

v3.3.8

Feature type

Change to existing functionality

Proposed functionality

Allow changing the used RQ queue for webhook scheduling, custom scripts and reports to a specific queue.

Use case

Currently script/report and webhook execution uses the same queue ("default"). One RQ worker will only ever run one job at a time:

Each worker will process a single job at a time. Within a worker, there is no concurrent processing going on. If you want to perform jobs concurrently, simply start more workers.

For webhooks we want to only ever run one job at the time, as the order is very important to avoid a webhook referencing a related object that hasn't been sent in a webhook yet.

For scripts/reports the user might not care about the order of executions, but as it stands now if the user runs one rqworker, long running scripts will block both webhooks and other scripts from execution before it is done.

If the user starts multiple rqworkers that will allow multiple scripts to run at a time, but it will also allow multiple webhooks to be sent at a time, which in turn can create situations where a newer webhook is done processing before an older one - imagine three workers all picking up a job at the same time, the webhook will potentially send the HTTP request at roughly the same time, which one goes first depends on CPU scheduling or other external factors.

Allowing webhooks to be configured to use a seperate queue will allow the user to start one worker for webhooks and multiple workers for script/report execution, without risking data consistency issues with webhooks.

Database changes

None

External dependencies

None

@kkthxbye-code kkthxbye-code added the type: feature Introduction of new functionality to the application label Nov 24, 2022
@kkthxbye-code
Copy link
Contributor Author

kkthxbye-code commented Nov 24, 2022

If accepted I'll implement it. We also need to sync up with netbox-docker as there needs to be changes to the default docker-compose file to add the new queue.

@julianstolp
Copy link

If adding the ability to set the queue priority for webhooks, would it be possible to add the ability to set the priority in general at the URL with maybe the flag ?priority=high, ?priority=default, ?priority=low. The usecase would be to prioritize manual triggered scripts to run before the background scripts.

@kkthxbye-code
Copy link
Contributor Author

kkthxbye-code commented Nov 24, 2022

@Bierfass - This feature request would be to set a static queue for webhooks, as there in my opinion is no reason to want to concurrently execute webhooks because of the mentioned pitfalls of the ordering not being perserved.

Allowing you to schedule scripts and reports in different queues is a good idea though, but would be another feature request. It would probably either be set in the Meta class of the script or as a form selection when executing the script though. Also note that the low, default and high queues only work as intended if running more than one rqworker as far as I can tell.

@jeremystretch
Copy link
Member

Just recapping our discussion from today's maintainers' meeting. This is a great idea and I'd like to be able to configure queues per process type. For example, something like this in configuration.py:

QUEUE_MAPPINGS = {
    'webhooks': 'my-custom-queue',
    'reports': 'high',
}

This would grant users a good amount of flexibility to create and assign their own queues as needed (each of which can be serviced by however many RQ workers is appropriate).

@hagbarddenstore
Copy link
Contributor

hagbarddenstore commented Dec 1, 2022 via email

@kkthxbye-code kkthxbye-code self-assigned this Dec 5, 2022
@kkthxbye-code kkthxbye-code added the status: accepted This issue has been accepted for implementation label Dec 5, 2022
@kkthxbye-code kkthxbye-code changed the title Use a seperate queue for webhooks Allow redefining internally used background queues Dec 5, 2022
jeremystretch added a commit that referenced this issue Dec 8, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

4 participants