Repository navigation
Conversation
With cache_metadata on, every cache hit read the whole stored document into its own buffer before any coalescing, since coalesceMetadata only covers upstream misses. metadata_max_size bounds one request, not the sum: 30 concurrent hits on a 20 MB document held about 820 MB. readCachedMetadata now goes through metadataReads, keyed by the row's storage path and content digest. Requests arriving while a read runs wait for it, and requests arriving later reuse its bytes while any earlier request still holds them, so one document is in memory once however many requests serve it. The map only holds weak pointers, so it keeps nothing alive, and a cleanup drops the entry once the bytes are collected. validate and the hit bookkeeping still run per request. Callers already had to treat metadata bodies as read-only, because coalesced misses hand every waiter the same slice. The shared slice has no spare capacity, so an append copies. Rows without a digest are read as before.
|
Thanks for measuring this. The 820 MB figure is a real problem, but I don't want to fix it with |
This reverts commit be13df1.
With cache_metadata on, every metadata cache hit read the whole stored document into its own buffer before any coalescing, since coalesceMetadata only covers upstream misses. metadata_max_size bounds one request, not the sum: 30 concurrent hits on a 20 MB document held about 820 MB. The handlers that go through ProxyCached serve the stored document unchanged, so a hit there no longer needs the bytes in memory. serveCachedMetadata streams the blob from storage to the response instead, with the row's Content-Type, Content-Encoding, size, ETag and Last-Modified, and answers If-None-Match and If-Modified-Since with 304 as before. The bytes are checked against the row's content digest as they are sent, and a mismatch aborts the response like a corrupt cached artifact. A row without a digest or size, a document over the size limit, or a blob that can't be opened takes the buffered path as before. Handlers that rewrite or filter metadata keep reading it into memory, and the rewrite cache stays in front of them. writeMetadataValidators is split out of writeMetadataCachedResponseWithEncoding so both paths share the conditional request handling.
simonchrz
changed the title
|
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
left a comment
andrew
left a comment
There was a problem hiding this comment.
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.
With
cache_metadata: true, every metadata cache hit read the whole stored document into its own buffer. Coalescing only covers upstream misses, andmetadata_max_sizelimits one request, not all of them together. 30 concurrent hits on one cached 20 MB document held about 820 MB.Handlers that go through
ProxyCachedserve 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:
writeMetadataValidators, shared by both paths.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:
validatefunction that needs the whole body.The earlier shared-read commit is reverted on the branch.
TestConcurrentMetadataCacheHitsStreamcaches 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.TestCachedMetadataHitAbortsOnDigestMismatchcorrupts the stored blob and expects the response to be aborted. Both fail on main.