Skip to content

Make the retention clocks survive a metaserver restart - #235

Merged
bjmeetsfo merged 2 commits into
mainfrom
oss/freeze-clock-survives-restart
Aug 25, 2026
Merged

Make the retention clocks survive a metaserver restart#235
bjmeetsfo merged 2 commits into
mainfrom
oss/freeze-clock-survives-restart

Conversation

@bjmeetsfo

@bjmeetsfo bjmeetsfo commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Independent of the other open work — main base.

Every retention clock restarted when the metaserver restarted

Everything the metaserver ages — retention purge, freeze aging — is stamped with
now_ms() inside the apply path. Replay runs that same path, so every clock
was re-stamped with the clock of whenever the metaserver last came up.

Measured across one restart, with a 40ms pause in between:

before  frozen : {"table:ns/frozen":  1787581005031}
after   frozen : {"table:ns/frozen":  1787581005075}     moved +44ms

before  dropped: {"server:server-b":  1787581005033,
                  "table:ns/dropped": 1787581005032}
after   dropped: {"server:server-b":  1787581005075,
                  "table:ns/dropped": 1787581005075}     moved +42ms, +43ms

Every clock moved by exactly the pause. On a real cluster the pause is the
restart, so:

  • retention purge never collects anything on a cluster that restarts more
    often than its retention window — tombstones accumulate forever
  • freeze aging never fires — a table frozen a month ago looks freshly frozen
    after any restart

Both failures are silent. The mechanism runs, finds nothing old enough, and
reports success.

The change

The log line now carries the time the metaserver accepted the change, and replay
stamps with that instead of the current clock. record_mutation returns the time
it wrote, and the five apply paths that stamp a clock take it as an argument
rather than reading the clock themselves.

It also fixes frozen_since_ms and freeze_cooldown_until_ms on a server, which
came from the same now and had the same problem.

Compatibility, measured rather than assumed

The log is JSONL. The time is an added field, so I checked both directions
before writing any of this:

  • New build, old lineat_ms is absent, defaults to 0, and replay falls
    back to the current clock. That is exactly what those lines have always been
    given, so nothing changes for a log already on disk.

  • Old build, new line — I measured this rather than trusting that serde
    ignores unknown fields, because adjacently tagged enums are a special case:

    new line : {"at_ms":1234,"kind":"add_namespace","request":{"namespace":"ns"}}
    downgrade: OLD BINARY CAN STILL READ IT
    

    So rolling back to a build that predates this does not strand the log. A test
    pins that behaviour.

Tests

  • the retention clocks survive a metaserver restart — fails without the change
  • a log written before the time was recorded still replays
  • a line carrying the time is still readable without it

The last two pass either way, deliberately. They guard the format change, which
is the part that would hurt most if it broke, and the part a later edit is most
likely to break.

Scope

Only the paths that stamp a clock consumed at replay. last_heartbeat_ms and
boot time still take the current clock on replay, which is arguably right — they
describe liveness, and a real heartbeat corrects them within seconds — but it is
a separate question from this one and I have not touched it.

Verification

  • cargo check --all-targets — 0 errors
  • cargo test --bin metaserver -- --test-threads=1
  • cargo test --lib -- --test-threads=1

The data_node::…jitter_backoff… and engine::…recovery_validates_… failures
reproduce on an unmodified tree at this base and are untouched here.


Correction: which backend this affects

I originally wrote the consequence as though it applied to any cluster. It does
not, and the difference matters for judging this change.

The single-node backend drives retention GC and freeze aging, so everything
above applies to it directly: the clocks reset on restart, and neither mechanism
can reach its threshold on a cluster that restarts often.

The raft backend does not run either loop at all. Both are refused at
startup:

TS_META_RETENTION_GC ignored: raft backend owns its own meta state
TS_META_FREEZE_AGING ignored: raft backend owns its own meta state

So on raft the clocks were wrong and nothing consumed them. This change makes
them correct; it does not make retention run there. That is a separate, larger
piece of work — the tombstones a raft metaserver accumulates are still
accumulating after this merges, and its mutation log is never compacted either.

The change is still worth making on its own terms — the clocks are wrong on both
backends, the single-node one acts on them today, and any future raft-side
retention needs them right before it can work — but I would rather state the
scope than let the measurement above imply more than it shows.

Second correction: the retention framing undersold this

My first correction narrowed the consequence to the backends that run retention.
That was accurate as far as it went and left the wrong impression, because the
same stamp feeds two things that are not behind any switch.

A freeze cooldown restarted on every metaserver restart. The cooldown is
computed as freeze_cooldown_until_ms = now + freeze_cooldown_ms from the same
value, and it is enforced in apply_register_server and apply_register_proxy,
gated by nothing:

