feat: prune repeated stream values on a schedule - #1424
Conversation
Time Submission Status
Submit or update total time with: Add time on top of previous submission with: See available commands to help comply with our Guidelines. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@holdex pr submit-time 4h |
* feat: run a duplicate prune sweep from the node * feat: prune repeated stream values on a schedule (#1424) * feat: refuse a one-off prune while a drain is running
1 participant
Second of two, and stacked on the first, so this diff is the scheduler alone. A node
operator can now let a network drop repeat records on its own, instead of running the
prune action by hand.
What this adds
A second cron in
tn_digestthat drainsauto_prune_duplicatesthe way the digest jobdrains
auto_digest. It shares the extension's leader gating and its config-reloadworker, and nothing else:
duplicate_prune_configrather thandigest_config, its ownenabled flag, its own schedule, its own cron and its own context.
That separation is the point, and two tests pin it in both directions: a digest schedule
change and a digest disable each have to leave the sweep running. On a shared cron either
one would silently cancel a prune drain partway through, with nothing to say it had
happened.
It ships off, and the gate is the table, not a build flag.
duplicate_prune_config.enabledalready ships false from migration 056, and it is whatturns pruning on.
TrimTxEventsEnabledis a Go constant because tx-event trimming has noconfig table; this has one, so a second gate would mean an operator sets the column
through a signed exec-sql and watches nothing happen, and turning pruning off again would
need a release.
Two things worth reviewing closely
The stop ordering.
gocron.Scheduler.Stop()does not signal, it joins:executor.stop()ends inwg.Wait(). A prune drain can sit for minutes waiting on thedrain slot and only stops waiting when its context is cancelled, so stopping the cron
before cancelling deadlocks, and holding the scheduler mutex across it blocks the job that
needs that mutex on entry.
leaderwatch's lose-leadership callback would block with it.Both stop paths now cancel first and hold no lock. The same reordering was applied to the
existing
Stop, which had the same shape and the same latent bug, narrowly before thischange and widely after it.
Worth knowing how that was caught. The first regression test passed against the broken
code: it blocked a plain goroutine on the slot, and gocron only joins jobs it owns, so
there was nothing for
Stopto wait for.TestStopPrune_ReleasesASweepWaitingForTheSlotdrives a real cron job, and against the wrong order it hangs to a 90-second timeout.
The drain slot. Digest and prune both broadcast from the node's signer account and
both default to
0 */6 * * *, so on most firings they start at the same instant and wouldfetch the same nonce. One token serialises them. It waits rather than skips: with
identical schedules, a firing that skipped on contention would skip every time.
Sizing
In
scheduler/constants.go, with the reasoning beside the numbers: 100 streams a run, 100runs a firing, so 10,000 streams a firing and about 19 firings to cover mainnet's ~182,000
primitive streams. The delete cap is digest's proven 100,000.
The sweep is cyclic, so
has_more_to_deletemeans "the cursor has not finished a pass"rather than "there is more to delete", and a firing runs its whole loop. A run that
deleted nothing therefore waits 5 s instead of 60 s: once the backlog is gone every run is
one of those, and a flat 60 s would spend 100 minutes of wall clock a firing moving a
cursor. The full delay stays for the runs that actually delete, which is what it is for.
Tests
Thirteen unit tests.
enable and disable through a config reload; the two separation cases above; and a node
whose binary is ahead of its migrations, which has to leave the sweep off rather than
fail every reload.
release, a hand-built scheduler with a nil slot, and the stop-ordering regression.
Not fixed here
go test -race ./extensions/tn_digest/fails, and fails onmaintoo:Extension'sconfig snapshot is written by the background retry worker and read by the consensus
goroutine with nothing between them. CI does not run
-raceon this package, so it hasgone unseen. The two fields added here follow the same discipline as the existing two
rather than diverging from them; the fix is a lock around the whole snapshot and belongs
in its own change.
Rollout
Nothing prunes on merge. Turning it on is a signed
kwil-cli exec-sqlagainstduplicate_prune_config, never psql, and the operator notes are in the extension README.