Skip to content

Close interleaved prefetch workers concurrently - #1430

Open
jabrailkhalil wants to merge 2 commits into
google:mainfrom
jabrailkhalil:fix/concurrent-interleave-close
Open

jabrailkhalil wants to merge 2 commits into
google:mainfrom
jabrailkhalil:fix/concurrent-interleave-close

Conversation

@jabrailkhalil

@jabrailkhalil jabrailkhalil commented Oct 12, 2026 •

Copy link
Copy Markdown

Fixes #1410.

I start closing the active interleave children before waiting for their cleanup, so independent mp_prefetch worker exit delays overlap instead of accumulating one worker at a time.

Closing remains blocking and idempotent: iterator prefetch is stopped first, every child cleanup is joined, and a child exception is propagated after the remaining children have finished. The zero/single-child path stays synchronous. Final interpreter GC also stays synchronous: Python 3.11 can hang when starting a thread during finalization. If thread creation is unavailable during normal cleanup, that child's close runs synchronously. Worker stop events, join/kill timeouts and data/checkpoint semantics are unchanged.

Validation

  • 12 regression cases cover concurrent cleanup, empty/partial slots, repeated close, exception propagation after sibling cleanup, unavailable threads, and real atexit/final-GC subprocesses. The final-GC subprocess asserts sys.is_finalizing() is actually true.
  • The identical tests against unchanged main fail in exactly five places: the three 2/4/8-child concurrency cases and the two sibling-cleanup-after-error cases. The other seven pass.
  • 200 additional executions of the ten non-subprocess cases pass.
  • Full upstream Bazel build/test matrix passes on Python 3.11/Linux x86, 3.12/macOS, 3.13/Windows and 3.14/Linux ARM for both the final head and unchanged base. The base's initial macOS data-loader timeout passed on a failed-job-only rerun at the same SHA; no timeout or assertion was changed. Author-only workflow commits differ from the pinned source revisions solely by the validation trigger.
  • The 67 process-prefetch and 90 thread-prefetch test methods also pass with the local absl runner, in addition to the framework's skipped base class.
  • The upstream Build & Test run is waiting for maintainer authorization; the linked matrices were run in my fork.
  • Upstream pre-commit hooks pass; changed ranges also pass the Google-style formatter check.

Controlled shutdown measurement on Windows / CPython 3.12.10, with a 300 ms atexit delay in each real spawned worker (median of three runs):

Workers Main This change
2 0.897 s 0.452 s
4 1.796 s 0.496 s
8 3.532 s 0.562 s

All 18 base/final measurements confirmed the requested number of distinct worker PIDs and no live workers after close(). This measures shutdown latency, not data throughput. Scalar outputs isolate this issue from the separate shared-memory cleanup problem in #1420/#1421.

For local Python tests I used the source Python code with the published 0.2.18 index-shuffle native extension; the full Bazel matrix builds the native extension from source.

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.

Closing an mp_prefetch iterator stops its worker processes one at a time

1 participant