Uh oh!
There was an error while loading. Please reload this page.
[pull] main from jsr-io:main - #132
Merged
Merged
Conversation
…mbols listing (#1542) Tracing showed the R2 download step taking 45-92% of request time on `/docs` and `/source`, spread across a wide, effectively random set of packages, with no download faster than ~130ms. That reads like a storage-latency problem, and the natural conclusion is to put a cache or CDN in front of the buckets. It isn't one, and that fix can't be built as stated. The API reaches the buckets over the S3 API at `<account>.r2.cloudflarestorage.com`, which is not served through Cloudflare's cache and cannot be fronted by one. (A CDN cache in front of the *public* bucket paths already exists — `proxyToR2` serves `jsr.io/@scope/pkg/…` out of `caches.default`. It just isn't in the path the API uses.) The traces are accurate about *where* time goes. What they can't show is that most of those downloads shouldn't be happening at all: the same immutable objects are fetched over and over. Latency per fetch is not really the problem; the number of fetches is. This PR removes the repeats, and fixes two other things found along the way. ## 1. The same immutable object is downloaded on every request `<version>_meta.json` is downloaded once per source-file view, to turn imports into links. It's immutable per version and shared by every file in that version — but nothing cached it. In a one-hour production sample, **76% of the fetches were re-reads of an object already pulled that same hour**, and the busiest package fetched the identical object 202 times. Three other call sites read the same object and also re-download it every time. The README on the package overview is the same shape: immutable per version, re-fetched on every render even when the doc context cache hits. This adds an in-process cache for immutable modules-bucket objects. The S3 path already encodes a published version, so it's a safe key with nothing to invalidate. The package-level `meta.json` is deliberately excluded — it's rewritten on every publish. Two details worth reviewing: - **Absences are not cached.** A 404 leaves the loader as an error so it isn't stored, which keeps an object still being written during publish from being remembered as missing. - **Misses are single-flighted**, so a cold key costs one download rather than one per waiting request. ### On memory The cache is bounded in **bytes**, not entries, and needs no increase to the container's memory limit. `GenerateCtxCache` used `max_capacity(64)`, which counts entries. That had two problems: 64 tiny packages evicted each other exactly as readily as 64 enormous ones (a sample of 2000 docs requests touched 534 distinct packages), and the bytes actually held were bounded by nothing at all — the comment estimated "2-5mb average, 320mb max" but nothing enforced it. It's now weighed by the stored size of the doc nodes each context was built from. So both caches are byte-bounded, and the two together have a smaller worst case than that one cache had on its own. `GenerateCtxCache` misses are single-flighted too. That matters more than for a typical cache: building a context allocates the 10-50 MB that the render permit exists to bound, so concurrent requests for the same cold package multiplied real memory, not just CPU. ## 2. The symbol listing limit doesn't bound the all-symbols page Found while looking at what else was consuming origin capacity, and it turned out to be the larger cost. `SYMBOL_LISTING_LIMIT` (2048) caps how many symbol rows a module listing may contain. Its comment says why it exists: to keep the docs response under Cloud Run's 32 MiB response limit. But it's applied inside `ModuleDocCtx::new`, so it bounds **one module**. `AllSymbolsCtx::new` builds one listing *per entrypoint* and concatenates them, so a package with many entrypoints multiplies straight past the cap. `@ubx/sdk-aws` publishes 1921 exports, putting its ceiling around 3.9 million rows. When the response exceeds the limit, Cloud Run kills the request mid-flight. Every attempt builds the entire listing, spends ~1.6s of CPU, and returns nothing usable. And a 500 carries no cache headers, so retries always reach the origin — a search crawler walking that package's symbol pages produced **871 identical failing requests in one hour**, roughly half of one instance's capacity spent on work that could never succeed. This refuses up front with a **413** instead. The count mirrors what rendering would actually emit, so it errs towards serving rather than refusing. The durable fix belongs in `deno_doc` — the limit should be a total budget rather than a per-module one. When that lands, this guard should stop firing on its own. **Making the refusal cacheable took three changes**, since the outcome is a fixed property of a published version: 1. The API attaches a long-lived `Cache-Control` to the 413 on pinned versions, alongside the existing `EntrypointOrSymbolNotFound` case. 2. `cache_versioned_impl` / `cache_impl` previously attached cache headers only to 404s. 3. `lb/proxy.ts` previously cached only 200s and 404s, so the header would have been inert. ## 3. Both docs search routes were missing `_shared` `docs/search` and `docs/search_structured` are derived purely from the published version, with no permission/member/sudo branch — exactly like `docs` above them, which is already `_shared`. Marking them lets the lb serve them from its shared cache to authenticated callers instead of bypassing the cache whenever auth is present. ## Frontend `LocalSymbolSearch` already disables the input when the index is missing, but `onInput` asserted the index was present. A refused listing now leaves it unset by design, so guard it rather than throw on every keystroke. ## Not in this PR - **The source file bytes and their syntax highlighting.** Both immutable per `(scope, package, version, path)` and both currently uncached, but the working set is much wider than the metadata objects, so it needs a memory budget this PR doesn't try to claim. - **Reading the module graph from Postgres instead of R2.** `source_specifier_links` needs exactly one module's info plus the file list, and Postgres already has the file list. Storing the per-module info at publish would delete the fetch entirely rather than caching it — but that's a migration plus a backfill. ## Testing - 3 new unit tests covering the per-entrypoint accumulation, including the case this fixes: many entrypoints each well under the per-module limit still exceeding the total. - 2 new lb tests: a 413 with a cacheable directive is cached, one without still isn't. - `cargo clippy`, `cargo fmt`, and the lb suite (43 tests) are clean. - The database-backed API tests were not run locally and need CI.
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 freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )