Skip to content

connectd: add --dev-max-wake-delay-ms - #9481

Open
Amperstrand wants to merge 2 commits into
ElementsProject:masterfrom
Amperstrand:pr9268-dev-max-wake-delay
Open

connectd: add --dev-max-wake-delay-ms#9481
Amperstrand wants to merge 2 commits into
ElementsProject:masterfrom
Amperstrand:pr9268-dev-max-wake-delay

Conversation

@Amperstrand

@Amperstrand Amperstrand commented Sep 5, 2026

Copy link
Copy Markdown

The wake-delay watchdog (write_to_subd's empty-queue check) measures the gap between handing a peer message to the subdaemon queue and the subdaemon consuming it, which includes lightningd-master's time to spawn the subdaemon: under CI load that alone can exceed the hardcoded 5000 msec and fail runs with wake delay for WIRE_OPEN_CHANNEL BROKEN (#9268) — tests/test_connection.py::test_funding_cancel_race spins up 100 nodes on a fast runner and hit a 11068 msec window. dev_lightningd_is_slow exists for the same class but is only settable by the memleak dev-RPC.

This makes the threshold a developer option (--dev-max-wake-delay-ms, default unchanged at 5000) so load-heavy tests can raise it; wired through connectd_init. The regression test freezes the accepter's master past the default threshold and expects a clean run with the option raised.

deterministic repro results (master build, regtest pyln) — expand
run1/C2-repro REPRODUCED (pytest failed on broken log, as in CI)
run2/C2-repro REPRODUCED      run3/C2-repro REPRODUCED
run1..3/C1-control PASS (identical flow, no SIGSTOP)
9268/option-b PASS, 9268/in-PR-test PASS (6/6 cells green)

Fuller repro record — the trigger recipe with its two ordering traps, all cell results, and the control arm: https://gist.github.com/Amperstrand/b50c4d604f88016047e104dc65f866c6

Fixes #9268

The wake-delay watchdog (write_to_subd's empty-queue check) measures
the gap between handing a peer message to the subdaemon queue and the
subdaemon consuming it, which includes lightningd-master's time to
spawn the subdaemon: under CI load that alone can exceed the hardcoded
5000 msec and fail runs with 'wake delay for WIRE_OPEN_CHANNEL'
BROKEN (ElementsProject#9268) -- tests/test_connection.py::
test_funding_cancel_race spins up 100 nodes and hit a 11068 msec
window. dev_lightningd_is_slow exists for the same class but is only
settable by the memleak dev-RPC.

Make the threshold a developer option (default unchanged at 5000) so
load-heavy tests can raise it; wired through connectd_init. The
regression test freezes the accepter's master past the default
threshold and expects a clean run with the option raised.

Changelog-Fixes: ElementsProject#9268

@Andezion Andezion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR says Fixes #9268, but it does not fix the flaky test named in that issue. test_funding_cancel_race is the test that hit the 11068ms delay in ci, and it still calls node_factory.get_nodes(num, opts={}) - no dev-max-wake-delay-ms is passed there. The pr only adds a separate, new test (test_dev_max_wake_delay) that exercises the new option in isolation

The 100+ nodes this test spawns starve the scheduler enough on loaded
CI runners that connectd's wake-delay watchdog fires BROKEN on the
delayed WIRE_OPEN_CHANNEL (the 11068ms case in ElementsProject#9268) long before the
funding-cancel race under test is affected. Give every node the raised
threshold (--dev-max-wake-delay-ms=60000) so the incidental telemetry
stops failing the test while genuine multi-minute hangs still trip it.

Fixes: ElementsProject#9268
Signed-off-by: Amperstrand <amperstrand@localhost>
@Amperstrand

Copy link
Copy Markdown
Author

Fair point — the test that actually hit #9268 never got the knob. Wired now: every node in test_funding_cancel_race runs with --dev-max-wake-delay-ms=60000, so scheduler starvation on CI runners (the observed 8–12 s wakes) no longer fails the test through the wake-delay telemetry, while a genuinely hung wake still trips it. The isolated test stays as documentation of the option itself. Pushed as 4c0624a.

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.

CI Flake test_funding_cancel_race

2 participants