Skip to content

fix(test): the ENOSPC bootstrap test no longer lets a deadline decide its verdict - #2272

Open
DeusData wants to merge 2 commits into
mainfrom
fix/test-daemon-bootstrap-sync-spawn
Open

DeusData wants to merge 2 commits into
mainfrom
fix/test-daemon-bootstrap-sync-spawn

Conversation

@DeusData

Copy link
Copy Markdown
Owner

Removes a CI lottery on the MSan lane. daemon_bootstrap_fails_fast_when_daemon_dies_at_publication went red on three PRs that touch no daemon code (#2245, #2158, #2140) and green on a rerun of the identical commit. Test-only change; no production code, no assertion changed.

What was wrong

The test forks a real daemon host whose record publication fails with ENOSPC and asserts that the client names that cause instead of waiting out its 30 s startup deadline.

Before it listens, the host SHA-256s its own executable image — and the test runner is about 450 MB: ~3 s natively, 20–32 s under MSan (48 CI samples, median 25.8 s). The test's spawn override returned as soon as it had forked, so the client's 30 s clock raced the host's hash. On a slow run the deadline fired first, the reaper SIGKILLed a host that had not failed yet, and there was no listen_failed line to find:

tests/test_daemon_bootstrap.c:1116: ASSERT(daemon_named_cause)

Nothing was wrong with the product. A deadline was deciding a test.

The fix

The spawn override now reaps the forked host before it returns. The host writes its start-failure record and only then releases its lifetime reservation and exits, so "record on disk, reservation released" is a stable state, pinned by construction, when the client takes its first look. That first look is guaranteed by the client code as it is today: the post-spawn wait loop runs at least once even past the deadline, and the first failure check after a spawn is unthrottled. How long the host takes no longer matters.

Proof

One build with temporary, env-switched instrumentation that is not part of this PR — a sleep in the forked host, and a switch back to the old fire-and-forget spawn:

spawn forked host delayed daemon_bootstrap
synchronous (this PR) no 28 passed
old, asynchronous 31 s 27 passed, 1 failed — ASSERT(log_read)
synchronous (this PR) 31 s 28 passed

The old spawn fails one assertion earlier than CI did (log_read rather than daemon_named_cause) because the injected sleep sits before the host opens its log, whereas in CI the host was killed mid-hash with the log already open. Same mechanism — the client's deadline beating the host — and the attribution on 20 September reproduced the CI message itself with a delay placed after the log is opened.

Final tree without the instrumentation: daemon_bootstrap 28 passed, three runs in a row; memory-core linter clean.

Not in this PR

On deadline expiry the client composes its "active or starting" message without one last look at the failure record (src/daemon/bootstrap.c). That is a small production wording defect found during attribution and deserves its own change with its own test.

… its verdict

daemon_bootstrap_fails_fast_when_daemon_dies_at_publication went red on
the MSan lane of three PRs that touch no daemon code at all (#2245,
#2158, #2140) with

    tests/test_daemon_bootstrap.c:1116: ASSERT(daemon_named_cause)

and green on a rerun of the identical commit.

Cause, attributed 2026-09-20. The test forks a real daemon host whose
record publication fails with ENOSPC, and asserts that the client names
that cause instead of waiting out its 30 s startup deadline. But before
it listens, the host SHA-256s its own executable image, and the test
runner is ~450 MB: ~3 s natively, 20-32 s under MSan (48 CI samples,
median 25.8 s). The spawn override returned as soon as it had forked, so
the client's 30 s clock raced the host's hash. On a slow run the
deadline fired first, the reaper SIGKILLed a host that had not failed
yet, and the log had no listen_failed line to find. Nothing was wrong
with the product: with a 28 s artificial delay the client does learn the
recorded ENOSPC cause.

The spawn override now reaps the forked host before it returns. The
host writes its start-failure record and only then releases its lifetime
reservation and exits, so "record on disk, reservation released" is a
stable state pinned by construction when the client takes its first look
-- and that first look is guaranteed: the client's post-spawn wait loop
runs at least once even past the deadline, and its first failure check
after a spawn is unthrottled. How long the host takes no longer matters.
No production code changes; the assertions are unchanged.

A regression to the pre-#1828 behaviour (a client that ignores the
record) still fails the unchanged assertions: its message no longer
names the failure, the errno and the path.

Proof, one build with temporary env-switched instrumentation that is
not part of this commit (a sleep in the forked host before it starts,
and a switch back to the old fire-and-forget spawn):

  spawn          forked host delayed   daemon_bootstrap suite
  synchronous    no                    28 passed
  old (async)    31 s                  27 passed, 1 failed:
                                       ASSERT(log_read)
  synchronous    31 s                  28 passed

The old spawn fails under the delay one assertion earlier than CI did
(log_read, not daemon_named_cause): the injected sleep sits before the
host opens its log, whereas in CI the host was killed mid-hash with the
log already open. Same mechanism -- the client's deadline beating the
host -- and the 2026-09-20 attribution reproduced the CI message itself
with a delay placed after the log is opened.

The final tree, without the instrumentation: daemon_bootstrap 28 passed,
three runs in a row.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

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