fix: harden idle-sleep scheduling against bad config and misattributed saves (#2567)

Follow-ups to #2559, from a review of the ported idle-sleep/scheduler-health changes.

### Fixes

- **`NetState.IsIdle` omitted `_pendingDisconnects`** — `Slice()` drains five queues; the property checked four. The other deferred work (`_connectingQueue`, alive checks, movement throttle) is time-gated and correctly excluded; the disconnect queue was the only ready-work omission. Impact was bounded (≤ one idle wait of delay), but the property's contract is "sleeping cannot strand pending work".
- **Neither new setting was clamped** (`Main.cs`):
  - `server.lateWakeThreshold: -1` made `late <= threshold` false for every sample even at zero late wakes, so from the second sample on, sleeping was re-suspended every second, forever — a permanent full-core spin whose only trace was a nonsense warning ("… at least 8ms late 0 time(s)").
  - `server.eventLoopIdleWaitMs: -1` disabled sleeping while the admin gump reported **Healthy** (it tested `== 0`).
  - Both now clamp to `>= 0` and log a warning naming the configured value. `-1` is a natural thing to reach for given the sibling key's doc says "set very high to disable".
- **World snapshots were misattributed to `StolenMs`** — `World.Snapshot` ran outside all five profiler phases, so a 3-second save inside a sample read as ~75% stolen, and `debugging-event-loop.md` teaches stolen = "the host ran something else". The diagnostic pointed operators at buying dedicated CPU for their own largest loop-thread stall. Saves now land in a new `WorldSnapshot` phase; `[LoopStats` iterates `PhaseCount` generically, so the report and CSV pick it up with no changes.
- **Admin gump conflated host-forced spin with configured spin** — when the startup probe finds no high-resolution wait support it zeroes the idle wait, after which the gump said "Spinning (configured)" and the operator's config said 2. New `Core.IdleSleepUnsupported` property; the gump now shows "Spinning - host cannot honor short waits" as a distinct fourth verdict. A genuinely configured 0 still reads "configured" (the probe only runs when the configured value was > 0).
- **The backoff-ceiling `Error` logged once per process lifetime** — `_loggedBackoffCeiling` never reset, and at the ceiling the method returns before the `Warning`, so a host that recovered (>60s clean streak) and later degraded back to the ceiling never re-logged the one operator-actionable message. The flag now resets with the clean-streak escalation reset.
- **Removed the unreachable "already suspended, extend" branch** — no sleeps occur while suspended, so `_lateWakes` stays 0 and every suspended sample early-returns before reaching it; with the threshold clamped it can never fire. If sleep gating ever changes, the normal path handles the case by counting a fresh episode.

`dev-docs/debugging-event-loop.md` updated to match (phase list + gump verdict table).

### Verification

- `dotnet build` clean (0 warnings) both normally and with `-p:EventLoopProfiling=true` (the snapshot phase only becomes live IL under the profiling flag).
This commit is contained in:
Kamron Batman 2026-08-09 22:05:18 -07:00 committed by GitHub
parent 6d846b11e5
commit 0628902644
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 40 additions and 15 deletions

View file

@ -8,7 +8,8 @@ funnel in order; most incidents resolve before the last step. Do not start with
Every second of the main thread's wall time goes to exactly one of four places:
1. **Work** — the loop's phases: mobile deltas, item deltas, timer callbacks (`Timer.Slice`),
network processing (`NetState.Slice`), posted tasks (`LoopContext`).
network processing (`NetState.Slice`), posted tasks (`LoopContext`), world snapshots
(`WorldSnapshot` — the on-loop portion of a save).
2. **Sleep** — idle blocking in `NetState.WaitForCompletion`, bounded by the next timer tick and
`server.eventLoopIdleWaitMs`.
3. **GC pauses** — land inside whichever phase (or sleep) was running.
@ -27,7 +28,7 @@ No build changes needed. Three signals exist, all actionable:
|---|---|---|
| Startup error: *host cannot honour short waits* | No high-resolution timer and `timeBeginPeriod` failed. Very old or unusual Windows. | Nothing is wrong with the server; it spins and uses a full core. Upgrade the OS or accept the core. |
| Warning: *host returned a Nms idle wait late* + sleeping suspended | The OS did not reschedule the process promptly after a 12ms wait. Shared/burstable vCPU signature. | Move to dedicated CPU, or set `server.eventLoopIdleWaitMs=0` to spin permanently. This is a **host** problem — no amount of server-side change fixes it. |
| Admin gump → Performance → *Event Loop* | `Healthy` / `Sleep suspended (host)` / `Spinning (configured)` | Same as above. |
| Admin gump → Performance → *Event Loop* | `Healthy` / `Sleep suspended (host)` / `Spinning (configured)` / `Spinning - host cannot honor short waits` | Same as above; the last verdict is the startup error's state, not a config choice. |
If none of these fired and the shard still feels laggy, the cause is work, GC, or something a
boot-time signal cannot see. Continue.