Repository navigation
Release catalog locks before worker ranking - #72
Conversation
PR reader API performanceResult: INCONCLUSIVE 7 inconclusive · 1 within threshold. 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 1 |
Code Review ✅ Approved🟡 Medium risk · Worker publication preparation retries on catalog/content changes before applying results. Moves catalog loading and ranking outside application locks, reducing lock-holding time from 602.2 ms to 52.4 ms (91.3% reduction) on synthetic production-scale data. Workers now prepare candidate results before acquiring locks, verify catalog revisions under locks, and reuse prepared data for validity and publication checks. Comprehensive testing confirms identical published results with no regressions across 1,120 integration tests. 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?
Analysis workers still loaded and ranked the full topic/tag catalog while holding source, article and topic
SHARElocks. They repeated catalog work during publication checks and when queueing replacements for superseded results. A catalog writer had to wait for that Python work to finish even when it was unrelated to the edited topic.The original production audit measured 541 ms average topic write-lock execution. A new read-only PostgreSQL window (2026-10-03 23:04:00–23:06:06 UTC) recorded 11 write-lock calls averaging 345.6 ms. It caught a writer waiting on
ShareRowExclusiveLock, blocked by a reader holdingShareLockwhile idle in its transaction onClientRead(reader transaction age 608 ms, writer transaction age 318 ms). These are production baseline observations; production remains unchanged.Workers now load the current catalog and article evidence, close the read transaction, and prepare candidate results and identity membership before acquiring application locks. Under the existing locks they verify PostgreSQL catalog revisions and the prepared article content, then reuse those results for validity, evidence, publication and superseded-job checks. A change rolls back the application transaction and prepares again outside locks, reusing the same inference result. After three preparation attempts, continued churn follows the existing durable dependency-failure retry policy. Job leases, source approval, editorial revisions, evidence validation and publication policy keep their existing checks.
Inference prompt ranking also runs after its read transaction closes. Full-catalog reuse scopes start after inference returns, so this change does not retain a full catalog through the external model wait.
On a synthetic catalog matching production size (4,450 topics and 34,057 tags), median catalog lock holding fell from 602.2 ms to 52.4 ms (91.3% less) across three paired runs, with identical published results and one inference call per run. The comparison used the parent PR worker versus this worker, warm Redis/catalog indexes, a fake model and disposable PostgreSQL. Timing runs from successful table-lock acquisition through database commit completion; fixture setup is excluded. This measures local lock holding, not production write-lock wait latency.
Related issues
Depends on #71 for revision-checked catalog reuse and migration
0022. #71 is merged, and this PR now targets master. The branch is updated after #69 and #71; its remaining diff contains only the worker follow-up. Merge this lock fix before updating the overlapping service-boundary refactor (#79).Validation
bash .github/scripts/python-checks.sh: Ruff lint/format, mypy, version consistency and pre-commit configuration passed.Rollout notes
This follow-up adds no migration. It requires #71 and its migration
0022so topic description edits invalidate retrieval revisions. Deploy through the normal migration-first release. The dependency is merged; deploy its migration before starting the updated services.Repeated edits can exhaust the three local preparation attempts. Existing dependency retries may rerun inference on a later delivery, with the existing attempt limit and backoff. No stale result is applied during churn. The lock order and topic lock modes remain unchanged; the improvement comes from moving catalog computation outside the critical transaction.
After release, compare topic write-lock mean latency, blocking chains and scheduler/worker resource use over matching windows. This work did not mutate or deploy production.
Merge queue validation
Retargeted to master after #71 merged, then updated against that merge. The changelog conflict retains the worker-lock fix, scheduler/catalog fix and authentication-redaction fix. 76 analysis/unit regressions passed. 28 focused real PostgreSQL 18 / Redis 8 lock and catalog regressions passed, with no skips. They verify concurrent writers, ranking without held connections/locks, stale-input and ownership rejection, and bounded churn retries. Disposable services were removed after the run. Full hosted CI,
CI required, CodeQL and Sonar passed on commite7b7f671. Hosted suites passed with 2,620 backend unit tests, 1,120 integration tests, 785 reader tests and 647 admin tests, with no skips. Populated-schema upgrades, web/Chrome/Edge parity and all ARM64 image build/scan/smoke checks passed. Sonar MCP confirms 95.6% new-code coverage, 0.0% duplication, and zero new bugs, vulnerabilities or code smells. The separate GitHub-managed AI review remains quota-blocked, independently of these passing checks. Auto-merge is disabled.Ready for review