fix(trim): bound per-coordinate trimming so history-heavy columns don't stall - #30
Open
clawd131662[bot] wants to merge 1 commit into
Open
fix(trim): bound per-coordinate trimming so history-heavy columns don't stall#30clawd131662[bot] wants to merge 1 commit into
clawd131662[bot] wants to merge 1 commit into
Conversation
…'t stall co:trim located each hot coordinate's keep-th newest row with `ORDER BY rowid DESC`, which the (wid, x, z, time) index can't satisfy, so a column with millions of rows sorted its whole history in one statement and blew past max_statement_time. The raised StatementTimeout aborted the run before save_trim_state, so the checkpoint never advanced and every daily cron re-scanned the same window for ~23h, trimming nothing. - Locate the cutoff via `ORDER BY time DESC` (the index's own order), costing O(keep) look-ups instead of sorting the coordinate's entire history. - Delete everything below the cutoff in bounded `--step` slices, so no single statement scales with the backlog. - Isolate per-coordinate failures: a StatementInvalid on one coordinate is logged and skipped instead of aborting the segment and forfeiting the checkpoint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
co:trim's nightly cron had been running for ~23 hours each night while advancingits checkpoint not at all for ~2 days:
db/trim_state.ymlstayed frozen whileevery run re-scanned the same ~700k-rowid window and then died.
Root cause — to keep a hot coordinate's newest
--keeprows,co:trimran:The
(wid, x, z, time)index can't satisfyORDER BY rowid, so for a coordinatethat had quietly accumulated millions of rows (an end-portal-arrival machine column,
~2.1M rows) MariaDB sorted the coordinate's entire history in a single statement. On
the server's HDD that pluck ran > 3600s, hit
max_statement_time, and raisedStatementTimeout. The exception was caught bytrim'srescuebeforesave_trim_state, so the segment checkpoint was never written — every followingnight restarted from the same rowid and repeated the doomed pluck, trimming nothing.
Fix
ORDER BY time DESC(the index's own order):offset(keep-1).limit(1).pick(:rowid)costs O(keep) look-ups instead of sorting thewhole history. Measured on the exact coordinate that was timing out: 0.118s
(was > 3600s).
--stepslices(
limit(step).pluck→delete_allby rowid), so no single statement scales with thebacklog.
StatementInvalidon one coordinate is logged(
trim_coordinate_skipped_message) and skipped instead of aborting the whole segmentand forfeiting the checkpoint.
Semantics are unchanged — still "keep the newest
--keeprows per(wid, x, y, z, action)". The cutoff is now chosen bytimerather thanrowid; bothare monotonic with insertion, so the kept set matches in practice (the same approach as
the host's proven
trim_grid.shcleanup script).Validation
This repo has no automated suite, so validated manually against the production DB:
ruby -cand rubocop clean (only the pre-existingMetrics/ClassLengthoffense).co:trim --dry-runover a live window: correct output, victims = total − keep onevery line, no runtime errors.
Follow-ups (not in this PR)
never stresses CoreProtect's single-threaded online consumer (routine daily churn is
trivial, but a newly-discovered mega-column currently deletes back-to-back).
README/CLAUDE.md) still use the old "pluck" wording.🤖 Generated with Claude Code