perf(ai-proxy): optimize SSE decoder - remove PCRE, add decode_buf, fix comment lines#13391
Merged
Conversation
…ix comment lines - Replace ngx_re.split (PCRE) in sse.decode() with a pure Lua forward scan. Avoids regex compilation and PCRE overhead on every streaming chunk. - Add sse.decode_buf(buf) that combines split_buf + decode into a single O(n) forward pass, halving the number of scans per chunk in hot paths. - Fix SSE comment lines (lines starting with ':') being incorrectly treated as fields. Per the SSE spec, comment lines must be silently ignored; the previous code set has_field=true for them, producing spurious empty events. - Add test: SSE comment lines are correctly discarded by decode_buf. Co-authored-by: Copilot <[email protected]>
…path - base.lua: replace sse_buf string concatenation with sse_parts table to avoid O(n) string allocation on every chunk; use table.insert + concat only when needed. Also adds ngx.sleep(0) yield at end of each iteration so health checks and concurrent requests are not starved under bursty SSE upstreams. - base.lua: add streaming_flush_interval_ms support - when > 0, a background ngx.thread flushes the output buffer every N milliseconds instead of per-chunk. Useful when the upstream bursts multiple tokens at once and you want to bound client latency without synchronous flush overhead on every chunk. Thread spawn failure falls back to sync flush. flush_thread is killed on all exit paths (EOF, error, 502, limits). - plugin.lua: add no_flush parameter to lua_response_filter(). When true, ngx.flush is skipped so the background interval thread can own flushing. Backward compatible - existing callers passing (ctx, headers, body, wait) are unaffected. - schema.lua: add streaming_flush_interval_ms field (integer, min=0, default=0) to both ai_proxy_schema and ai_proxy_multi_schema. - t/plugin/ai-proxy-flush.t: E2E tests for flush interval behavior, using error_log / no_error_log to assert whether the background flush thread fires. Also covers schema validation of negative values.
…blank-line chunks decode() previously broke callers that pass full response bodies or per-chunk bodies without a terminal blank line. The old ngx.re-based decoder treated the remaining content as a complete event; the new implementation discarded it. Fix: when no boundary is found for the remaining content, parse it as a final event via parse_raw_event instead of silently dropping it.
…eaming_flush_interval_ms docs Co-authored-by: Copilot <[email protected]>
…on.t Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Replace debug log grep approach with a hook-based counter: - patch plugin_mod.lua_response_filter in extra_init_worker_by_lua to count flush calls per request in ngx.ctx._flush_count - log the count via serverless-post-function in the log phase - TEST 2 asserts 'flush count: 5' (one flush per SSE chunk) Co-authored-by: Copilot <[email protected]>
… mode) TEST 3 route now includes serverless-post-function log hook. TEST 4 adds 'flush count: 0' assertion to confirm sync flush is skipped when background flush_thread is active. Co-authored-by: Copilot <[email protected]>
…ate flush count Instead of wrapping lua_response_filter or using rawset(ngx,'flush') in init_by_lua (which doesn't survive worker fork), use debug.setupvalue in init_worker to replace the ngx_flush upvalue captured by lua_response_filter. This directly counts actual ngx.flush calls: - TEST 2 (interval_ms=0): 5 per-chunk flushes - TEST 4 (interval_ms=50): flush_thread handles flushing, no per-chunk flushes Co-authored-by: Copilot <[email protected]>
- Deduplicate print+flush code in lua_response_filter by inverting the early-return guard and merging both paths into one - Add debug log in lua_response_filter flush path to prove per-chunk sync flush behavior in tests - Simplify ai-proxy-flush tests: replace ngx.flush hook mechanism with log-based assertions (grep_error_log for exact counts) Co-authored-by: Copilot <[email protected]>
base.lua parse_streaming_response calls framing.decode_buf() on every chunk. sse.lua already had this method but aws-eventstream.lua did not, causing a nil-call 500 error on all Bedrock streaming requests. Co-authored-by: Copilot <[email protected]>
…m decode_buf docstring - Restore the original upstream WORKAROUND comment for ngx.sleep(0) - Update aws-eventstream.lua decode_buf docstring to match EE Co-authored-by: Copilot <[email protected]>
- Check ngx.flush(false) return in flush_thread; propagate error via shared flush_err upvalue so the main loop can abort cleanly - Check flush_err at the top of the streaming loop and call abort_on_disconnect to stop upstream reads on client disconnect - Skip final ngx.flush(true) when flush_err is set to suppress noisy 'final flush failed' log on client disconnect - Change streaming_flush_interval_ms default from 0 to 10 ms - Add TEST 3/4 to ai-proxy-client-disconnect.t covering async flush disconnect scenario - Minor ai-proxy-flush.t cleanups (explicit ms=0 in TEST 1, TEST 6 title) Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| -- each split so the table never grows beyond two elements. | ||
| -- Initialized with "" so the fast-path (sse_parts[1] == "") activates | ||
| -- immediately on the first chunk, avoiding an unnecessary table.concat. | ||
| local sse_parts = {""} |
nic-6443
previously approved these changes
May 21, 2026
Member
|
Changing the default to 10ms looks fine, but a few places still need to be updated so the behavior is documented consistently: The PR description still says 0 (default); it should say the default is 10ms, and 0 disables background interval flushing. |
… default - Update base.lua comment: clarify default is 10ms, not 0; describe both modes (0 = per-chunk ngx.flush(true), >0 = background ngx.flush(false)) - Update EN/ZH docs: explicitly document 0 behavior vs >0 behavior - Add TEST 7/8 to ai-proxy-flush.t: omit streaming_flush_interval_ms and verify the 10ms default activates the background flush thread Co-authored-by: Copilot <[email protected]>
Replace route-setup + streaming request pair with a single check_schema test that verifies the JSON Schema default (10) is injected into the conf table when streaming_flush_interval_ms is omitted. Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
nic-6443
approved these changes
May 21, 2026
membphis
approved these changes
May 22, 2026
shreemaan-abhishek
approved these changes
May 22, 2026
wistefan
pushed a commit
to wistefan/apisix
that referenced
this pull request
Jun 16, 2026
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.
What
Optimize the AI proxy SSE streaming hot path: fewer allocations, less CPU per chunk, and a new interval-flush mode for bursty LLM streams.
Changes
ngx_re.split(PCRE) withstring.findin SSE decoderdecode_buf(buf)— combinessplit_buf+decodein one forward pass:) being emitted as empty events: keep-alivepings no longer produce spurious eventsbuf = buf .. chunkwith a 2-element table +table.concatstreaming_flush_interval_msconfig option (default10ms)ngx.flushsyscalls by ~95% on bursty streams (100 chunks → 2–4 flushes at 10 ms)no_flushparam tolua_response_filterstreaming_flush_interval_msexample{ "provider": "openai", "streaming_flush_interval_ms": 10 }10(default)ngx.flush(false)every 10 ms — batches output on bursty streams> 0(e.g.50)lua_response_filterskipsngx.flushper chunk0ngx.flush(true)The background thread only flushes when there is new data (via a
needs_flushflag), so empty intervals cost nothing.