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

Improve throughput for immediate tasks (1/3) #5841

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pedro-psb
Copy link
Member

@pedro-psb pedro-psb commented Sep 25, 2024

In busy tasking environments, "immediate" running tasks (which are only dispatched to workers due to resource locking requirements) can wait for very long behind long-running tasks, such as syncs and publishes.

This tries to solve this implementing the solution proposed in #5767, as discussed in the Pulp team.

@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch from 7fb9d32 to d18b071 Compare September 25, 2024 21:36
@pedro-psb pedro-psb changed the title Rrun immediate tasks on worker parent process Improve throughput for immediate tasks (1/3) Sep 26, 2024
Comment on lines 349 to 352
.first()
)
with contextlib.suppress(AdvisoryLockError), task:
task.refresh_from_db()
Copy link
Contributor

Choose a reason for hiding this comment

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

.first() can return None. Will this not error trying to call a method on None?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, nice catch.
But anyway, this was a sketch of the approach I had in mind, which won't be used.
I force pushed with the ideas I discussed with @mdellweg yesterday.

@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch 2 times, most recently from 9ab7302 to 589053b Compare September 27, 2024 20:46
@pedro-psb
Copy link
Member Author

(there is a refator and a impl commit)

@pedro-psb pedro-psb marked this pull request as ready for review September 28, 2024 17:02
@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch from 589053b to b417979 Compare September 30, 2024 12:11
@pedro-psb pedro-psb marked this pull request as draft September 30, 2024 17:45
@pedro-psb
Copy link
Member Author

pedro-psb commented Oct 1, 2024

Curious how the tests passed on the lowerbound but got stuck in the others scenarios 🤔
https://github.com/pulp/pulpcore/actions/runs/11112991180/job/30876360344

@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch 3 times, most recently from ce087b4 to ff396e0 Compare October 2, 2024 12:44
@pedro-psb pedro-psb marked this pull request as ready for review October 2, 2024 13:25
@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch from ff396e0 to 40fd2a0 Compare October 2, 2024 17:31
@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch 2 times, most recently from d710146 to 697f97a Compare October 2, 2024 17:37
* immediate prioritization

This defines tasks of the type "immediate" by adding a Task.immediate(bool)
field to the Task model. These type of task are ordered first in the
worker's query to pick up a new available task.

* misc

This also adds a "deferred" field to the Task model, so we can later
harden the constraint that 'deferred == False' and immediate == True'
shouldn't get past the API level. This is possible today in some
edge case failure scenarios on dispatch.

Closes pulp#5767
@pedro-psb pedro-psb force-pushed the feat/run-immediate-tasks-on-parent branch from 697f97a to ef6ddb0 Compare October 2, 2024 17:45
@pedro-psb
Copy link
Member Author

Ready to review.

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

Successfully merging this pull request may close these issues.

2 participants