Skip to content

perf(metadata): count refresh debt attempt buckets in one scan - #2083

Open
blurbery wants to merge 1 commit into
Silo-Server:mainfrom
blurbery:perf/metadata-refresh-debt-metrics-one-scan
Open

blurbery wants to merge 1 commit into
Silo-Server:mainfrom
blurbery:perf/metadata-refresh-debt-metrics-one-scan

Conversation

@blurbery

@blurbery blurbery commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Related issue: N/A
Validation tasks: none

Every read of the metadata refresh metrics scans the whole metadata_refresh_debt table five times just to count its attempt buckets. GetMetrics builds the five counts (0, 1, 2-3, 4-7, 8+) from five SELECTs joined by UNION ALL, and each SELECT reads the full table. The cost grows with the debt backlog, which has one row per item, season and episode with refresh debt.

This change counts all five buckets in one scan.

Approach

One SELECT with a COUNT(*) FILTER per bucket replaces the union. The labels, their order and the counts in the response are unchanged: the old query's ORDER BY label sorted the same five labels into the same order the new code builds. Only the attempt-bucket query changes; GetMetrics' other queries are untouched.

Validation

  • go test ./internal/metadata/ -count=1: pass, with and without a migrated local PostgreSQL 18 database.
  • TestBuildMetadataRefreshAttemptBucketsPreservesLabelsAndCounts checks the label order and counts.
  • TestMetadataRefreshAttemptBucketCountsSQLUsesOneAggregateScan checks that the query reads metadata_refresh_debt once, without UNION, and with one filter per bucket in label order.
  • The benchmark below also checks that the old and new queries return the same five counts on every seeded table.
  • go vet ./internal/metadata/ and make lint-changed: clean.
  • CI on 836b3f9cb: passed.

Benchmarks

Before is the attempt-bucket query on main (52096ba88); after is this branch's query. Old and new returned identical counts at every size.

Local runs: PostgreSQL 18.6 (Homebrew) on an Apple silicon Mac, in a disposable migrated database with fsync off and 256 MB of shared buffers. Each run seeded metadata_refresh_debt with N rows, attempt_count spread over 0 to 12, then ran VACUUM ANALYZE and three warm-up runs. The timings are 15 alternating EXPLAIN (ANALYZE, BUFFERS) runs of each query.

Rows Before, median (range) After, median (range) Table scans Shared buffer hits
10,000 2.53 ms (2.46-3.57) 0.74 ms (0.70-0.83) 5 → 1 573 → 114
100,000 12.95 ms (12.75-13.36) 5.13 ms (5.02-5.35) 5 → 1 5,805 → 1,137
1,000,000 125.1 ms (119.9-133.1) 28.5 ms (27.6-29.8) 5 → 1 61,360 → 12,248

My own server: PostgreSQL 18, 3,742 debt rows in 624 pages. Read-only EXPLAIN (ANALYZE, BUFFERS) inside a read-only transaction, 7 runs of each query:

  • Before: median 18.3 ms (14.3-23.4). Five sequential scans under a parallel append with 3 workers, 3,258 shared buffer hits.
  • After: median 1.76 ms (1.47-2.04). One sequential scan, 624 shared buffer hits.

Limitations: these time the query alone, not the metrics endpoint, which runs other queries as well. Local numbers come from a laptop with fsync off; they show the scan count, not production latency.

Evidence

No user-visible change. The metrics response has the same buckets, labels, order and counts.

Evidence: https://evidence.siloserver.org/r/silo-server/pr-2083/

Risks

None identified. Same result set from one statement, no migration.

Checklist

  • I read and can explain the complete diff.
  • This pull request addresses one concern.
  • The Evidence section shows every change a user can see, or says there is none.

AI Disclosure

  • Harness: Claude Code (desktop app); the change itself first came from an AI-assisted commit on my fork, and the tool used there was not recorded
  • Tool(s): Claude Code, psql
  • Model(s): claude-opus-5-5
  • Involvement: AI-assisted
  • Adversarial review: n/a. This is a one-query change, with result equality checked in the benchmark.

AI-assisted with Claude Opus. I directed the task and designed the work.

GetMetrics counted the five attempt buckets with five SELECTs joined by
UNION ALL, so every metrics read scanned metadata_refresh_debt five times.
One SELECT with a filtered COUNT per bucket gives the same labels, order
and counts from a single scan.
@silo-kody

silo-kody Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Silo Kody — review complete

Review finished. Check the inline comments for findings and verify each suggestion against the code and tests.

Reviewing changes in Silo
  • Include the related issue, expected behavior, and validation steps in the PR description.
  • For API changes, describe the effect on Apple and Android clients and Jellyfin compatibility.
  • For plugin changes, identify the affected SDK contract, plugin, and catalog entry.
  • Follow this repository's AGENTS.md and CONTRIBUTING.md.
  • Request another review with @kody start-review in a PR comment.
  • React with 👍 or 👎 to give feedback on individual suggestions.
Review settings
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 43 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cba4c391-fab6-4fa4-a9f1-54857a7f96ef
📥 Commits

Reviewing files that changed from the base of the PR and between ca186fe and 836b3f9.

📒 Files selected for processing (2)
  • internal/metadata/refresh_debt_repo.go
  • internal/metadata/refresh_debt_test.go
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Quick104 Quick104 added priority: P3 Needs-info or parked impact: perf Unusable slowness on normal hardware labels Oct 8, 2026 — with Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact: perf Unusable slowness on normal hardware priority: P3 Needs-info or parked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants