Skip to content

Tracing: wait on an event instead of polling in the batch worker - #4695

Closed
rajarshidattapy wants to merge 1 commit into
openai:mainfrom
rajarshidattapy:fix/trace-processor-worker-wakeups
Closed

Tracing: wait on an event instead of polling in the batch worker#4695
rajarshidattapy wants to merge 1 commit into
openai:mainfrom
rajarshidattapy:fix/trace-processor-worker-wakeups

Conversation

@rajarshidattapy

Copy link
Copy Markdown
Contributor

Fixes #4688.

Problem

BatchTraceProcessor's export thread polled in a fixed 200 ms loop:

if current_time >= self._next_export_time or queue_size >= self._export_trigger_size:
    self._export_batches()
    self._next_export_time = time.monotonic() + self._schedule_delay
else:
    # Sleep a short interval so we don't busy-wait.
    time.sleep(0.2)

The thread starts lazily on the first trace/span and never stops, so for the rest of the process lifetime it woke five times a second and did nothing. With the default schedule_delay = 5.0, 24 of every 25 wakeups were pure overhead — constant background CPU in profiles of an idle agent service, and timer wakeups on battery-powered hosts.

time.sleep() is also not interruptible by the shutdown event, so shutdown() waited out the rest of the interval before the final drain even with an empty queue.

Fix

The worker now blocks until it actually has something to do. It waits on a threading.Event with a timeout of "time until the next scheduled export", so an idle process wakes once per schedule_delay instead of 25 times.

Both of the things that previously needed polling to be noticed now signal the event explicitly:

  • the queue-size triggeron_trace_start/on_span_end set the event once the queue reaches _export_trigger_size, so the immediate-export path is as responsive as before (more so: it no longer waits for the next poll tick);
  • shutdownshutdown() sets it alongside _shutdown_event, so the worker leaves the loop and drains immediately.

This is the second shape offered in the issue rather than the min(0.2, ...) one-liner: capping the wait at 200 ms would have fixed the shutdown latency but left the idle wakeups — the main complaint — in place. It keeps the existing drain logic untouched, unlike blocking on queue.Queue.get.

There is no lost-wakeup window. A set() that lands while the worker is exporting, or between its wait() returning and its clear(), is covered by the queue-size check at the top of the next iteration — the condition that caused the set() is re-read there, not assumed.

As the issue notes, BackendSpanExporter._sleep_before_retry in this same file already waits on its shutdown event, so this brings the worker in line with it.

Tests

  • test_batch_trace_processor_queue_trigger_wakes_the_idle_worker — with schedule_delay=60, a sub-trigger queue leaves the worker parked and reaching the trigger wakes it. Hangs to failure if the producer-side signal is dropped.
  • test_batch_trace_processor_shutdown_wakes_the_idle_workershutdown() on a parked worker joins promptly and still drains the queued span, instead of timing out and warning.
  • test_batch_trace_processor_schedule_uses_monotonic_clock drove _run through its time.sleep calls, so it now drives it through the wake event's wait instead. Its ControlledTime.sleep raises, which makes the test fail if polling is ever reintroduced into the loop.

Each was verified to fail against a deliberately broken version of the corresponding change.

BatchTraceProcessor's worker thread slept 200 ms at a time and re-checked its
conditions, so an otherwise idle process woke five times a second for the rest
of its lifetime - with the default 5 s schedule_delay, 24 of every 25 wakeups
did nothing. time.sleep() is also not interruptible, so shutdown() waited out
the remainder of the interval before draining.

The worker now blocks on a wake event until its next scheduled export.
Producers set it once the queue reaches the export trigger size and shutdown()
sets it to break out immediately, so the queue-size trigger and shutdown stay
as responsive as before while an idle process wakes once per schedule_delay.
@seratch

seratch commented Aug 27, 2026

Copy link
Copy Markdown
Member

Please refer to #4688 (comment)

@seratch seratch closed this Aug 27, 2026
@rajarshidattapy
rajarshidattapy deleted the fix/trace-processor-worker-wakeups branch August 28, 2026 05:53
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.

BatchTraceProcessor worker thread busy-polls with time.sleep(0.2) instead of waiting on its shutdown event

2 participants