You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "janiussyafiq (via GitHub)" <gi...@apache.org> on 2026/08/07 04:51:00 UTC

[PR] fix(ai-proxy-multi): keep the client request body intact across fallback retries [apisix]

janiussyafiq opened a new pull request, #13793:
URL: https://github.com/apache/apisix/pull/13793

   ### Description
   
   The retry loop in `before_proxy` re-reads the parsed request body from the per-request cache (`ctx._request_body_table`), but `build_request` rewrites that shared table in place with per-instance options. A fallback attempt therefore starts from the previous instance's rewritten body: `$request_llm_model` is clobbered with the first instance's model, and the first instance's options (e.g. `temperature`) leak into the retry request sent to the fallback instance.
   
   This PR deep-copies the parsed body at the start of every attempt so each instance builds its request from the client's original body, and drops the redundant `request_llm_model` re-assignment in `before_proxy` (`detect_request_type` already captures it from the client body in the access phase, for both `ai-proxy` and `ai-proxy-multi`).
   
   The copy is O(body) and cheaper than the JSON decode of the same body already performed in the access phase. Keeping the shared cache pristine also fixes log-phase `post_arg.*` variable reads and the semantic balancer's re-pick on retry, both of which previously saw the instance-rewritten body instead of the client's request.
   
   #### Which issue(s) this PR fixes:
   Fixes #13769
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(ai-proxy-multi): keep the client request body intact across fallback retries [apisix]

Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 merged PR #13793:
URL: https://github.com/apache/apisix/pull/13793


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(ai-proxy-multi): keep the client request body intact across fallback retries [apisix]

Posted by "janiussyafiq (via GitHub)" <gi...@apache.org>.
janiussyafiq commented on PR #13793:
URL: https://github.com/apache/apisix/pull/13793#issuecomment-5261211621

   Tested the deepcopy overhead on both axes: string length and message count.
   
   **Long strings are free to copy.**
   LuaJIT strings are immutable, so `deepcopy` copies them by reference and never touches the bytes. The copy cost grows only with the number of messages in the body. For scale, here it is next to the `cjson.decode` that ai-proxy already runs on the same body every request:
   
   | body | deepcopy (new cost) | cjson.decode (already paid today) |
   |---|---|---|
   | 1 msg, 1KB string | 0.25 µs | 1.2 µs |
   | 1 msg, 1MB string | 0.21 µs | 668 µs |
   | 1 msg, 10MB string | 0.22 µs | 6835 µs |
   | 100 msgs x 1KB | 10 µs | 84 µs |
   | 1000 msgs x 1KB | 110 µs | 823 µs |
   
   Even in the worst case the copy is a fraction of the JSON parse we already do for the same body, so it cannot move total request cost by more than that fraction.
   
   **E2E check**: wrk at full saturation against a local 1-worker APISIX with an ai-proxy-multi route to a mock LLM upstream.
   Each number is the mean of 10 x 15s runs, interleaved and spread across two instances to cancel machine-side bias.
   
   | body | req/s without the change | req/s with the change | delta |
   |---|---|---|---|
   | 1 msg x 1KB | 14802 | 14682 | -0.8% (within noise) |
   | 1 msg x 128KB | 2807 | 2790 | -0.6% (within noise) |
   | 1 msg x 1MB | 418 | 409 | -2.3% (within noise) |
   | 200 msgs x 1KB | 1789 | 1624 | -9.2% |
   
   Single-message bodies show no measurable difference regardless of string size.
   The only visible case is the 200-message body with the worker CPU fully saturated: ~+53 µs per request, mostly GC churn from the 200 copied message tables.
   At normal load, +53 µs on an LLM request that takes hundreds of ms is negligible.
   
   If the many-messages-at-saturation case ever matters, a copy-on-write follow-up could reclaim it, but the numbers do not seem to warrant the complexity.
   


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org