Skip to content

syncProbeTimers publishes scheduler state BEFORE arming the anchor, so nextAnchoredRunAt reads null exactly when an operator checks it #176

Description

@harper-joseph

Small, but it breaks the one signal the code designs for this.

What happens

In syncProbeTimers (util/changeProbe.js):

armedSweep = desiredSweep;
armedCanary = desiredCanary;
void publishScheduler();          // <-- publishes here; nextAnchorAt is still null
if (desiredSweep === null) return;
if (wasEnabled || isAnchored()) {
    armIntervals();               // <-- arms the anchor, sets nextAnchorAt
    return;
}

publishScheduler() runs before armIntervals(), so the published snapshot captures nextAnchorAt while it is still null. The admin surface then reports nextAnchoredRunAt: null until something else republishes — in practice the end of the current sweep, which can be hours away.

Why it matters

armAnchorTimer uses that exact field as its failure signal, and says so:

An unusable anchor arms NOTHING. setTimeout(fn, NaN) fires at once, which would turn a typo in the timezone into a full-rate pass on every config apply; a warning and a null nextAnchoredRunAt on the admin surface is the failure mode that gets noticed and fixed.

So null is supposed to mean "your anchor is broken, fix it". After this ordering bug it also means "the anchor is perfectly fine, it just has not been republished yet" — and the two are indistinguishable from the admin API.

Observed on a 4-node deployment, switching changeProbe.mode from continuous to anchored live:

mode=anchored  armed=anchored:00:05|America/Chicago  nextAnchoredRunAt=None  stillRunning=True

on all four nodes. The anchor was in fact healthy — zero anchored mode has no next run and zero anchorTimezone ... is not usable warnings in the logs. The only way to tell the difference was to shell into the containers and grep, which is precisely what the published field exists to avoid.

It is worst at the moment it is most likely to be read: right after flipping to anchored mode, when an operator wants to confirm the next run.

Fix

Publish after arming. Either move the void publishScheduler() below the armIntervals() calls, or publish again at the end of each branch. The desiredSweep === null path should still publish, since a disarm is exactly what the field should report.

Worth a test that a live continuous -> anchored switch leaves nextAnchoredRunAt finite in the published state.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions