slice: purge every block of an object, not just those before a gap - #13475
Open
masaori335 wants to merge 2 commits into
Open
slice: purge every block of an object, not just those before a gap#13475masaori335 wants to merge 2 commits into
masaori335 wants to merge 2 commits into
Conversation
A PURGE is meant to discard the object, but the block walk stopped at the first block that was not in cache, so any object whose cached blocks were not a contiguous run from block 0 was only partially purged, and the client still got a 200. A gap in the middle left every block behind it cached; an uncached first block purged nothing and relayed that block's 404; and a "bytes=-N" purge deleted the head while leaving the tail it had named. The stop was load-bearing. The walk's only other terminator needs the object length, which slice only ever learned from a 206's Content-Range, and a PURGE response has none. So the core now reports the removed object's extent as X-Purged-Content-Range on a PURGE cache hit, and the walk learns where the object ends from the blocks it is already deleting. It is not Content-Range itself, since that header on a 200 is meaningless under RFC 9110 and cache_range_requests reads the pair as a stored 206 and rewrites the status. PURGE gets its own state machine in the plugin, so it no longer routes through handleFirstServerHeader, whose double duty as "form and emit the client response" is what leaked the 404. A 404 for a block is stepped over, nothing is written downstream until the walk finishes, and the response is then synthesized: 200 if any block was removed, 404 if none was. The extent is taken as a maximum rather than the first value seen, since blocks of one object disagree when the origin object was replaced in place. Until some block reports an extent the walk has no end but a miss bound, so add --purge-probe-blocks, default 8, capping consecutive uncached blocks. It never limits how many blocks a purge removes. A per-request override named by --purge-probe-header, default X-Slice-Purge-Probe, lets an operator who knows the object size widen it. A suffix range names its blocks by distance from an end slice does not know yet, so such a purge is widened to the whole object, a superset of what was asked. A PURGE whose Range cannot be parsed is refused with a 400 rather than guessing which blocks were meant. Tests cover the traversal over gaps, an uncached first block, both open-ended range forms, blocks that disagree about the object length, the miss bound and its override, and the refusal. They measure on the origin rather than the response body, since a purged block and a surviving block are indistinguishable to the client. Two further tests reproduce the client-visible failures of an origin object replaced in place under a child/parent hierarchy, which is how this problem was found.
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
The functional changes and accompanying tests/docs appear coherent and complete, with only a minor documentation wording/capitalization nit noted.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes sliced PURGE behavior in the slice plugin so it purges all relevant cached blocks (including those after gaps), and adds ATS core support to report the purged partial-object extent back to the caller to bound that traversal.
Changes:
- ATS core: on PURGE cache hits for partial objects, emit
X-Purged-Content-Rangebased on the cached response’sContent-Range. - Slice plugin: implement a dedicated PURGE state machine that walks blocks across gaps, learns/updates object extent from
X-Purged-Content-Range, and synthesizes a final200/404response after the walk. - Add AuTest gold tests and documentation describing the new PURGE behavior and configuration knobs.
File summaries
| File | Description |
|---|---|
src/proxy/http/HttpTransact.cc |
Adds X-Purged-Content-Range on PURGE cache hits to expose partial-object extent without misusing Content-Range on 200. |
plugins/slice/util.cc |
Avoids CRR prefetch signaling during PURGE; hardens buffer-reader helper for null readers. |
plugins/slice/server.h |
Declares PURGE-specific response handling and completion helper. |
plugins/slice/server.cc |
Implements PURGE block-walk logic (gap-tolerant traversal, extent discovery, final synthesized response). |
plugins/slice/response.h / plugins/slice/response.cc |
Adds helper to synthesize a PURGE response header (zero-length body). |
plugins/slice/HttpHeader.h / plugins/slice/HttpHeader.cc |
Defines PURGED_CONTENT_RANGE constant and adds HdrMgr::create_response() utility for synthesized responses. |
plugins/slice/Data.h |
Adds PURGE walk state (hit/miss counters, configured/overridden miss bound, purge-range helper). |
plugins/slice/Config.h / plugins/slice/Config.cc |
Adds --purge-probe-blocks and --purge-probe-header configuration. |
plugins/slice/client.cc |
Parses per-request miss-bound override, refuses unparseable PURGE ranges, widens suffix PURGE ranges, and strips override header from internal requests. |
tests/gold_tests/pluginTest/slice/slice_purge_gaps.test.py |
New gold test covering PURGE traversal over gaps, open-ended/suffix ranges, bounds/overrides, and refusal behavior. |
tests/gold_tests/pluginTest/slice/replay/slice_purge_gaps_server.replay.yaml |
Origin-side replay for PURGE gap/bound scenarios. |
tests/gold_tests/pluginTest/slice/replay/slice_purge_gaps_client.replay.yaml |
Client-side replay for PURGE gap/bound scenarios. |
tests/gold_tests/pluginTest/slice/slice_stale_generation.test.py |
New gold test reproducing stale/mixed generation behaviors across a child/parent slicing hierarchy. |
tests/gold_tests/pluginTest/slice/replay/slice_stale_generation_server.replay.yaml |
Origin-side replay modeling object replacement-in-place across phases. |
tests/gold_tests/pluginTest/slice/replay/slice_stale_generation_client.replay.yaml |
Client-side replay asserting stale/mixed generation outcomes. |
doc/admin-guide/storage/index.en.rst |
Documents X-Purged-Content-Range behavior for partial-object PURGE hits. |
doc/admin-guide/plugins/slice.en.rst |
Documents new PURGE traversal semantics and the new probe/bound configuration and override header. |
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Contributor
There was a problem hiding this comment.
🟡 Human review recommended
It changes both core PURGE response semantics and slice’s internal state machine behavior, so a final human review should validate integration/runtime effects beyond the added gold tests.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
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.
Motivation
A sliced PURGE only removed the blocks before the first gap. The walk stopped at
the first block that was not in cache, so any object whose cached blocks were not
a contiguous run from block 0 was partially purged — and the client still got a
200. A gap in the middle left every block behind it cached; an uncached firstblock purged nothing and relayed that block's
404; andbytes=-Ndeleted thehead while leaving the tail it had named.
The stop was load-bearing rather than a stray early return: the walk's only other
terminator needs the object length, which slice learns from a
206'sContent-Range, and a PURGE response has none.Changes
ATS core — report the purged object's extent as
X-Purged-Content-Rangeon aPURGE cache hit, so a caller holding one piece of a larger resource can learn its
extent. Deliberately not
Content-Range: on a200that is meaningless under RFC9110, and
cache_range_requestsreads the pair as a stored206being served as200and rewrites the status.Slice plugin — give PURGE its own state machine, so it no longer routes through
handleFirstServerHeaderand cannot leak a block's404to the client. It walksevery block, steps over a
404, takes the extent as the largest any block reports(blocks disagree when the origin object was replaced in place), and synthesizes the
response only once the walk finishes:
200if any block was removed,404if none.Until some block reports an extent the walk is bounded by
--purge-probe-blocks(default 8), overridable per request via
--purge-probe-header. A suffix range iswidened to the whole object; an unparseable range is refused with
400.Tests
slice_purge_gapscovers the traversal over gaps, both open-ended range forms, themiss bound and its override, and the refusal.
slice_stale_generationreproducesthe client-visible failures of an origin object replaced in place, which is how
this was found.