[libvirt PATCH v2] src: warn if client hits the max requests limit

Daniel P. Berrangé posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20221007125627.1473332-1-berrange@redhat.com
src/rpc/virnetserverclient.c | 8 ++++++++
1 file changed, 8 insertions(+)
[libvirt PATCH v2] src: warn if client hits the max requests limit
Posted by Daniel P. Berrangé 1 year, 6 months ago
Since they are simply normal RPC messages, the keep alive packets are
subject to the "max_client_requests" limit just like any API calls.

Thus, if a client hits the 'max_client_requests' limit and all the
pending API calls take a long time to complete, it may result in
keep-alives firing and dropping the client connection.

This has been seen by a number of users with the default value of
max_client_requests=5, by issuing 5 concurrent live migration
operations.

By printing a warning message when this happens, admins will be alerted
to the fact that their active clients are exceeding the default client
requests limit.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

In v2:

  - now only warn once per client to avoid log spam

 src/rpc/virnetserverclient.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 30f6af7be5..c9a4eb521e 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -93,6 +93,9 @@ struct _virNetServerClient
      * throttling calculations */
     size_t nrequests;
     size_t nrequests_max;
+    /* True if we've warned about nrequests hittin
+     * the server limit already */
+    bool nrequests_warning;
     /* Zero or one messages being received. Zero if
      * nrequests >= max_clients and throttling */
     virNetMessage *rx;
@@ -1261,6 +1264,11 @@ static virNetMessage *virNetServerClientDispatchRead(virNetServerClient *client)
                 client->rx->buffer = g_new0(char, client->rx->bufferLength);
                 client->nrequests++;
             }
+        } else if (!client->nrequests_warning) {
+            client->nrequests_warning = true;
+            VIR_WARN("Client hit max requests limit %zd. This may result "
+                     "in keep-alive timeouts. Consider tuning the "
+                     "max_client_requests server parameter", client->nrequests);
         }
         virNetServerClientUpdateEvent(client);
 
-- 
2.37.3

Re: [libvirt PATCH v2] src: warn if client hits the max requests limit
Posted by Peter Krempa 1 year, 6 months ago
On Fri, Oct 07, 2022 at 13:56:27 +0100, Daniel P. Berrangé wrote:
> Since they are simply normal RPC messages, the keep alive packets are
> subject to the "max_client_requests" limit just like any API calls.
> 
> Thus, if a client hits the 'max_client_requests' limit and all the
> pending API calls take a long time to complete, it may result in
> keep-alives firing and dropping the client connection.
> 
> This has been seen by a number of users with the default value of
> max_client_requests=5, by issuing 5 concurrent live migration
> operations.
> 
> By printing a warning message when this happens, admins will be alerted
> to the fact that their active clients are exceeding the default client
> requests limit.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> In v2:
> 
>   - now only warn once per client to avoid log spam

Reviewed-by: Peter Krempa <pkrempa@redhat.com>