bneradt commented on code in PR #13791:
URL: https://github.com/apache/trafficserver/pull/13791#discussion_r4287423406
##########
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:
Good catch, thanks. `vector::clear()` kept the capacity, so the buffered
copy doubled the memory held for each refresh. `removeChunk()` now swaps in an
empty vector to release the storage, so peak memory is about one body plus one
chunk. I updated the commit message to match.
##########
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:
Agreed. Fixed in the same push: `removeChunk()` now releases each chunk's
storage as it is copied, so the IOBuffer copy no longer sits on top of the full
`BodyData` allocation.
--
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]
