Skip to content

fix(executor): do not retain callbacks when worker startup fails - #437

Open
realAaronWu wants to merge 4 commits into
apache:mainfrom
realAaronWu:codex/paimon-436-executor-submission
Open

realAaronWu wants to merge 4 commits into
apache:mainfrom
realAaronWu:codex/paimon-436-executor-submission

Conversation

@realAaronWu

Copy link
Copy Markdown

Purpose

Fixes #436. Related to #435, but does not implement planning cancellation or budgets.

DefaultExecutor::Add currently queues its callback before lazy worker startup. If startup throws, a later submission can run the callback whose submission failed.

Start workers under the existing mutex before enqueueing the callback. A failed invocation releases its captures immediately; partial worker startup can leave idle workers and does not restore the configured worker count on retry.

Tests

  • Add a Linux 64-bit GNU/Clang static-library regression target with submitting-thread-only allocation faults. Cover startup, partial startup, immediate capture release, recovery, shutdown, and queue growth with an accepted prefix. CTest bounds the executable to 30 seconds.
  • Actual unmodified-main production archive fails the startup regression with exit 1 through the native runner; the fixed production target passes.
  • Local Linux x86_64 / GCC 15.3 / Debug CMake + CTest: new target 2 passed; common 1,721 passed + 2 expected skips; core 2,351 passed; scan 87 passed; scan-and-read 511 passed; read 302 passed.
  • Scoped ASan/UBSan/LSan: the executor/status implementation and the new/existing executor test translation units, 14 cases passed. This is not whole-project sanitizer coverage.
  • Pinned pre-commit hooks passed. Public CI remains pending for this validation draft. Local network tests and optional Rust formats were disabled; ARM, TSAN, and GPU execution are not claimed.

RelWithDebInfo new/common targets also passed. Its wider build hit an unchanged GCC 15 -O2 -Werror=maybe-uninitialized warning in remote_lookup_file_manager_test.cpp; the identical compile fails on main too. Debug wider targets above completed without suppressing that warning or changing unrelated code.

API and Format

No public API or storage-format change. Custom executors, general ShutdownNow allocation-failure safety, and complete pool rollback are outside this fix.

Documentation

No new user-facing feature. The implementation comment explains the submission boundary.

Generative AI tooling

Generated-by: OpenAI Codex (desktop; model/version not exposed).

@realAaronWu

Copy link
Copy Markdown
Author

Local validation and isolated internal review are complete. The public workflows are awaiting maintainer approval; could you please approve their execution? This remains a validation draft until the applicable CI checks complete.

@lxy-9602
lxy-9602 marked this pull request as ready for review October 16, 2026 05:53
@lxy-9602
lxy-9602 requested a review from lucasfang October 16, 2026 05:54

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.

fix(executor): avoid retaining a callback when lazy worker startup fails

1 participant