Skip to content

Make deltaproxy parallel_startup behave like serial startup (#70213) - #70214

Open
ggiesen wants to merge 1 commit into
saltstack:3008.xfrom
ggiesen:fix-deltaproxy-parallel-startup
Open

Make deltaproxy parallel_startup behave like serial startup (#70213)#70214
ggiesen wants to merge 1 commit into
saltstack:3008.xfrom
ggiesen:fix-deltaproxy-parallel-startup

Conversation

@ggiesen

@ggiesen ggiesen commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Brings deltaproxy's parallel_startup path to parity with the serial one. Fixes two independent defects that both only appear when parallel_startup: True.

What issues does this PR fix?

Fixes #70213

Root cause

One sub-proxy failing killed the whole control proxy. An unloadable proxytype makes the proxy loader raise KeyError on <proxytype>.init. That propagated out of asyncio.gather into the except Exception: log.error(...); raise immediately below it, aborting post_master_init so the daemon could not connect -- taking every healthy sub-proxy down with it. The non-parallel branch has always caught per sub-proxy and carried on.

Sub-proxy schedules, beacons and subprocess cleanup never fired. threaded_subproxy_tune_in installs a brand new asyncio loop as the worker thread's current loop, and PeriodicCallback.start() binds to IOLoop.current(). So every timer bound to a loop that is never run and dies when the ThreadPoolExecutor worker exits. schedule.list still reported the job, which made it look correctly configured. Each sub-proxy also leaked that loop's descriptors.

One note for reviewers: io_loop here is a plain asyncio loop, not a tornado.ioloop.IOLoop, so the thread-safe hand-off is call_soon_threadsafe rather than add_callback (the latter raises AttributeError and breaks startup).

Previous Behavior

With parallel_startup: True, a single sub-proxy with a bad proxytype left the salt-proxy daemon dead and no sub-proxy reachable; and no sub-proxy's schedule, beacons or subprocess cleanup ever ran.

New Behavior

The parallel path degrades exactly like the serial path: the failing sub-proxy is logged and skipped, the healthy ones come up, and sub-proxy schedules, beacons and cleanup run normally.

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated

Tests written?

Yes. Three unit tests in tests/pytests/unit/metaproxy/test_deltaproxy.py, each with its inverse case: gather_subproxies skips the failure and keeps the healthy results, passes everything through untouched when all succeed, and subproxy_tune_in queues the periodic registration on the sub-proxy's own loop rather than running it inline. All three fail against unmodified 3008.x.

Verified end to end against a live deltaproxy on 3008.2 (one control proxy, three dummy sub-proxies, multiprocessing: False):

scenario before after
bad proxytype on one sub-proxy, parallel startup healthy sub-proxies unreachable, control proxy dead both healthy sub-proxies answer, bad one skipped and logged, control proxy alive
sub-proxy schedule (test.ping every 10s), parallel startup 0 executions in 45s 4 executions in 45s
regression: all sub-proxies healthy, parallel startup -- 3/3 up
regression: bad proxytype, serial startup -- 2/3 up, behaviour unchanged

Commits signed with GPG?

No

…k#70213)

Two ways the parallel startup path diverged from the serial one.

A sub-proxy that failed to initialise took the whole control proxy with
it. An unloadable proxytype raises KeyError from the proxy loader, which
propagated out of asyncio.gather into the except/raise below it and
aborted post_master_init, so the daemon could not connect and every
healthy sub-proxy went down too. The serial branch has always caught per
sub-proxy and carried on. Collect the results with return_exceptions and
skip the failures the same way.

Sub-proxy schedules, beacons and subprocess cleanup never fired.
threaded_subproxy_tune_in installs a brand new asyncio loop as the
worker thread's current loop, and PeriodicCallback.start() binds to
IOLoop.current(), so every timer bound to a loop that is never run and
dies with the worker. schedule.list still reported the job, which made
it look configured. Hand the registration to the sub-proxy's own
io_loop with call_soon_threadsafe instead, and close the throwaway loop
rather than leaking it once per sub-proxy.

Note that opts["io_loop"] here is a plain asyncio loop, not a tornado
IOLoop, so call_soon_threadsafe is the thread-safe hand-off rather than
add_callback.
@ggiesen
ggiesen requested a review from a team as a code owner September 2, 2026 17:59
@twangboy twangboy added the test:full Run the full test suite label Sep 2, 2026
@twangboy twangboy added this to the Argon v3008.3 milestone Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: deltaproxy parallel_startup is not equivalent to serial startup: one bad sub-proxy kills the daemon, and no sub-proxy schedule/beacon ever fires

2 participants