Skip to content

Lookout pruner: repair lease-returned/lease-expired zombie jobs - #5058

Open
mauriceyap wants to merge 5 commits into
masterfrom
lookout=pruner-lease-returned
Open

Lookout pruner: repair lease-returned/lease-expired zombie jobs#5058
mauriceyap wants to merge 5 commits into
masterfrom
lookout=pruner-lease-returned

Conversation

@mauriceyap

Copy link
Copy Markdown
Collaborator

The zombie-job reconciler only handled runs that reached an unconditionally terminal state (SUCCEEDED/FAILED/CANCELLED/PREEMPTED). Jobs whose latest run was LEASE_RETURNED or LEASE_EXPIRED were left unhandled, so if the expected follow-up event (a requeue or failure) was lost by the ingester, those jobs stayed stuck showing a non-terminal state forever.

This adds LEASE_RETURNED/LEASE_EXPIRED to the reconciler, mapping them conservatively to FAILED. Because these run states are normally followed by a legitimate scheduler decision that can take much longer than ordinary ingester lag, they use their own grace period (leaseReturnedZombieRepairThreshold which defaults to 3h) rather than sharing zombieRepairThreshold (which defaults to 15m).

Also adds a diagnostic count/log for zombie-shaped jobs the reconciler can't repair because their run has no finished timestamp.

The zombie-job reconciler only handled runs that reached an unconditionally terminal state (SUCCEEDED/FAILED/CANCELLED/PREEMPTED). Jobs whose latest run was LEASE_RETURNED or LEASE_EXPIRED were left unhandled, so if the expected follow-up event (a requeue or failure) was lost by the ingester, those jobs stayed stuck showing a non-terminal state forever.

This adds LEASE_RETURNED/LEASE_EXPIRED to the reconciler, mapping them conservatively to FAILED. Because these run states are normally followed by a legitimate scheduler decision that can take much longer than ordinary ingester lag, they use their own grace period (`leaseReturnedZombieRepairThreshold` which defaults to 3h) rather than sharing `zombieRepairThreshold` (which defaults to 15m).

Also adds a diagnostic count/log for zombie-shaped jobs the reconciler can't repair because their run has no finished timestamp.

Signed-off-by: Maurice Yap <mauriceyap@hotmail.co.uk>
@mauriceyap
mauriceyap enabled auto-merge (squash) July 28, 2026 09:52
Comment thread internal/lookout/pruner/reconcile_zombies.go Outdated
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends Lookout zombie reconciliation to lease-returned and lease-expired runs.

  • Adds an independently configurable three-hour repair threshold for these run states.
  • Maps qualifying stale jobs to FAILED and records diagnostics for candidates lacking a finished timestamp.
  • Adds requeue safeguards, UTC-normalized cutoffs, simulator support, and reconciliation tests.

Confidence Score: 4/5

This PR is not yet safe to merge because a legitimate requeue can still be changed to FAILED when executor and scheduler clocks order its timestamps differently.

The strict comparison fixes equal timestamps and normal later requeues, but it assumes run.finished and job.last_transition_time share a clock domain even though lease-returned and requeue events can be timestamped by different machines.

Files Needing Attention: internal/lookout/pruner/reconcile_zombies.go

Important Files Changed

Filename Overview
internal/lookout/pruner/reconcile_zombies.go Adds lease-returned/expired repair and a requeue timestamp guard, but the guard can misclassify requeues timestamped by a slower scheduler clock.
internal/lookout/pruner/pruner.go Passes the independent threshold into reconciliation and normalizes deletion cutoffs to UTC.
cmd/lookout/main.go Loads the new threshold with a three-hour default and passes it into the pruner.
internal/lookout/configuration/types.go Defines and documents the optional lease-returned zombie repair threshold.
internal/lookout/pruner/reconcile_zombies_test.go Covers repair, grace periods, ordinary and equal-timestamp requeues, UTC clocks, and null-finished diagnostics.

Sequence Diagram

sequenceDiagram
    participant E as Executor
    participant LI as Lookout Ingester
    participant S as Scheduler
    participant P as Lookout Pruner
    E->>LI: Lease returned (executor timestamp)
    LI->>LI: Store run.finished
    S->>LI: JobRequeued (scheduler timestamp)
    LI->>LI: Set job QUEUED and last_transition_time
    Note over LI: latest_run_id still references returned run
    P->>LI: Reconcile after grace period
    P->>LI: "Compare last_transition_time < finished"
    LI-->>P: Cross-clock ordering may match
    P->>LI: Set job FAILED
Loading

Reviews (4): Last reviewed commit: "Merge branch 'master' into lookout=prune..." | Re-trigger Greptile

Signed-off-by: Maurice Yap <mauriceyap@hotmail.co.uk>
Comment thread internal/lookout/pruner/reconcile_zombies.go Outdated
Signed-off-by: Maurice Yap <mauriceyap@hotmail.co.uk>
Signed-off-by: Maurice Yap <mauriceyap@hotmail.co.uk>
)
LIMIT $3
) AS mapping
WHERE job.job_id = mapping.job_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Cross-clock requeue ordering fails

When an executor timestamps a lease return ahead of the scheduler clock used for the subsequent legitimate JobRequeued event, last_transition_time < finished remains true. Because latest_run_id still references the returned run, the pruner changes the queued job to FAILED while it is waiting for another lease.

Knowledge Base Used:

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.

1 participant