fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

FleetdAssembly's herdr boot wait bypasses ResourcePorts: every test with an unhealthy lead herdr pays 30 real seconds #629

New Issue
Open
opened 2026-09-22 07:21:17 +02:00 by ltms · 0 comments
ltms commented 2026-09-22 07:21:17 +02:00
Owner
Copy Link
Copy Source

What this is

FleetdAssembly.assembleAndStart waits for the lead herdr socket at line 197:

Fleetd.HerdrAwaitOutcome herdrOutcome = Fleetd.awaitHerdr(herdr, ports.nanoClock(), Fleetd::sleepHerdrPoll);

The clock goes through ResourcePorts. The sleep does not — Fleetd::sleepHerdrPoll is a hardcoded Thread.sleep:

static void sleepHerdrPoll() {
    try {
        Thread.sleep(HERDR_WAIT_POLL_MILLIS);
    } catch (InterruptedException ie) {
        Thread.currentThread().interrupt();
    }
}

With HERDR_WAIT_SECONDS = 30 and HERDR_WAIT_POLL_MILLIS = 500 (Fleetd.java:108 and :117), a test that assembles against an unhealthy lead herdr runs the loop 60 times and burns 30 seconds of real wall clock, whatever clock it injected.

Measured

FleetdAssemblyFleetAppTest on 14b169c, per-test times from the surefire XML:

    0.93s  healthzIsGreenWhenBothDaemonsAreUp
   30.50s  healthzGoesRedWhenTheLeadDaemonIsDownEvenThoughTheMemberIsUp
    0.05s  healthzGoesRedWhenTheMemberDaemonIsDownEvenThoughTheLeadIsUp

A 600x gap between the two "down" directions. The cause is not the test: only the lead client is passed to awaitHerdr, so a down member is observed instantly while a down lead pays the full boot wait. The test uses new FakeHerdr().healthy(false) — no real socket is involved, and the injected nanoClock() is real System::nanoTime, so the deadline is only reached by actually waiting.

Same figure on the green path as on a mutant, so this is a permanent suite cost, not a failure artifact.

Why it is a defect rather than a slow test

ResourcePorts' own javadoc states the rule this breaks:

every boot-time side effect FleetdAssembly.assembleAndStart performs that a real daemon must do for real, and a test must not — read the process environment, connect a herdr client, open a broker, read a clock, start a background scheduler, register the JVM shutdown hook, and bind the HTTP server.

A 30-second Thread.sleep is exactly such a side effect. The list simply missed it. Injecting the clock but not the sleep gives a seam that looks controllable and is not — which is the same shape as the rest of #612: the call site names the right things and still does the wrong one.

Consequence

Today it costs one test 30 seconds. It gets worse, not better: several of the ranks still open on #612 are boot-path wirings, and step 4 will add more tests that assemble with a degraded herdr. Each one pays the same 30 seconds unless the seam is closed first.

There is no production impact. A real daemon waiting 30s for its herdr socket is the intended behaviour.

Suggested fix

Add the poll sleep to ResourcePorts alongside nanoClock() and newScheduler(), so SystemResourcePorts keeps Fleetd::sleepHerdrPoll and a test supplies a no-op that advances its own clock instead. Then awaitHerdr's deadline becomes reachable without real time passing.

Note ResourcePorts deliberately ships no none() default, for the reason its javadoc gives — a test writes its own fake and owns that choice. Keep that; do not add an inert default for this either.

Provenance

Found while verifying PR #626 (#612 unit B2). The test that exposed it is correct and is merged. This ticket is only about the seam it revealed.

## What this is `FleetdAssembly.assembleAndStart` waits for the lead herdr socket at line 197: ```java Fleetd.HerdrAwaitOutcome herdrOutcome = Fleetd.awaitHerdr(herdr, ports.nanoClock(), Fleetd::sleepHerdrPoll); ``` The **clock** goes through `ResourcePorts`. The **sleep** does not — `Fleetd::sleepHerdrPoll` is a hardcoded `Thread.sleep`: ```java static void sleepHerdrPoll() { try { Thread.sleep(HERDR_WAIT_POLL_MILLIS); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); } } ``` With `HERDR_WAIT_SECONDS = 30` and `HERDR_WAIT_POLL_MILLIS = 500` (`Fleetd.java:108` and `:117`), a test that assembles against an unhealthy lead herdr runs the loop 60 times and burns **30 seconds of real wall clock**, whatever clock it injected. ## Measured `FleetdAssemblyFleetAppTest` on `14b169c`, per-test times from the surefire XML: ``` 0.93s healthzIsGreenWhenBothDaemonsAreUp 30.50s healthzGoesRedWhenTheLeadDaemonIsDownEvenThoughTheMemberIsUp 0.05s healthzGoesRedWhenTheMemberDaemonIsDownEvenThoughTheLeadIsUp ``` A 600x gap between the two "down" directions. The cause is not the test: only the **lead** client is passed to `awaitHerdr`, so a down member is observed instantly while a down lead pays the full boot wait. The test uses `new FakeHerdr().healthy(false)` — no real socket is involved, and the injected `nanoClock()` is real `System::nanoTime`, so the deadline is only reached by actually waiting. Same figure on the green path as on a mutant, so this is a permanent suite cost, not a failure artifact. ## Why it is a defect rather than a slow test `ResourcePorts`' own javadoc states the rule this breaks: > every boot-time side effect `FleetdAssembly.assembleAndStart` performs that a real daemon must do for real, and a test must not — read the process environment, connect a herdr client, open a broker, read a clock, start a background scheduler, register the JVM shutdown hook, and bind the HTTP server. A 30-second `Thread.sleep` is exactly such a side effect. The list simply missed it. Injecting the clock but not the sleep gives a seam that *looks* controllable and is not — which is the same shape as the rest of #612: the call site names the right things and still does the wrong one. ## Consequence Today it costs one test 30 seconds. It gets worse, not better: several of the ranks still open on #612 are boot-path wirings, and step 4 will add more tests that assemble with a degraded herdr. Each one pays the same 30 seconds unless the seam is closed first. There is no production impact. A real daemon waiting 30s for its herdr socket is the intended behaviour. ## Suggested fix Add the poll sleep to `ResourcePorts` alongside `nanoClock()` and `newScheduler()`, so `SystemResourcePorts` keeps `Fleetd::sleepHerdrPoll` and a test supplies a no-op that advances its own clock instead. Then `awaitHerdr`'s deadline becomes reachable without real time passing. Note `ResourcePorts` deliberately ships no `none()` default, for the reason its javadoc gives — a test writes its own fake and owns that choice. Keep that; do not add an inert default for this either. ## Provenance Found while verifying PR #626 (#612 unit B2). The test that exposed it is correct and is merged. This ticket is only about the seam it revealed.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#629
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?