slice: purge every block of an object, not just those before a gap - #13475
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.
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.
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.
Both values are client supplied, so a bad one repeats as fast as requests arrive. Route them through Config::canLogError() like the other slice error paths.
The requested range end comes from the client's Range header, but the walk only consulted it once some block had reported the object's extent, which only a block that was actually removed can do. A closed-range PURGE whose leading blocks were uncached therefore ran past its range end and removed blocks the client never named.
There was a problem hiding this comment.
🟢 Ready to approve
The changes are cohesive across core/plugin/docs, and the new gold tests directly cover the previously incorrect PURGE traversal and the reported failure scenarios.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
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.
A block PURGE answering neither 200 nor 404 was read as "already absent", so a 403 from ip_allow or a 502 counted as a miss and the walk carried on to answer 200 on the strength of blocks it had removed earlier, telling the client the object was gone while part of it was still cached. Such a status says nothing about the blocks behind it either, so the walk now stops there and reports it.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (1)
plugins/slice/server.cc:665
- handle_purge_resp treats any parse result other than TS_PARSE_CONT as a successfully parsed header and calls note_purge_block_result(). If TSHttpHdrParseResp returns TS_PARSE_ERROR (malformed/oversized response), HttpHeader::status() will be TS_HTTP_STATUS_NONE and note_purge_failure() will set m_purge_error to NONE, so the purge walk can continue as if nothing went wrong. This should be handled as a hard failure (e.g. 502) that stops the walk and is reported to the client.
TSParseResult const res = data->m_resp_hdrmgr.populateFrom(data->m_http_parser, reader, TSHttpHdrParseResp, &consumed);
TSVIONDoneSet(input_vio, TSVIONDoneGet(input_vio) + consumed);
if (TS_PARSE_CONT == res) {
return;
}
data->m_server_block_header_parsed = true;
note_purge_block_result(data);
}
…13475) * slice: purge every block of an object, not just those before a gap 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. * Doc: Fix example of HTTP/1.1 messages * slice: pace the two PURGE request-validation error logs Both values are client supplied, so a bad one repeats as fast as requests arrive. Route them through Config::canLogError() like the other slice error paths. * slice: let a PURGE range bound the walk before any extent is known The requested range end comes from the client's Range header, but the walk only consulted it once some block had reported the object's extent, which only a block that was actually removed can do. A closed-range PURGE whose leading blocks were uncached therefore ran past its range end and removed blocks the client never named. * slice: do not report a partial PURGE as a success A block PURGE answering neither 200 nor 404 was read as "already absent", so a 403 from ip_allow or a 502 counted as a miss and the walk carried on to answer 200 on the strength of blocks it had removed earlier, telling the client the object was gone while part of it was still cached. Such a status says nothing about the blocks behind it either, so the walk now stops there and reports it. (cherry picked from commit 48fc842)
|
Cherry-picked to the 10.2.x branch as 430ba4b for the 10.2.0 release. |
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.