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

Add GenericEnqueuer for consistent job priorities #4237

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

Conversation

johha
Copy link
Contributor

@johha johha commented Feb 28, 2025

Some delayed jobs enqueue other delayed jobs. For example, an app delete job may enqueue secondary jobs like blobstore deletion and buildpack cache cleanup.
If CAPI is configured with dedicated job priorities or dynamic job priorities is enabled, these secondary jobs might unintentionally receive a higher priority than their primary job. This could lead to less critical jobs, like blobstore deletion, being processed before more critical ones, such as service instance creation.

This change introduces GenericEnqueuer, a singleton enqueuer ensuring that all jobs enqueued within the same job execution context inherit the same priority.
It is automatically initialized and destroyed within the CCJob wrapper, ensuring consistent priority propagation. Jobs can now be enqueued using:
GenericEnqueuer.shared.enqueue(job)

  • Priority Incrementation:
    • enqueue and enqueue_pollable now support an optional priority_increment parameter.
    • This increases the priority value (making the job less important), allowing secondary jobs (e.g., blobstore deletion) to have a lower priority than their parent job.
  • Preserving Priority for Re-enqueued Jobs:
    • The preserve_priority flag ensures that re-enqueued jobs retain their previous priority.
    • Prevents unnecessary priority increases on repeated executions.
  • Config Priority Handling:
    • If a job has a dedicated priority configured, it is added to the current priority and priority_increment, ensuring correct propagation.
Job Type Preserve Priority Config Priority Increment Final Priority
Parent Job ❌ No 100 nil 100
Sub-Job A ❌ No 200 50 350 (100+200+50)
Sub-Job B ❌ No nil 50 150 (100+50)
Sub-Job C ❌ No nil nil 100 (inherits parent)
Tertiary Job A1 ❌ No 50 20 420 (350+50+20)
Tertiary Job B1 ✅ Yes 10 (ignored) 30 150 (preserved from Sub-Job B)
Tertiary Job C1 ❌ No nil 50 150 (100+50)
Re-enqueued Job ✅ Yes 20 (ignored) 100 42 (original preserved priority)
  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@johha johha force-pushed the priority-inheritance branch 3 times, most recently from 4c3a938 to c703fc7 Compare March 3, 2025 13:26
Some delayed jobs enqueue other delayed jobs. For example, an app delete job
may enqueue secondary jobs like blobstore deletion and buildpack cache cleanup.
If CAPI is configured with dedicated job priorities or dynamic job priorities
is enabled, these secondary jobs might unintentionally receive a higher priority
than their primary job. This could lead to less critical jobs, like blobstore deletion,
being processed before more critical ones, such as service instance creation.

This change introduces `GenericEnqueuer`, a singleton enqueuer ensuring that all
jobs enqueued within the same job execution context inherit the same priority.
It is automatically initialized and destroyed within the CCJob wrapper, ensuring
consistent priority propagation. Jobs can now be enqueued using:
`GenericEnqueuer.shared.enqueue(job)`

- **Priority Incrementation:**
  - `enqueue` and `enqueue_pollable` now support an optional `priority_increment` parameter.
  - This increases the priority value (making the job less important), allowing
    secondary jobs (e.g., blobstore deletion) to have a lower priority than their parent job.
- **Preserving Priority for Re-enqueued Jobs:**
  - The `preserve_priority` flag ensures that re-enqueued jobs retain their previous priority.
  - Prevents unnecessary priority increases on repeated executions.
- **Config Priority Handling:**
  - If a job has a dedicated priority configured, it is added to the current priority
    and `priority_increment`, ensuring correct propagation.

| Job Type               | Preserve Priority | Config Priority | Increment | Final Priority                       |
|------------------------|-------------------|-----------------|-----------|--------------------------------------|
| **Parent Job**         | ❌ No             | `100`           | `nil`     | **100**                              |
| **Sub-Job A**          | ❌ No             | `200`           | `50`      | **350** (100+200+50)                 |
| **Sub-Job B**          | ❌ No             | `nil`           | `50`      | **150** (100+50)                     |
| **Sub-Job C**          | ❌ No             | `nil`           | `nil`     | **100** (inherits parent)            |
| **Tertiary Job A1**    | ❌ No             | `50`            | `20`      | **420** (350+50+20)                  |
| **Tertiary Job B1**    | ✅ Yes            | `10` (ignored)  | `30`      | **150** (preserved from Sub-Job B)   |
| **Tertiary Job C1**    | ❌ No             | `nil`           | `50`      | **150** (100+50)                     |
| **Re-enqueued Job**    | ✅ Yes            | `20` (ignored)  | `100`     | **42** (original preserved priority) |
@johha johha force-pushed the priority-inheritance branch from c703fc7 to 7567ab6 Compare March 3, 2025 14:12
@johha johha marked this pull request as ready for review March 3, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant