fix(core): large response body logging causes memory growth#13581
Draft
AlinsRan wants to merge 1 commit into
Draft
fix(core): large response body logging causes memory growth#13581AlinsRan wants to merge 1 commit into
AlinsRan wants to merge 1 commit into
Conversation
hold_body_chunk accumulated every response body chunk into a Lua table
and, once the size limit was reached or on eof, concatenated the whole
table into a single string with table.concat. For large bodies (the
issue reports ~128K), this keeps both the list of chunks and the final
concatenated string alive at the same time, and every concat allocates
a fresh string, causing excessive memory allocation/retention.
Rewrite the accumulation to use a LuaJIT string.buffer
(require("string.buffer")): chunks are appended with buffer:put and the
final body is produced with buffer:get, which avoids holding the chunk
table plus an intermediate concatenated copy. For the truncation path
buffer:get(max_resp_body_bytes) returns exactly the first
max_resp_body_bytes bytes; the buffer is then dropped once `done` is set.
All existing semantics are preserved: the hold_the_copy flag,
max_resp_body_bytes truncation, the done flag (later chunks/eof return
nil after truncation), the single-chunk eof path and the per-buffer-key
storage.
Add t/core/response-hold-body.t covering multi-chunk accumulation, byte
truncation across chunks, the done flag, the single-chunk eof case and
small-body passthrough.
Fixes apache#11244
029861f to
b4cb6cb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #11244
Root cause
core.response.hold_body_chunkaccumulated every response-body chunk into a Lua table ({chunk, n, bytes, ...}) and, when the size limit was reached or oneof, concatenated the whole table into one string withtable.concat. For large bodies (the issue reports ~128K), this keeps both the list of individual chunks and the final concatenated string alive simultaneously, and eachtable.concatallocates a brand new string. The result is excessive allocation/retention that shows up as memory growth when loggers hold large response bodies.Fix
Rewrite the accumulation to use a LuaJIT
string.buffer(require("string.buffer")), reviving the approach from the abandoned #12035 and adapting it to the currenthold_body_chunkshape:buffer:put(chunk)instead of being stored in a growing Lua table;buffer:get()(full body) orbuffer:get(max_resp_body_bytes)(truncated), which returns exactly the firstmax_resp_body_bytesbytes and avoids materializing an intermediate concatenated copy before truncation;bytescounter and thedoneflag so the buffer object can carry per-buffer_keystate.All existing semantics are preserved:
hold_the_copyflag (still flushesarg[1]on non-final chunks when false);max_resp_body_bytestruncation (returns exactlymax_resp_body_bytes);doneflag (later chunks andeofreturnnilafter a truncated body was already returned);body_buffer_key/ctx._plugin_namestorage.Test
Adds
t/core/response-hold-body.tcovering:max_resp_body_bytesacross multiple chunks;doneflag — later chunks and eof returnnilafter truncation;max_resp_body_bytes;t/core/response.tTEST 8 mock pattern).Verification
luacheck apisix/core/response.luapasses (0 warnings / 0 errors)..tparse cleanly.ngx.arg[1]/[2]chunk/eof protocol; all six scenarios above pass and return byte-identical bodies (e.g. truncation returns exactly 131072 bytes).string.bufferpath avoids the simultaneous chunk-table + concatenated-string retention that caused the growth.Checklist