[simonchrz]

With cache_metadata: true, every metadata cache hit read the whole stored document into its own buffer. Coalescing only covers upstream misses, and metadata_max_size limits one request, not all of them together. 30 concurrent hits on one cached 20 MB document held about 820 MB.

Handlers that go through ProxyCached serve the stored document unchanged, so their cache hits now stream the blob from storage to the response (serveCachedMetadata) instead of reading it into memory. Nothing is shared between requests.

The streamed response keeps what the buffered one had:

• Content-Type, Content-Encoding (gzip stays verbatim for conda and Homebrew), Content-Length, ETag and Last-Modified come from the cache row.
• If-None-Match and If-Modified-Since still get a 304. The check is now writeMetadataValidators, shared by both paths.
• The hit is still recorded for eviction and in the metrics.
• The bytes are checked against the row's content digest as they are sent. A mismatch aborts the response, records an integrity failure and deletes the row, queueing its blob for deletion, like a corrupt cached artifact (Abort the response when a cached artifact fails its integrity check #504). The next request fetches the document from upstream. The buffered path doesn't check this.

A row without a digest or size, a document over metadata_max_size, or a blob that can't be opened falls back to the buffered path. Stale responses (upstream down) also stay buffered. They come from the coalesced miss path, which already gives concurrent callers one shared body.

Hits stream for: apk, arch (package files), conan, conda, cran, debian, freebsd, generic, gem, go (module proxy), hex, homebrew, julia, rpm.

Hits still buffer:

• npm, composer, chef: rewritten, with the rewrite cache in front as before.
• npm and composer hosted, helm: rewritten.
• cargo: cooldown/denylist filtering.
• pypi, pub, swift, nuget: parsed or rewritten.
• go sumdb, arch databases: checked by a validate function that needs the whole body.
• maven: served unchanged, but through its own fallback path. It could stream later. I left it alone here because Hash path-based metadata cache keys #578 changes its cache keys.

TestConcurrentMetadataCacheHitsStream caches a 4 MB document, then sends 10 concurrent hits with storage readers that stop after the first 64 KB. Every response has to start while its reader is stopped, so a hit that reads the whole document first fails the test. It also checks the body, Content-Type, Content-Length, ETag and a 304. TestCachedMetadataHitAbortsOnDigestMismatch corrupts the stored blob, expects the response to be aborted and the next request to be fetched from upstream. Both fail on main.

[andrew]

Thanks for measuring this. The 820 MB figure is a real problem, but I don't want to fix it with unsafe.Slice over a weak pointer. It would be the first unsafe code in the proxy, and it makes every metadata body shared across requests with nothing to enforce that it stays read-only. One handler that writes into its body in place would then corrupt that document for every concurrent and later request, where today the damage stays inside one request. I'd rather solve the memory use without shared mutable buffers: stream cache hits for the handlers that pass the document through unchanged, and keep the rewrite cache in front of the ones that rewrite it.

[simonchrz]

Agreed, I've dropped the shared buffer and reverted that commit. Cache hits through ProxyCached now stream the stored blob straight to the response, checking it against the row's digest on the way and aborting on a mismatch like artifacts do. The rewriting handlers (npm, composer, chef and the others listed in the description) still read the document and use the rewrite cache as before. Nothing is shared between requests any more.

[andrew]

Thanks, this is the shape I was after: streaming for the pass-through handlers, the rewriting ones unchanged, and nothing shared between requests.

The digest mismatch callback in serveCachedMetadata only logs. The artifact path it follows also records metrics.RecordIntegrityFailure and drops the record with DiscardArtifact, so the next request fetches again. Here the row stays, so every hit on a corrupt blob is aborted until the TTL runs out. After that the buffered path revalidates with the row's ETag, a 304 refreshes fetched_at over the same blob, and streaming aborts again. The document stays broken until upstream changes it. Please record the failure and drop the row in the callback, as eviction does, and extend TestCachedMetadataHitAbortsOnDigestMismatch so the request after the aborted one is fetched from upstream.

Please also drop be13df1 and its revert 6ad1610 from the branch, so the shared-buffer commit doesn't end up in main's history.

[simonchrz]

Thanks. The mismatch callback now records an integrity failure and deletes the row in the same transaction that queues its blob, only if the row still has that path and digest, so a newer fetch isn't dropped. TestCachedMetadataHitAbortsOnDigestMismatch now also checks that the request after the aborted one is fetched from upstream. be13df1 and its revert are gone; the branch is rebased on main.