Skip to content

perf(cleaner): walk only metadata rows in the TTL sweep and reconcile - #278

Merged
ovaistariq merged 1 commit into
mainfrom
fix/cleaner-prefix-bounded-scans
Sep 27, 2026
Merged

ovaistariq merged 1 commit into
mainfrom
fix/cleaner-prefix-bounded-scans

Conversation

@ovaistariq

@ovaistariq ovaistariq commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Problem

cleanupExpiredKeys (every cleaner tick, 1 min by default) and reconcileFromMetadata (startup, then hourly) both started at SeekToFirst and skipped every row that is not under !meta/. The eviction indexes, back-references, compaction and deletion queues live in the same RocksDB, and under LRU there are two index rows per key, so each pass read three rows for every one it could act on. The cost scales with the number of keys, which for TAG's 1 MiB blocks is the cache size in MiB.

Change

  • Both loops seek to keys.MetadataPrefix and stop with ValidForPrefix. The per-row prefix check is gone.
  • New counter ocache_cleaner_rows_scanned_total{scan="ttl"|"reconcile"} and a scanned field on each pass's completion log, so the walked-row count is visible in production.
  • TestCleanerScans_WalkOnlyMetadataRows seeds 1000 rows in keyspaces sorting on both sides of the prefix plus five metadata rows, and asserts the sweep walks exactly 5 and the reconcile exactly 3 (after two expiries are deleted), leaving the other rows intact.

make lint and make test-storage pass.

Closes perfloop cases case_hb73egdr2s and case_456vm3wag5 (the prefix bound is the useful half of the latter; a separate expiry index is not worth its write amplification).

🤖 Generated with Claude Code

https://claude.ai/code/session_01YCoN4Him5ZY4hZovVp37ra


Note

Medium Risk
Changes core cleaner iteration for TTL deletion and size/eviction-index reconciliation; behavior should be equivalent but mis-scoped prefix iteration could miss rows or skew totals.

Overview
Limits cleaner scans to metadata keys so periodic TTL expiry and startup/hourly size reconciliation no longer walk the full RocksDB and skip eviction indexes, back-references, and queues. Both paths now Seek to keys.MetadataPrefix and iterate with ValidForPrefix, removing per-row IsMetadataKey checks.

Adds ocache_cleaner_rows_scanned_total{scan="ttl"|"reconcile"} and a scanned field on the TTL and reconcile completion logs. TestCleanerScans_WalkOnlyMetadataRows seeds foreign keyspaces and asserts only metadata rows are counted and touched.

Reviewed by Cursor Bugbot for commit 769fcdf. Bugbot is set up for automated code reviews on this repo. Configure here.

Both the per-minute TTL sweep and the startup and hourly size
reconciliation iterated from the first key of the database and skipped
every row outside the metadata prefix. The eviction indexes,
back-references, compaction and deletion queues share the database, and
under LRU those rows outnumber the metadata rows two to one, so each pass
read three rows for every one it could act on, on a row count that grows
with the cache.

Seek to the metadata prefix and stop when it ends. A new counter,
ocache_cleaner_rows_scanned_total{scan="ttl"|"reconcile"}, and a
"scanned" field on each pass's summary log expose the rows walked; the new
test seeds rows on both sides of the prefix and pins the count to the
metadata rows alone.

Perfloop cases case_hb73egdr2s and case_456vm3wag5 (the prefix bound is
the useful part of the latter; an expiry index is not worth its write
amplification).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YCoN4Him5ZY4hZovVp37ra
Signed-off-by: Ovais Tariq <ot@tigrisdata.com>
@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Optimizes the metadata cleanup sweep to skip non-metadata rows.

The PR appears safe to merge.

Summary

The PR bounds TTL cleanup and size reconciliation to metadata rows, adds scanned-row telemetry, and tests that both scans leave other keyspaces untouched.

Reviews (1) · Last reviewed commit: "perf(cleaner): walk only metadata rows i..."

@ovaistariq
ovaistariq merged commit 13e7fb6 into main Sep 27, 2026
9 checks passed
@ovaistariq
ovaistariq deleted the fix/cleaner-prefix-bounded-scans branch September 27, 2026 07:19
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