serrislew commented on code in PR #13475:
URL: https://github.com/apache/trafficserver/pull/13475#discussion_r3732148482
##########
plugins/slice/server.cc:
##########
@@ -499,12 +502,174 @@ handleNextServerHeader(Data *const data)
return true;
}
+// Take the largest extent any block reports: blocks disagree when the origin
+// object was replaced in place, and the shorter one would leave a tail cached.
+void
+note_purge_extent(Data *const data, int64_t const length)
+{
+ if (length <= data->m_contentlen) {
+ return;
+ }
+
+ data->m_contentlen = length;
+ DEBUG_LOG("purge extent now %" PRId64 ", walking through block %" PRId64,
length,
+ data->purge_range().lastBlockFor(data->m_config->m_blockbytes));
+}
+
+// Record what the block response said, without answering the client.
+void
+note_purge_block_result(Data *const data)
+{
+ HttpHeader const header(data->m_resp_hdrmgr.m_buffer,
data->m_resp_hdrmgr.m_lochdr);
+ DEBUG_LOG("Purge block header\n%s", header.toString().c_str());
+
+ TSHttpStatus const status = header.status();
+
+ if (TS_HTTP_STATUS_OK == status) {
+ ++data->m_purge_hits;
+ data->m_purge_misses = 0;
+
+ // Not Content-Range: cache_range_requests reads that on a 200 as a stored
206
+ // and rewrites the status
+ ContentRange const purgedcr = content_range_for_key(header,
PURGED_CONTENT_RANGE.data(), PURGED_CONTENT_RANGE.size());
+ if (purgedcr.isValid() && 0 < purgedcr.m_length) {
+ note_purge_extent(data, purgedcr.m_length);
+ } else {
+ DEBUG_LOG("Purged block %" PRId64 " reported no usable extent",
data->m_blocknum);
+ }
+ } else {
+ // Already absent. The walk used to stop here, leaving every later block
cached.
+ ++data->m_purge_misses;
+ DEBUG_LOG("Purge block %" PRId64 " was not cached (%d)", data->m_blocknum,
status);
+ }
+}
+
+// Issue the next purge, or answer the client if the walk is over.
+void
+advance_purge(TSCont const contp, Data *const data)
+{
+ int64_t const blockbytes = data->m_config->m_blockbytes;
+ Range const range = data->purge_range();
+
+ ++data->m_blocknum;
+ int64_t const firstblock = range.firstBlockFor(blockbytes);
+ if (data->m_blocknum < firstblock) {
+ data->m_blocknum = firstblock;
+ }
+
+ // The requested range bounds the walk whether or not the extent is known yet
+ if (!range.blockIsInside(blockbytes, data->m_blocknum)) {
+ finish_purge(contp, data);
+ return;
+ }
+
+ // An open ended range has no such bound until some block reports an extent
+ if (data->m_contentlen < 0 && data->m_purge_miss_bound <=
data->m_purge_misses) {
+ DEBUG_LOG("purge gave up after %d consecutive uncached block(s)",
data->m_purge_misses);
+ finish_purge(contp, data);
+ return;
+ }
+
+ data->m_blockstate = BlockState::Pending;
+ if (!request_block(contp, data)) {
+ ERROR_LOG("Failed to issue purge for block %" PRId64, data->m_blocknum);
+ finish_purge(contp, data);
+ }
+}
+
} // namespace
+// Answer the client once every block has been walked. Nothing is written
+// downstream before this, so one uncached block cannot leak a 404 to the
client.
+// A non-NONE status overrides the outcome of the walk.
+void
+finish_purge(TSCont const contp, Data *const data, TSHttpStatus const status)
+{
+ data->m_upstream.close();
+ data->m_blockstate = BlockState::Done;
+
+ TSHttpStatus const reply =
+ (TS_HTTP_STATUS_NONE != status) ? status : (0 < data->m_purge_hits ?
TS_HTTP_STATUS_OK : TS_HTTP_STATUS_NOT_FOUND);
Review Comment:
I noticed that we return 200 to client as long as we have at least one purge
hit since we assume that purge miss means block is not cached but we continue
the path to purge. But what if there was a 5xx error from a later purge block
and we stop purging. Even though we purged the beginning blocks, we could leave
the later blocks cached but tell client 200 (assuming the whole object is
gone).
I think we should distinguish the non-200 responses so 404 is purge miss and
that is fine, we can continue to purge down the object but if its 5xx or
non-404, the client should not get a 200 for incomplete PURGE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
