Copilot commented on code in PR #13791:
URL: https://github.com/apache/trafficserver/pull/13791#discussion_r4224296153
##########
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 calls `vector::clear()`, so each chunk retains its
allocated capacity until the intercept is destroyed. Preloading the output
buffer therefore keeps the entire accounted `BodyData` allocation alive while
allocating another full response in the IOBuffer; concurrent refreshes can use
roughly twice `max_body_data_memory_usage` (1 GiB by default). The chunks need
to relinquish their backing storage as they are copied (for example, by
changing `BodyData::removeChunk()` to replace/swap out the vector), or the
output copy must be included in the memory accounting.
--
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]
