Repository navigation
Reduce recurring query spills and database work - #73
Conversation
PR reader API performanceResult: INCONCLUSIVE 8 inconclusive. Zero request failures across all six runs. Base Three independent runs per revision, each on a fresh GitHub-hosted ARM64 runner. 16 users, 60 seconds/run, 1,000 synthetic articles, cache off, five PgBouncer slots. Latency comparisonAll latency values are in milliseconds; lower is better. Values are medians of three runs.
Additional latency and throughput metricsEach cell shows base → PR. Latencies are in milliseconds.
Individual runs
How to interpret this reportRegressed / Improved: a p95 regression needs >20% and >20 ms increase in the medians and at least two head samples versus the base median. Improvements use the inverse threshold. Within threshold means the change did not meet both thresholds. Inconclusive: a within-revision p95 range >35% of its median and >20 ms is too noisy for a confident latency verdict. Inconclusive latency comparisons are non-blocking: the check passes without claiming an improvement. Confirmed regressions and test/report errors fail the PR gate. Throughput is descriptive: paced users do not measure maximum capacity. This uncached public API test does not establish production health, browser performance or worker capacity. Separate runners can differ in hardware or host load; repetitions and noise checks reduce but cannot eliminate that uncertainty. Full HTML/CSV reports and logs · attempt 2 |
Code Review ✅ Approved🟡 Medium risk · Two recurring database queries change execution plans while preserving reported counts. Reduces tag reconciliation query time by ~31% and 30-day publication reporting by ~43% through early deduplication and projection-before-grouping to minimize temporary writes. Comprehensive regression tests, performance validation across PostgreSQL 17–18, and production diagnostics confirm exact result preservation with zero tag-count spills and ~96% reduction in reporting temporary writes. No issues found. OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
1 participant
What changes and why?
Recurring tag reconciliation hashes hundreds of thousands of assignments, while admin publication reporting sorts full review automation JSON. The production audit measured tag counting at 577 ms and publication reporting at 1.98 seconds, with substantial temporary writes.
Count distinct visible tag UUIDs before the final count. For publication reporting, project each eligible review to its article UUID and automatic/manual booleans before grouping; an
OFFSET 0boundary prevents PostgreSQL from pulling the full JSON back into the sort. Exact publication, review and approved-origin visibility checks, manual-intervention rules, publication-time cutoffs and median calculations are preserved. Both paths reuse existing indexes.Add semantic regressions and a production-scale plan comparison, plus profiling guidance and a user-facing changelog entry.
Validation
Updated against master after #69, #71 and #72. The changelog conflict retains every fix. Current commit:
8b83f32ac4efbb4607ec40d0484910384e571e7d.After the branch update, 46 PostgreSQL 18/Redis regressions passed, including exact result semantics and the production-scale query-plan comparison. Two unit cases were excluded from that integration-only command. Markdown formatting, whitespace checks and commit hooks passed.
Current-head CI, attempt 2: 2,620 backend unit tests, 1,132 integration tests, 785 web tests and 647 admin tests passed, with no skips. Populated migration upgrades, web/Chrome/Edge browser parity, admin and authenticated API journeys, all ARM64 image builds/scans/smoke checks and all three CodeQL analyses passed.
CI requiredand both Sonar checks passed.Sonar MCP confirms the quality gate is OK, with 100% new-code coverage, 0% new duplication, and zero new bugs, vulnerabilities or code smells.
The first performance attempt reported latest-feed p95 99 → 120 ms and topic-feed 61 → 90 ms, with zero request failures. Neither route imports or calls the changed reconciliation/reporting queries; the measured public API files are identical to the base. A full repeat on the same commit passed with zero failures and eight inconclusive route comparisons. Runner variation prevents a latency claim from that comparison; no code or regression threshold was changed to obtain the repeat result.
Earlier targeted PostgreSQL 17 checks also passed all 12 new regressions and 62 surrounding integration cases. Full frontend/browser and integration validation of the updated commit is supplied by hosted CI rather than a complete local rerun.
GitHub's separate managed AI findings scan remains unavailable because the monthly quota is exhausted (HTTP 402). Standard CodeQL completed successfully; no paid service was enabled.
The fixture contains 60,340 articles, 34,074 tags, 603,400 tag assignments and 153,680 reviews. It is entirely synthetic and runs only against explicitly configured disposable databases and Redis database 15.
Driver-ready application SQL is compared with the previous production queries at
work_mem=4MB. Results match exactly. Regression budgets require zero tag-count temporary writes and reporting spills below 8 MiB and below 15% of the previous broad-window spills. Timing is diagnostic, not a CI threshold.Paired local PostgreSQL 17 medians (three repetitions per query, same data and connection):
The tag count is about 31% faster; the default 30-day report is about 43% faster. Reporting temporary writes fall about 96%. PostgreSQL 18 also passes the plan budgets, with zero reporting spills on this fixture. Small reporting windows remain similar in latency.
Read-only production correlation (PostgreSQL 17.10, ARM64,
work_mem=4MB):sum(increase(cnpg_pg_stat_database_temp_bytes{datname="devfeed"}[24h]))). This is a database-wide total, not attribution to these two queries.pg_stat_statementswindow recorded 14 existing tag-count calls averaging 374.0 ms, writing 225.5 MB of temporary data. No existing publication-report calls occurred in that window; its 1.98-second finding comes from the earlier audit interval.EXPLAIN (ANALYZE, BUFFERS)diagnostics showed tag deduplication with zero temporary writes, and the proposed 30-day publication query with 2.23 MiB, versus 50.83 MiB for the previous query. These diagnostic executions do not change the deployed application.Post-deployment latency and daily temporary-write reductions still need measurement.
Rollout notes
master; scoped to these two query paths, their regression coverage and documentation.Ready for review