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

[QOLSVC-4633] allow configuration of multiple target queues #98

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

ThrawnCA
Copy link

  • Select a queue by hashing the package ID, so jobs for the same package will always go to the same queue to minimise deadlock risks.
  • Also add the queue name to the job metadata so retries can remain on the same queue.

- Select a queue by hashing the package ID, so jobs for the same package will always go to the same queue
to minimise deadlock risks.
- Also add the queue name to the job metadata so retries can remain on the same queue.
@ThrawnCA ThrawnCA requested a review from a team June 14, 2024 01:36
Copy link
Member

@duttonw duttonw left a comment

Choose a reason for hiding this comment

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

How does this work if there is two workers on the same queue? Ie two boxes stood up.

@ThrawnCA
Copy link
Author

How does this work if there is two workers on the same queue? Ie two boxes stood up.

Then there is still a possibility that they will clash. Can't really be helped.

I suppose it would be possible to configure the two boxes to use different sets of queues, if you really wanted to go that far.

@duttonw
Copy link
Member

duttonw commented Jun 14, 2024

Lets take it back a level, we want to follow the KISS principal. Since we are getting into lock issues and xloader and co already take a lock on the dataset (not just the resource). Should we not look at a way to flag that the dataset is being worked on before a worker starts processing and checks the job to the back of the queue to go through the other resources quicker.

Also the issue we are running into may not be based on xloader only but also the validation scanner and other job queued products + the main thread altering the resource/s if its a upload or a re-ordering.

@ThrawnCA
Copy link
Author

Since we are getting into lock issues and xloader and co already take a lock on the dataset (not just the resource). Should we not look at a way to flag that the dataset is being worked on before a worker starts processing and checks the job to the back of the queue to go through the other resources quicker.

That would likely be much more complex to implement in a robust fashion, actually.

Also the issue we are running into may not be based on xloader only but also the validation scanner and other job queued products + the main thread altering the resource/s if its a upload or a re-ordering.

Being able to customise the queue name from "default" to something else still has value.

@ThrawnCA ThrawnCA requested a review from a team June 26, 2024 02:56
@@ -128,6 +128,13 @@ groups:
to True.
type: bool
required: false
- key: ckanext.xloader.queue_names
Copy link
Member

Choose a reason for hiding this comment

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

Add to the readme and lets get it included.

Copy link
Member

@duttonw duttonw left a comment

Choose a reason for hiding this comment

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

Docs need updating

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

Successfully merging this pull request may close these issues.

2 participants