if existing.state == MetaEntityState::Frozen && existing.freeze_cooldown_until_ms > now

So a resource frozen with a ten-minute cooldown was locked out for a fresh ten
minutes after every restart, on any configuration, including the default one.

frozen_since_ms is operator-visible. It is a public field on both
ServerMetaInfo and ProxyMetaInfo, so it is serialised to /servers and
/proxies. Before this change it reported "frozen since the last restart"
rather than when the freeze happened.

Neither depends on retention GC or freeze aging being enabled. Nine of the
metaserver's twelve switches default to off — including retention GC — so the
retention consequence reaches fewer deployments than these two do.


Correction to the verification note above

I described two failures as reproducing on an unmodified tree. Re-checked on
a quiet machine with disk headroom, run in isolation against unmodified main:

test unmodified main
data_node::…jitter_backoff… fails 3/3 — genuinely pre-existing, deterministic
engine::…recovery_validates_all_timestamped_kv_page_families passes 3/3

Only the first is pre-existing. My original evidence for the second came from a
run taken immediately after the disk hit 100%, so it was environmental — that
test is sensitive to disk pressure and load, not broken on main.

The conclusion this change is not responsible for either failure is unchanged.
The evidence offered for half of it was wrong, and the record should say so.

@bjmeetsfo
bjmeetsfo requested a review from superhaiou as a code owner August 24, 2026 14:44
Everything the metaserver ages -- retention purge, freeze aging -- was
stamped with now_ms() inside the apply path. Replay runs that same path, so
every clock was re-stamped with the clock of whenever the metaserver last
came up.

Measured across one restart with a 40ms pause: the frozen table's clock
moved 44ms, the dropped table's 43ms, the dropped server's 42ms. Every one
moved by exactly the pause. On a real cluster the pause is the restart, so
retention purge never collects anything and freeze aging never fires on a
cluster that restarts more often than those windows. Both fail silently --
the mechanism runs, finds nothing old enough, and reports success.

The log line now carries the time the metaserver accepted the change and
replay stamps with that. record_mutation returns the time it wrote, and the
five apply paths that stamp a clock take it as an argument instead of
reading the clock themselves. This also fixes frozen_since_ms and
freeze_cooldown_until_ms on a server, which came from the same value.

Both compatibility directions were measured, not assumed. A line written
before the field exists has no time, defaults to zero, and replay falls back
to the current clock -- exactly what those lines have always been given. And
a build that predates the field can still read a line that carries one:
serde ignores it as unknown, which is worth checking rather than assuming
because adjacently tagged enums are a special case. Tests pin both.

Only the paths whose stamp is consumed at replay are changed. Heartbeat and
boot times still take the current clock, which is arguably right for
liveness values a real heartbeat corrects within seconds, and is a separate
question.
@bjmeetsfo
bjmeetsfo force-pushed the oss/freeze-clock-survives-restart branch from 7057399 to b9ef8f5 Compare August 24, 2026 18:09
@bjmeetsfo

Copy link
Copy Markdown
Collaborator Author

This PR also collides with #285, and that resolution is written and tested — branch oss/one-lock-and-the-recorded-drop-clock.

No branch of yours has been touched. That one is #231, then #285 composed onto
it, then this PR composed onto that, so the whole registration.rs cluster is
there in one place.

The collision is over the same two lines. This PR makes the drop clock come from
the time record_mutation recorded, so replay stamps the same instant instead of
whatever the clock says when it replays. #285 splits the apply so the guarded
path can check and apply under one write lock without letting go.

Git's automatic merge produces something that does not compile, which is the
good outcome: it carries this PR's at_ms into the body of the split-out helper
while the helper's signature comes from #285 and does not take it. The compiler
stops there.

The trap is what comes next. The quickest way to make it compile is to delete
at_ms and put now_ms() back, and that is exactly this PR undone -- the clock
would restart on every replay, and no test in either PR fails, because each one
passes on its own branch.

The resolution that keeps both: the lock-holding helper takes at_ms, the
guarded path passes what record_mutation answered, and the replay entry passes
its own through. meta:: is 295 green, and the namespace drop still completes
rather than deadlocking:

test meta::tests::the_emptiness_check_still_refuses_and_still_lets_go ... ok
EXIT_CODE=0

@bjmeetsfo
bjmeetsfo merged commit 5388a1c into main Aug 25, 2026
7 checks passed
@bjmeetsfo
bjmeetsfo deleted the oss/freeze-clock-survives-restart branch August 25, 2026 19:48
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.

2 participants