Cache empty chunked responses#13410
Open
bneradt wants to merge 1 commit into
Open
Conversation
Empty chunked responses never start a cache write VIO, so ATS aborts the cache entry and sends later requests back to the origin. This starts a zero-byte cache write and preserves the empty-document state through cache setup. It also adds coverage for negative and successful responses. Fixes: apache#11313
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a cache correctness gap where chunked responses with an empty body could fail to commit cache metadata (leading to later requests going back to origin), and adds replay-based coverage to prevent regressions. The change is implemented by ensuring cache writes create a VIO even for zero-byte bodies and by teaching the cache write path to treat an explicit zero-byte write as an empty document.
Changes:
- Start a zero-byte cache write VIO for empty-body cache writes so closing the cache VC commits (rather than aborts) the entry.
- Mark zero-byte cache writes as “empty document” in
CacheVCand ensure object size is set to 0 in that case. - Extend the negative-caching replay to cover empty chunked bodies for both negative (404) and successful (200) responses.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml | Adds replay coverage for caching empty chunked 404 and 200 responses (issue #11313). |
| src/proxy/http/HttpTunnel.cc | Ensures cache-write consumers get a VIO even when the computed write size is 0. |
| src/iocore/cache/CacheVC.cc | Treats do_io_write(..., nbytes=0, ...) as an empty-document write and propagates that into cache metadata handling. |
Comment on lines
+213
to
+217
| if (nbytes == 0) { | ||
| // A zero-byte write represents an empty document, while closing without a | ||
| // write represents a header-only update. | ||
| f.allow_empty_doc = 1; | ||
| if (alternate.valid()) { |
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
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.
Empty chunked responses never start a cache write VIO, so ATS aborts
the cache entry and sends later requests back to the origin.
This starts a zero-byte cache write and preserves the empty-document
state through cache setup. It also adds coverage for negative and
successful responses.
Fixes: #11313