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

Adds basic PoC of new worker type #1222

Closed
wants to merge 3 commits into from

Conversation

bmbouter
Copy link
Member

@bmbouter bmbouter commented Mar 31, 2021

This PR is very unpolished, but here is the state of things:

Done to a working point:

  • Fallback support allowing this code to work with RQ style workers
  • Introduction of the USE_NEW_WORKER_TYPE setting
  • The puplcore-worker entrypoint
  • The heartbeating of workers into the status API the same as before
  • Finding the next safe task
  • Correctly using advisory locks to synchronize exclusive access to
    resources, and tested locks are released upon worker exit.
  • transitioning of task states from running, completed, and failed
  • calling into the task code itself

Not done:

  • The distributed worker cleanup. Right now the workers do not watch
    each other.
  • worker forking
  • general docs
  • changelogs
  • setting the Task.parent_task when a task is dispatched from within a
    task.

The "recording of task reserved resources" to be shown to users in
TaskSerializer is a huge mess. This PR basically avoids that entirely by
introducing Task.new_reserved_resources_record. The hope is that once
this worker style is the only type Pulp uses we can delete the 4-ish
models providing the "ReservedResource..." models.

However! The TaskSerializer currently is not integrated with
Task.new_reserved_resources_record.

Also I used the not-recommended hashtext postgresql function to
transform the string to 64-bit data for the advisory locks. It works
well, but it's not safe in the long-term because it's an internal to
postgresql. It was fast and easy for this PoC though. The blake2
implementation I have on the ticket wasn't working so I moved on for
now.

@pulpbot
Copy link
Member

pulpbot commented Mar 31, 2021

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

@bmbouter bmbouter force-pushed the tasking-system-redesign branch 3 times, most recently from f11cf41 to a25d292 Compare April 2, 2021 16:39
The use of non-JSON serializable `args` and `kwargs` is now deprecated.

closes #8505
This adds a new `pulpcore.plugin.tasking.dispatch` interface which will
replace the `pulpcore.plugin.tasking.enqueue_with_reservation`
interface. This also deprecates the
`pulpcore.plugin.tasking.enqueue_with_reservation` and causes it to
emit warnings if used.

Additionally the `pulpcore.plugin.viewsets.OperationPostponedResponse`
has been ported to support both the `dispatch` and
`enqueue_with_reservation` interfaces.

closes #8496
@bmbouter bmbouter force-pushed the tasking-system-redesign branch 2 times, most recently from 1dc2e7f to 50ed95f Compare April 2, 2021 20:36
@bmbouter bmbouter changed the title asdf Adds basic PoC of new worker type Apr 2, 2021
name=f"{func.__module__}.{func.__name__}",
args=args_as_json,
kwargs=kwargs_as_json,
parent_task=None, # TODO implement me!
Copy link
Member Author

Choose a reason for hiding this comment

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

This needs resolution.

This PR is very unpolished, but here is the state of things:

Done to a working point:
* Fallback support allowing this code to work with RQ style workers
* Introduction of the ` USE_NEW_WORKER_TYPE` setting
* The `puplcore-worker` entrypoint
* The heartbeating of workers into the status API the same as before
* Finding the next safe task
* Correctly using advisory locks to synchronize exclusive access to
  resources, and tested locks are released upon worker exit.
* transitioning of task states from running, completed, and failed
* calling into the task code itself

Not done:
* The distributed worker cleanup. Right now the workers do not watch
  each other.
* worker forking
* general docs
* changelogs
* setting the Task.parent_task when a task is dispatched from within a
  task.

The "recording of task reserved resources" to be shown to users in
TaskSerializer is a huge mess. This PR basically avoids that entirely by
introducing `Task.new_reserved_resources_record`. The hope is that once
this worker style is the only type Pulp uses we can delete the 4-ish
models providing the "ReservedResource..." models.

However! The TaskSerializer currently is *not* integrated with
`Task.new_reserved_resources_record`.

Also I used the not-recommended `hashtext` postgresql function to
transform the string to 64-bit data for the advisory locks. It works
well, but it's not safe in the long-term because it's an internal to
postgresql. It was fast and easy for this PoC though. The blake2
implementation I have on the ticket wasn't working so I moved on for
now.
@mdellweg
Copy link
Member

This is superseeded by #1261

@mdellweg mdellweg closed this Apr 20, 2021
@bmbouter bmbouter deleted the tasking-system-redesign branch June 24, 2021 18:17
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.

None yet

3 participants