Skip to content

[unsupervised AI] Defer semaphore lease refresh until registration - #9383

Open
GeorgiaPreda wants to merge 1 commit into
dask:mainfrom
GeorgiaPreda:fix-8999-semaphore-callback-lifetime
Open

GeorgiaPreda wants to merge 1 commit into
dask:mainfrom
GeorgiaPreda:fix-8999-semaphore-callback-lifetime

Conversation

@GeorgiaPreda

@GeorgiaPreda GeorgiaPreda commented Oct 28, 2026 •

Copy link
Copy Markdown

RFC to address #8999.

Problem and change

Every deserialized Semaphore started a lease-refresh timer, even when it was only an unused task argument. Task objects can retain these instances, causing active timer counts and idle event-loop CPU to grow with submissions. The bound callback also retained otherwise unreachable semaphore instances.

Start the callback only after successful registration, and use a weak reference inside it. Add regressions covering collection before and after registration, unused arguments not starting timers, and acquisition starting lease renewal.

Verification

  • All three new regression cases fail against the original initialization.
  • 31 semaphore/lock tests passed.
  • Ruff 0.15.16 check and format check passed on both changed files; git diff --check passed.

Bounded CPU comparison

Separate baseline and changed-code runs used four worker processes, ten unused semaphore arguments per task, 1,200 total submissions, 5 ms simulated tasks, and the default lease timeout. Idle CPU was sampled over approximately 6.5 seconds per checkpoint using thread_time on the shared scheduler/client event-loop thread. Both runs used the same /proc compatibility adjustment.

Completed tasks Original idle CPU Changed idle CPU Active callbacks, original / changed
0 3.15% 2.51% 10 / 0
200 4.31% 2.93% 2,010 / 0
600 6.05% 3.36% 6,010 / 0
1,200 7.33% 3.09% 12,010 / 0

This supports removal of timer-driven CPU growth in the bounded workload. It is one comparison, not a statistically controlled benchmark, a scheduler-only process measurement, or proof that the original long-running cluster crash is resolved. Task objects still retained semaphore instances during the measurement; unused instances no longer create active timers.

  • Tests added / passed for the selected scope
  • Changed-file Ruff checks passed
  • Full test suite and all pre-commit hooks

AI tooling: OpenAI GPT-6 via Codex in ChatGPT Work assisted with investigation, implementation, testing and publication.

@GeorgiaPreda
GeorgiaPreda marked this pull request as ready for review October 28, 2026 12:45
@github-actions

Copy link
Copy Markdown
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    40 files  ± 0      40 suites  ±0   14h 0m 32s ⏱️ - 37m 3s
 4 163 tests + 3   3 983 ✅ + 4    178 💤 ±0   2 ❌  -  1 
81 019 runs  +58  76 759 ✅ +43  4 239 💤  - 3  21 ❌ +18 

For more details on these failures, see this check.

Results for commit 58ee127. ± Comparison against base commit dc182bd.

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