Serve catalogue scale from one exact, cached snapshot - #2003
Merged
Conversation
estimate_open_jobs() backs meta.total on the DB-backed /jobs list, but it estimated `closed_at IS NULL` alone while the list also applies `duplicate_of IS NULL AND NOT is_private`. Every suppressed repost and every private posting was counted in the total and absent from the results. On production the published figure is 5,226,661 against 3,300,658 actual rows — the missing predicate compounded by a reltuples inflated by table bloat. The count also steps rather than drifts, because planner statistics only move when ANALYZE runs. The total stays an estimate: the planner still answers from statistics, so this is O(1) and still not an exact count. It now estimates the right set. Making it exact and stable is the rest of the consistent-catalog-counts change. The test asserts which set is estimated — the result must sit nearer the paginated count than the not-closed one — rather than a tolerance around the exact count. On a 50-row table the planner's selectivity arithmetic carries real error, and that error is not the defect under test. Verified in both directions: the test fails against the old function (32, nearer 40 than 20) and passes against the new one (26).
Three present consumers want the same thing: a value expensive to compute, identical for every caller, tolerable slightly stale. The open-job count, the company count, and /open's six-leg payload — which today memoizes in a single frontend process, so it differs per process and resets on deploy. Cache is two methods over bytes. Errors are reported rather than swallowed: a caller treats both a miss and an error as "no cached value" and falls back, but only one of them is worth logging, and an implementation that hid the difference would make "the backend is down" look like "this key was never written". That is the split ratelimit.Throttler already uses. Two implementations. RedisCache is the shared one — every process reading it agrees, which is the point for a figure published to users. Memory is for tests and for running without Redis; it has no eviction, so it suits a bounded set of long-lived keys and says so. GetJSON/SetJSON are free functions because Go does not allow type parameters on methods. A payload that no longer decodes into T reads as a miss so the caller recomputes, but still returns the error — a deploy that changed T while the previous build's entries are live should be visible, not look like cache churn. Memory aliased the caller's slice in both directions while RedisCache, going through a socket, could not. Code exercised against Memory in tests would have behaved differently against Redis in production, which is the worst shape of leaky abstraction. Memory now copies, both implementations are asserted against the same table-driven test, and no-aliasing is stated in the interface contract.
The package owns the public catalogue-scale numbers so that every surface quoting them quotes the same ones. Before it, /about and /open each took their own estimate at their own moment and could disagree. Compute measures exactly, in one statement. Both totals come from a single CountCatalogueScale so they describe the same instant — counting them separately would let an ingest land between the two reads and publish a company count for a catalogue the job count beside it no longer describes. The predicate is the one the public listings apply, so the totals describe exactly the set a visitor can page through. Load reads and never recomputes. That is enforced by its signature rather than by a test: it takes no ExactCounter, so no read path can reach the catalogue-wide scan even by mistake. A cold cache, an unreachable backend and a payload left by an older build all degrade the same way — the approximate count, the registry-derived figures, and Exact false so a consumer knows which it holds. It returns no error, because every caller is rendering a page and none of them has a decision left to make. The registry figures are derived rather than stored, which turned up a mislabel worth fixing: /open's "166 ATS platforms" counted every registered adapter, and 104 of today's 227 are aggregators while 30 are single-company career feeds. The honest ATS count is 93. Snapshot therefore carries both — Sources for breadth under an accurate label, ATSPlatforms for the narrower question — and neither is a literal any more, so adding an adapter moves them. The 24h TTL deliberately outlives the worker's schedule: a skipped run should degrade to stale-but-exact, not back to fresh-but-wrong.
The exact counts are a full catalogue scan, and this worker is already scanning jobs for the rollups — so the snapshot rides along on a run that is already doing heavier work, rather than paying for a new cron unit, a new timer, and another entry in the release script's worker list. It runs after the rollups commit and never changes the run's exit code. The rollups are this worker's job and they are already done by then; a snapshot that could not be computed or stored is worth a log line, not a failed run that cron will alert on. publishSnapshot returns the error rather than acting on it, so that decision is visible at the call site instead of buried. A missing or unreadable sources/telegram.yml costs one stat, not the snapshot. The counts are the reason this exists, and publishing them with a zero channel count beats publishing nothing and leaving every surface on the estimate. The 24h retention is now asserted rather than asleep in a constant: a recording cache captures what Store asks for, and the test states why it must comfortably exceed the worker's intra-day schedule — a TTL near the cron interval would make one skipped run visible to users as a jump back to the estimate.
GET /api/v1/stats/catalog returns every figure a surface quotes about how big the catalogue is, in one response. That is what stops /about and /open disagreeing: today each takes its own estimate at its own moment, and one snapshot cannot contradict itself. The jobs list's meta.total now reads the same snapshot, so the most-quoted number freehire publishes is the exact count rather than a planner estimate — and the same exact count the transparency page shows. Both reads degrade instead of failing. A cold cache before the first worker run, an unreachable Redis, a payload left by an older build, and no cache configured at all take the same path: the approximate count, and `exact: false` so a consumer knows which it holds. That also repairs a smaller fault in ListJobs, where a failing EstimateOpenJobs returned 500 and took the whole page of jobs with it — the page is the endpoint's payload, and it is not worth trading for one number. handler.Config gains an optional Cache alongside the required Throttler. The server builds it from the Redis client it already constructs for rate limiting, so this adds a use of an existing dependency rather than a new one.
Both pages now make one catalogScale call instead of two list reads, so they cannot show figures measured at different moments — which is the fault this change exists to fix. The /open stat strip stops carrying its own constants. ATS_PLATFORMS = 166 and TELEGRAM_CHANNELS = 88 were literals recounted by hand on deploy, and both had gone stale; worse, 166 was the whole adapter registry under a label that says ATS, while 104 of today's 227 adapters are aggregators and 30 are single-company career feeds. The strip now leads with total sources under an accurate label, live from the registry, and the narrower ATS figure stays available in the API. Verifying it against a running stack caught a real bug. With the cache flushed, /open rendered "290K+ companies" — a build-time constant — at the exact moment the backend had reported it could not measure one. The cause was encoding "not measured" as zero, which a renderer cannot tell from a real count. The loaders now map database-only figures to null on a degraded snapshot, and the two pages answer that differently on purpose: /about falls back to its last-known marketing figure, while /open, the transparency page, drops the stat entirely rather than show a number it cannot source. Verified end to end against a stack seeded with 40 listed and 40 excluded postings: the API, /about and /open all reported 40/1/227/95. Degraded and Redis-down paths both stayed 200.
Two places, because the rule has two audiences. internal/handler/AGENTS.md says how the endpoints work — Load, never count, and pass `exact` through. The root AGENTS.md carries it as a convention, because it binds anything that ever wants to quote catalogue size, not just today's two routes. Recorded there rather than in a new internal/catalogstats/AGENTS.md: the rule is cross-cutting, and the package is too small to justify a module file whose only job would be to repeat it.
|
Warning Review limit reached
Next review available in: 5 minutes Limit details: You’ve used all 2 included reviews currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (43)
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. Comment |
strelov1
added a commit
that referenced
this pull request
Aug 16, 2026
Conflict was in internal/db/jobs.sql.go alone — a generated file, so it was regenerated from the merged queries rather than hand-resolved. #2003's catalogue snapshot queries and this branch's folded-slug column both survive; the full internal/db integration suite passes on the result.
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
1 participant
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.
The problem
/aboutand/openadvertise 5,226,661 open jobs. The catalogue holds 3,300,658. The figure is 58% high, and it steps rather than drifts.Both faults are in
estimate_open_jobs(), whichGET /api/v1/jobsuses formeta.total:WHERE closed_at IS NULLonly. The list it labels also appliesduplicate_of IS NULL AND NOT is_private, so every suppressed repost is counted in the total and absent from the results.reltuples— currently 9,574,771 against 7,356,316 real rows. Bloat inflates it, and it only moves whenANALYZEruns, which is what makes it jump.Measured on production 2026-08-16:
There is a smaller instance of the same fault:
/aboutand/openeach issue their ownlimit=1list read, so two adjacent pages can show two different totals taken at two moments.The change
internal/cache— a small best-effort key/value layer with TTL.Cachereports errors rather than swallowing them, leaving the fail-open decision to one caller, the wayratelimit.Throttleralready does. Two implementations:RedisCache(shared, so every process agrees) andMemory(tests, and running without Redis).internal/catalogstats— owns the figures.Computecounts exactly, both totals in one statement so they describe the same instant.Loadreads and never recomputes — enforced by its signature, which has no exact counter to reach for, not by a runtime check.cmd/rollup-statspublishes the snapshot. It already scansjobsfor the rollups, so this costs one more aggregate rather than a new cron unit. It runs after the rollups commit and never changes the run's exit code.GET /api/v1/stats/catalogreturns every scale figure at once./about,/openand the jobs list'smeta.totalall read it, so they are structurally incapable of disagreeing.Migration 0109 corrects
estimate_open_jobs()to apply the full predicate. It stays approximate — that is its job — but a fallback should not be systematically 58% high.Two things this turned up
The
/open"ATS platforms" figure was mislabelled. The hardcoded166counted every registered adapter. Of today's 227, 104 are aggregators and 30 are single-company career feeds. The honest ATS count is 93. The strip now leads with total sources under an accurate label, live from the registry; the narrower figure stays in the API.TELEGRAM_CHANNELS = 88was stale too (95 configured).Verification caught a real bug. With the cache flushed,
/openrendered "290K+ companies" — a build-time constant — at the moment the backend had reported it could not measure one. The cause was encoding "not measured" as zero, which a renderer cannot tell from a count. Database-only figures now map to null on a degraded snapshot, and the pages answer differently on purpose:/aboutfalls back to its last-known marketing figure,/opendrops the stat rather than show a number it cannot source.Degradation
A cold cache, an unreachable Redis, a payload from an older build and no cache configured at all take one path: the approximate count with
exact: false. This also repairs a smaller fault — a failingEstimateOpenJobsused to return 500 and take the whole page of jobs with it.Verification
go test ./...— 162 packages, clean.go vet ./...andgo vet -tags=integration ./...clean.go test -tags=integration ./internal/db/ ./internal/catalogstats/ ./internal/handler/— clean.pnpm check0 errors;pnpm test1008 tests; eslint clean on changed files./aboutand/openall reported 40 / 1 / 227 / 95. Cache flushed → all three degraded consistently. Redis stopped → both endpoints still 200.Migration 0109 verified applied by the compose initdb, and the integration test fails against the old function (32, nearer 40 than 20) and passes against the new one.
Deploy order
rollup-statsonce by hand to populate the snapshot.Docs branch #1999 corrects the same figures in the README and
docs/sources.md.OpenSpec change:
openspec/changes/consistent-catalog-counts/.