Skip to content

feat: add co:trim command to trim hot coordinates in co_block#27

Merged
clawd131662[bot] merged 1 commit into
masterfrom
feat/co-trim
Jul 8, 2026
Merged

feat: add co:trim command to trim hot coordinates in co_block#27
clawd131662[bot] merged 1 commit into
masterfrom
feat/co-trim

Conversation

@clawd131662

@clawd131662 clawd131662 Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Why

Players' auto tree farms write thousands of co_block rows at fixed coordinates.
When a tree grows, CoreProtect's BlockLookup.whoPlaced() runs a
USE INDEX(wid) lookup, but the wid index is (wid, x, z, time) — it lacks
y, so the lookup scans the entire accumulated (x, z) column. On coordinates
with 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 the
invariant: 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 deletes
by 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(*) >= threshold finds hot keys;
each hot key keeps its newest --keep rows (bounded by the segment's upper
rowid) and the rest are deleted by primary key in --step-sized slices. The
checkpoint in db/trim_state.yml (gitignored) only advances — it is never
written below the existing frontier — so a daily cron and an ad-hoc bounded
backfill (--start/--end) can be mixed freely without the backfill rewinding
the cron frontier and forcing a multi-billion-row re-scan.

Options

  • --start=ROWID — scan from this rowid (overrides the checkpoint; required on
    the 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 — session max_statement_time in seconds (default: 600)
  • --dry-run — report hot coordinates and the planned deletion total without
    deleting and without saving the checkpoint

Safety

  • --dry-run previews everything (per-coordinate plans and the aggregate
    Would trim total N rows.) and does not advance the checkpoint. Because a dry
    run 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.
  • The checkpoint is monotonic: save_trim_state refuses to write a rowid at or
    below the existing checkpoint, so a below-frontier backfill can never rewind
    the cron frontier.
  • kill is excluded by default: deleting kill rows would orphan co_entity
    rows (delete_all bypasses dependent: :destroy). If kill is included the
    command prints a reminder to run co:purge_orphaned_entities afterwards.
  • --timeout defaults to 600 s, overriding the .env TIMEOUT of 10 s that
    would kill the long scans/plucks on history-heavy coordinates.

Test plan

  • bin/thor co:trim --start=… --dry-run on production: verifies scan + hot-key
    detection + planned deletions without mutating data or the checkpoint.
  • First supervised real run left to the operator.

🤖 Generated with Claude Code

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread co.thor
Comment thread co.thor Outdated
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review summary

Re-reviewed after the latest revision. Both concerns from the prior round are now addressed, and I resolved those threads:

  • Monotonic checkpointsave_trim_state now returns early when rowid <= checkpoint_floor, and checkpoint_floor is memoized from the initial state read at the start of the run. A below-frontier --start/--end backfill therefore writes nothing and cannot rewind the cron frontier, while a backfill that crosses above the frontier still advances it correctly. Verified against the exact scenario from the earlier comment.
  • Dry-run aggregatetrim_victims accumulates the planned victim count in dry-run, and the ensure block prints Would trim total N rows. (vs Trimmed total N rows.), so the dry-run now reports the volume it exists to estimate.

I went back through the trim algorithm and did not find anything else worth changing:

  • Segment ranges via trim_from.step(trim_to, TRIM_SEGMENT_SIZE) with r2 = [r1 + SIZE - 1, trim_to].min are contiguous with no gaps/overlaps, and the checkpoint saved as each segment's r2 (only after the segment completes) makes interrupts resume cleanly from checkpoint + 1.
  • Trimming is idempotent: hot keys are detected per-(wid, x, y, z, action) within the window, and each is trimmed against its full history up to the segment upper bound, keeping the newest --keep.
  • world_name resolves wid via World.find_by(id:), matching how co_block.wid maps to co_world.id elsewhere (Block::Tile).
  • SQL interpolations are guarded (Integer(options[:timeout]), bound param in HAVING), and the confirm?/ACTIONS refactors are behavior-preserving dedup of the existing purge/orphaned paths.
  • Ordering is right: the heavy GROUP BY/pluck queries run after apply_trim_timeout raises max_statement_time, so the 10 s .env TIMEOUT won't kill them.

One inherent tradeoff worth keeping in mind operationally (not a code change): the per-coordinate pluck filters on (wid, x, y, z, action), which the wid (wid, x, z, time) index can only partially serve — this is the same shape of scan the tool exists to relieve, but it's run deliberately under the raised timeout during maintenance, so that's acceptable. Documentation (co:trim section in CLAUDE.md, README) reads clearly.

Nice work — LGTM.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the current revision. The two earlier concerns are genuinely fixed and I've resolved those threads:

  • Monotonic checkpointsave_trim_state bails when rowid <= checkpoint_floor (memoized from the state read at run start), so a below-frontier --start/--end backfill can't rewind the cron frontier, while a backfill that crosses above it still advances correctly.
  • Dry-run aggregatetrim_victims now accumulates the planned count and the ensure prints Would 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.

Comment thread co.thor Outdated
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
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review summary

Re-reviewed the current revision. The three prior concerns are all genuinely addressed and I've resolved the outstanding thread:

  • Monotonic checkpointsave_trim_state bails when rowid <= checkpoint_floor (memoized at run start), so a below-frontier --start/--end backfill can't rewind the cron frontier while a crossing backfill still advances it.
  • Dry-run 0.0 bug — the ensure block now reports Would trim total N rows. and report_planned_trim accumulates the planned count.
  • Dry-run cross-segment over-countreport_planned_trim tracks each key's cumulative victims in planned_victims and adds only the delta. I re-derived the two-segment example (200+200 rows, threshold 100, keep 7): dry-run now reports 393, matching what a real deleting run removes. Since hot-key detection depends only on the per-segment rowid window (deletions never touch a later segment's window), dry-run and real-run hotness are identical, so the estimate tracks reality. Resolved.

I went back through the trim path and didn't find anything else worth changing:

  • Segment ranges (step(TRIM_SEGMENT_SIZE) with r2 = [r1+SIZE-1, trim_to].min) are contiguous with no gaps/overlaps; the checkpoint saved as each segment's r2 only after it completes makes interrupts resume cleanly from checkpoint+1, and re-trimming is idempotent.
  • Heavy GROUP BY/pluck queries run after apply_trim_timeout raises max_statement_time, so the 10 s .env TIMEOUT won't kill them (Block.last and the checkpoint read that run before it are trivial PK/file lookups).
  • SQL interpolation is guarded (Integer(options[:timeout]), bound param in HAVING); world_name resolves wid via World.find_by(id:), matching Block::Tile; the kill-orphan reminder is guarded so it can't re-raise inside ensure.
  • The confirm?/ACTIONS refactors are clean, behavior-preserving dedup of the purge/orphaned paths.

One thing worth doing (couldn't inline — README isn't in the diff): README.md walks through co:purge with worked examples but has no co:trim section. co:trim is a new destructive command, and a reader of the user-facing doc won't discover it (or its safer --dry-run/checkpoint semantics). CLAUDE.md covers it well; worth mirroring a short section + example into the README so operators find it there too.

Nice work — the algorithm and safety rails read carefully. LGTM once the README is updated.

@clawd131662
clawd131662 Bot merged commit ee7aca1 into master Jul 8, 2026
1 check passed
@shikendon
shikendon deleted the feat/co-trim branch July 8, 2026 15:09
@shikendon
shikendon restored the feat/co-trim branch July 8, 2026 15:11
@shikendon
shikendon deleted the feat/co-trim branch July 8, 2026 15:15
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.

1 participant