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

[Daniel P. Berrangé]

ping

On Wed, Aug 24, 2022 at 04:21:02PM +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 at redhat.com>
> ---
> 
> I'm a little wary of this change. If we use anything less than VIR_WARN
> it is not going to be useful, as we need it visible by default. At the
> same time though I'm concerned that this might expose very many
> deployments using an unreasonably low max_client_requests value for
> their workload. For example OpenStack deployment tools have often left
> this on the default setting and have been known to exceed it with live
> migration running concurrently.
> 
> One possible optimization would be to only issue this warning once per
> connected client, so we don't spam repeatedly ?
> 
>  src/rpc/virnetserverclient.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index d57ca07167..0d82726194 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -1259,6 +1259,10 @@ static virNetMessage *virNetServerClientDispatchRead(virNetServerClient *client)
>                  client->rx->buffer = g_new0(char, client->rx->bufferLength);
>                  client->nrequests++;
>              }
> +        } else {
> +            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.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|