Am 17/06/2024 um 18:03 schrieb Friedrich Weber: > The API server proxies HTTP requests in two cases: > > - between cluster nodes (pveproxy->pveproxy) > - between daemons on one node for protected API endpoints > (pveproxy->pvedaemon) > > The API server uses AnyEvent::HTTP for proxying, with unfortunate > settings for connection reuse (details below). With these settings, > long-running synchronous API requests on the proxy destination's side > can cause unrelated proxied requests to fail with a misleading HTTP > 599 "Too many redirections" error response. In order to avoid these > errors, improve the connection reuse settings. > > In more detail: > > Per default, AnyEvent::HTTP reuses previously-opened connections for > requests with idempotent HTTP verbs, e.g. GET/PUT/DELETE [1]. However, > when trying to reuse a previously-opened connection, it can happen > that the destination unexpectedly closes the connection. In case of > idempotent requests, AnyEvent::HTTP's http_request will retry by > recursively calling itself. Since the API server disallows recursion > by passing `recurse => 0` to http_request initially, the recursive > call fails with "HTTP 599 Too many redirections". > > This can happen both for pveproxy->pveproxy and pveproxy->pvedaemon, > as connection reuse is enabled in both cases. Connection reuse being > enabled in the pveproxy->pvedaemon case was likely not intended: A > comment mentions that "keep alive for localhost is not worth it", but > only sets `keepalive => 0` and not `persistent => 0`. This setting > switches from HTTP/1.1 persistent connections to HTTP/1.0-style > keep-alive connections, but still allows connection reuse. > > The destination unexpectedly closing the connection can be due to > unfortunate timing, but it becomes much more likely in case of > long-running synchronous requests. An example sequence: > > 1) A pveproxy worker P1 handles a protected request R1 and proxies it > to a pvedaemon worker D1, opening a pveproxy worker->pvedaemon > worker connection C1. The pvedaemon worker D1 is relatively fast > (<1s) in handling R1. P1 saves connection C1 for later reuse. > 2) A different pveproxy worker P2 handles a protected request R2 and > proxies it to the same pvedaemon worker D1, opening a new pveproxy > worker->pvedaemon connection C2. Handling this request takes a long > time (>5s), for example because it queries a slow storage. While > the request is being handled, the pvedaemon worker D1 cannot do > anything else. > 3) Since pvedaemon worker D1 sets a timeout of 5s when accepting > connections and it did not see anything on connection C1 for >5s > (because it was busy handling R2), it closes the connection C1. > 3) pveproxy worker P1 handles a protected idempotent request R3. Since > the request is idempotent, it tries to reuse connection C1. But C1 > was just closed by D1, so P1 fails request R3 with HTTP 599 as > described above. > > In addition, AnyEvent::HTTP's default of reusing connections for all > idempotent HTTP verbs is problematic in our case, as not all PUT > requests of the PVE API are actually idempotent, e.g. /sendkey [2]. > > To fix the issues above, improve the connection reuse settings: > > - Actually disable connection reuse for pveproxy->pvedaemon requests, > by passing `persistent => 0`. > - For pveproxy->pveproxy requests, enable connection reuse for GET > requests only, as these should be actually idempotent. > - If connection reuse is enabled, allow one retry by passing `recurse > => 1`, to avoid the HTTP 599 errors. > > [1] https://metacpan.org/pod/AnyEvent::HTTP#persistent-=%3E-$boolean > [2] > https://pve.proxmox.com/pve-docs/api-viewer/index.html#/nodes/{node}/qemu/{vmid}/sendkey > > Suggested-by: Fabian Grünbichler <f.gruenbich...@proxmox.com> > Signed-off-by: Friedrich Weber <f.we...@proxmox.com> > --- > src/PVE/APIServer/AnyEvent.pm | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) >
Nice work and write up! Acked-by: Thomas Lamprecht <t.lampre...@proxmox.com> But yeah, seeing some benchmarking for before/after this patch would still be great, that's also the main reason for me not applying this now already. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
