Skip to content

fix(ipc): update the active-job gauge in the thread job executor - #7920

Open
BlueX888 wants to merge 2 commits into
livekit:mainfrom
BlueX888:fix/active-job-count-thread-executor
Open

BlueX888 wants to merge 2 commits into
livekit:mainfrom
BlueX888:fix/active-job-count-thread-executor

Conversation

@BlueX888

Copy link
Copy Markdown
Contributor

fix(ipc): update the active-job gauge in the thread job executor

What breaks

lk_agents_active_job_count never moves for the thread job executor: the gauge reports 0 while a job is running, and stays 0 after it ends. ThreadJobExecutor never calls telemetry.metrics.job_started() / job_ended(), unlike ProcJobExecutor which calls them in launch_job and its supervise task.

The thread executor is the default on Windows (worker.py falls back to JobExecutorType.THREAD) and is forced by the devmode CLI, so on those deployments the active-job metric is permanently zero β€” dashboards and autoscaling rules that read it see an idle worker while jobs are running.

Root cause

launch_job() (livekit-agents/livekit/agents/ipc/job_thread_executor.py) starts the runner and sends the StartJobRequest without touching the metrics module, and _main_task() completes after _join_fut without decrementing. The gauge is therefore only ever updated by the process executor path.

Fix

Mirror ProcJobExecutor exactly:

  • launch_job(): call metrics.job_started() once the initialized check passes; if sending the StartJobRequest fails, reset _running_job/_job_status and call metrics.job_ended() before re-raising (no leaked increment).
  • _main_task(): after await self._join_fut, if a job was running, call metrics.job_ended().

Tests

New test tests/test_ipc.py::test_thread_executor_updates_active_job_metric spies on telemetry.metrics.job_started/job_ended, runs a real ThreadJobExecutor through _create_thread_proc, and asserts the gauge is incremented while the entrypoint is live and decremented after aclose().

  • On base: fails β€” AssertionError: job_started must be called while a job is running (both metric functions never called; the job provably runs).
  • With the fix: passes; tests/test_ipc.py + tests/test_ipc_preload.py green (31 passed); ruff clean.

The test is marked no_concurrent (it monkeypatches process-global metric functions) so parallel thread-executor tests cannot pollute the spy. The change does not overlap the in-flight #7899 hunks in the same file (initialize()/aclose()/_monitor_task()); both patches apply independently.

ThreadJobExecutor never called telemetry.metrics.job_started()/job_ended(),
so lk_agents_active_job_count had no writer when jobs ran under
JobExecutorType.THREAD (the Windows default and the console/Jupyter path).
The gauge's only writers were ProcJobExecutor.launch_job/_supervise_task,
leaving the metric at 0 (and the labeled series absent entirely) for the
whole lifetime of thread-executed jobs.

Mirror the process executor's accounting: increment when a job is launched,
roll back the increment if the StartJobRequest cannot be sent, and decrement
once the runner thread exits while a job was assigned.

Add a regression test that spies on job_started()/job_ended() and asserts
the thread executor calls them around a job lifecycle.
@BlueX888
BlueX888 requested a review from a team as a code owner October 18, 2026 12:20

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 2 potential issues.

Devin Review

Comment on lines +280 to +283
except Exception:
self._running_job = None
self._job_status = None
metrics.job_ended()

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.

🟑 Failed dispatch double-decrements active jobs

If the runner exits during launch_job's send, _main_task can decrement the gauge before the send fails. The exception handler decrements it again, leaving a negative active-job count.

Learn more

The active-job gauge counts jobs across executors, so each increment needs exactly one decrement. A failed send can overlap with the runner's exit: the join callback wakes _main_task, which observes _running_job and calls job_ended. If the pending send then raises, this handler decrements the same job again. The resulting negative gauge persists and offsets counts from other jobs.

Example: A runner exits while the parent is awaiting the send of job A. The join handler reduces the count from 1 to 0; the send failure handler then reduces it from 0 to -1.

Recommended fix: Track whether the increment remains owned by launch_job or has been settled by _main_task, and make the decrement idempotent across both paths. Include a test that pauses a failing send until after the join callback runs.

Devin Review


Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

Comment on lines +278 to +280
try:
await channel.asend_message(self._pch, start_req)
except Exception:

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.

🟑 Cancelled dispatch leaves a phantom job

Cancelling launch_job during the send bypasses its Exception handler, leaving the gauge incremented and _running_job set. If the runner remains idle, the active-job count stays high until it exits.

Learn more

The parent writes a serialized job request and awaits the channel drain. Cancellation at that await raises asyncio.CancelledError, which the new except Exception does not catch. If cancellation happens before the request is delivered, the runner remains idle while the incremented gauge counts a job. _main_task only decrements it when the runner exits.

Example: A caller cancels dispatch for job A while its socket is draining and the runner has not received the complete request. No job starts, yet the gauge remains at 1 until the runner is closed.

Recommended fix: Define cancellation semantics for sends that may already have delivered a request. Either shield dispatch until its outcome is known, or handle cancellation while closing the executor and settling the gauge exactly once after the runner exits; avoid marking a potentially delivered job as unstarted.

Devin Review


Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

The fake job's entrypoint could complete between launch_job() and the
'started' assertion, letting _main_task record 'ended' first. CI hit that
window; keep the entrypoint parked on a threading.Event until the test has
observed the running state, then release it and assert the final sequence.

This branch has not been deployed

No deployments
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