feat: add co:trim command to trim hot coordinates in co_block#27
Conversation
There was a problem hiding this comment.
Thorough, well-documented change — the design doc (docs/hot-coordinate-trim.md) makes the intent and the incremental-checkpoint idea easy to follow, and the confirm?/ACTIONS refactors nicely dedup the existing purge paths. The trim algorithm looks correct to me: segment ranges are contiguous with no gaps/overlaps, re-trimming is idempotent, and interrupted segments are safely re-scanned since the checkpoint only advances after a segment fully completes. world_name correctly resolves wid via the id column (matching Block::Tile), and the SQL interpolation in apply_trim_timeout is guarded by Integer().
Two things worth addressing below — one operational footgun around the shared checkpoint, and one dry-run reporting nit. Neither blocks the core logic.
Review summaryRe-reviewed after the latest revision. Both concerns from the prior round are now addressed, and I resolved those threads:
I went back through the trim algorithm and did not find anything else worth changing:
One inherent tradeoff worth keeping in mind operationally (not a code change): the per-coordinate pluck filters on Nice work — LGTM. |
There was a problem hiding this comment.
Re-reviewed the current revision. The two earlier concerns are genuinely fixed and I've resolved those threads:
- Monotonic checkpoint —
save_trim_statebails whenrowid <= checkpoint_floor(memoized from the state read at run start), so a below-frontier--start/--endbackfill can't rewind the cron frontier, while a backfill that crosses above it still advances correctly. - Dry-run aggregate —
trim_victimsnow accumulates the planned count and theensureprintsWould trim total N rows.
The confirm?/ACTIONS refactors are clean, behavior-preserving dedup of the purge paths. Segment ranges are contiguous, real-run trimming is idempotent, and interrupted segments re-scan safely.
Two things below.
1. Dry-run total over-counts across segments (inline) — the aggregate the dry-run exists to produce can be inflated by orders of magnitude for persistent hot coordinates.
2. README doesn't mention co:trim (minor) — the README walks through co:purge with examples but has no co:trim section, so a reader of the user-facing doc won't discover this new destructive command. CLAUDE.md covers it, but worth mirroring in the README. (Couldn't leave this inline since README isn't part of the diff.)
Neither blocks the core logic — nice work overall.
Scans rows added since the checkpoint (db/trim_state.yml, gitignored) in 1M-rowid segments; GROUP BY (wid, x, y, z, action) HAVING COUNT(*) >= --threshold flags hot keys, each keeping its newest --keep rows while the rest are deleted by primary key in --step slices. The checkpoint is saved after every segment, so interrupted runs resume. The kill action is excluded by default because deleting kill rows orphans co_entity rows; a reminder to run co:purge_orphaned_entities is printed when it is included. --timeout sets the session max_statement_time (default 600 s) because the .env TIMEOUT of 10 s kills the long scans on history-heavy coordinates. Also consolidates the three confirmation prompts into Co#confirm? and rewrites option_action_ids around the ACTIONS map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019rxDnL4R89uo579K66Gtbd
Review summaryRe-reviewed the current revision. The three prior concerns are all genuinely addressed and I've resolved the outstanding thread:
I went back through the trim path and didn't find anything else worth changing:
One thing worth doing (couldn't inline — README isn't in the diff): Nice work — the algorithm and safety rails read carefully. LGTM once the README is updated. |
Why
Players' auto tree farms write thousands of
co_blockrows at fixed coordinates.When a tree grows, CoreProtect's
BlockLookup.whoPlaced()runs aUSE INDEX(wid)lookup, but thewidindex is(wid, x, z, time)— it lacksy, so the lookup scans the entire accumulated(x, z)column. On coordinateswith tens of thousands of rows this stalls CoreProtect's single-threaded
consumer, which then backs up every subsequent
INSERT.The full root-cause analysis and live measurements are kept as local operator
notes (deliberately not committed to the repo).
What
Adds
co:trim, an incremental "hot coordinate" trim that maintains theinvariant: any coordinate gaining ≥ threshold new rows in a scan window is
trimmed to its newest N rows per
(wid, x, y, z, action). Quiet coordinates(old buildings) are never touched, so it complements
co:purge: purge deletesby age, trim caps per-coordinate accumulation while preserving old history.
How it works — scans rows added since the last checkpoint (primary-key
range scan) in 1,000,000-rowid segments. Per segment,
GROUP BY (wid, x, y, z, action) HAVING COUNT(*) >= thresholdfinds hot keys;each hot key keeps its newest
--keeprows (bounded by the segment's upperrowid) and the rest are deleted by primary key in
--step-sized slices. Thecheckpoint in
db/trim_state.yml(gitignored) only advances — it is neverwritten below the existing frontier — so a daily cron and an ad-hoc bounded
backfill (
--start/--end) can be mixed freely without the backfill rewindingthe cron frontier and forcing a multi-billion-row re-scan.
Options
--start=ROWID— scan from this rowid (overrides the checkpoint; required onthe very first run)
--end=ROWID— stop at this rowid (default: current max)--keep=N— newest rows to keep per hot coordinate (default: 7)--threshold=N— new rows per coordinate within the window to flag it hot(default: 100; must be ≥
--keep)--action=LIST/-a— actions to consider (default:-block,+block,click)--step=N— delete batch size (default: 1000)--timeout=N— sessionmax_statement_timein seconds (default: 600)--dry-run— report hot coordinates and the planned deletion total withoutdeleting and without saving the checkpoint
Safety
--dry-runpreviews everything (per-coordinate plans and the aggregateWould trim total N rows.) and does not advance the checkpoint. Because a dryrun deletes nothing, a coordinate that stays hot across multiple segments is
re-plucked in full each pass, so the counter reports only each pass's new
victims — the total matches what a real, deleting run would remove instead of
over-counting the shared history.
save_trim_staterefuses to write a rowid at orbelow the existing checkpoint, so a below-frontier backfill can never rewind
the cron frontier.
killis excluded by default: deletingkillrows would orphanco_entityrows (
delete_allbypassesdependent: :destroy). Ifkillis included thecommand prints a reminder to run
co:purge_orphaned_entitiesafterwards.--timeoutdefaults to 600 s, overriding the.envTIMEOUTof 10 s thatwould kill the long scans/plucks on history-heavy coordinates.
Test plan
bin/thor co:trim --start=… --dry-runon production: verifies scan + hot-keydetection + planned deletions without mutating data or the checkpoint.
🤖 Generated with Claude Code