cmcfarlen commented on code in PR #13791:
URL: https://github.com/apache/trafficserver/pull/13791#discussion_r4224291145
##########
plugins/experimental/stale_response/ServerIntercept.cc:
##########
@@ -152,51 +150,54 @@ connShutdownDataDestory(SContData *cont_data)
}
/*-----------------------------------------------------------------------------------------------*/
+// Move the entire body into the output buffer before arming the write. The
+// write then completes as the consumer drains the buffer, without depending
+// on a VCONN_WRITE_READY callback to refill it.
static bool
writeOutData(SContData *cont_data)
{
- int64_t total_current_write = 0;
- uint32_t max_chunk_count = cont_data->pBody->getChunkCount();
- for (uint32_t chunk_index = cont_data->next_chunk_written; chunk_index <
max_chunk_count; chunk_index++) {
+ int64_t total_written = 0;
+ uint32_t const chunk_count = cont_data->pBody->getChunkCount();
+ for (uint32_t chunk_index = 0; chunk_index < chunk_count; ++chunk_index) {
const char *start;
int64_t avail;
if (!cont_data->pBody->getChunk(chunk_index, &start, &avail)) {
- SRDBG(TAG_BAD, "[%s] Error while getting chunk_index %d", __FUNCTION__,
chunk_index);
- TSError("[%s] Error while getting chunk_index %d", __FUNCTION__,
chunk_index);
- break;
+ SRDBG(TAG_BAD, "[%s] Error while getting chunk_index %u", __FUNCTION__,
chunk_index);
+ TSError("[%s] Error while getting chunk_index %u", __FUNCTION__,
chunk_index);
+ return false;
}
if (TSIOBufferWrite(cont_data->output.buffer, start, avail) != avail) {
- SRDBG(TAG_BAD, "[%s] Error while writing content avail=%d",
__FUNCTION__, (int)avail);
+ SRDBG(TAG_BAD, "[%s] Error while writing content avail=%" PRId64,
__FUNCTION__, avail);
+ TSError("[%s] Error while writing content avail=%" PRId64, __FUNCTION__,
avail);
+ return false;
}
cont_data->pBody->removeChunk(chunk_index);
Review Comment:
`removeChunk()` only does `chunk_list[chunk].start.clear()`, and
`vector::clear()` keeps the capacity, so nothing is released here until the
`BodyData` is deleted at shutdown. Before this change that cost an extra ~64
KiB per in-flight refresh. Now the IOBuffer holds a second full copy of every
body being replayed, outside `max_body_data_memory_usage` accounting. Freeing
the storage in `removeChunk()`
(`std::vector<char>().swap(chunk_list[chunk].start);`) would make the "peak
memory stays the same" claim hold, at roughly one body plus one chunk.
--
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]
