[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] rpc: Allow up to 256K records to be returned from virConnectGetAllDomainStats.



On Fri, May 26, 2017 at 01:29:20PM +0100, Richard W.M. Jones wrote:
> In commit 89a706681cb3a4aa003d920db3163b809cfbc9ca, the returned
> array of stats is limited to REMOTE_DOMAIN_LIST_MAX entries (4096).
> 
> As well as being far too low -- this breaks if a single guest is added
> with 320 disks -- it also seems as if use of REMOTE_DOMAIN_LIST_MAX
> was a mistake, and it should be using
> REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX instead.
> 
> However REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX is also too low (also
> 4096), so this patch also increases the limit to something more
> sensible, allowing us to cope with lots of stats from lots of domains
> in future.  (This limit could be increased further quite easily since
> each stats record takes about 32 bytes, and the maximum message size
> is currently 32 MB, so we could increase the limit by another factor
> of 4 without touching the maximum message size).

262144 stats per guest, 16384 guests per host, 32 bytes per stat
gives 128 GBs of memory actually :-)

Regardless though, raising the max stats per guest is still fine
IMHO - it just means if you have lots of guests, all with lots
of stats you'll still need to batch them long before you hit
this new max stats limit.

> 
> I tested this using a guest with 320, 500 and 1000 disks with no issues.
> 
> Signed-off-by: Richard W.M. Jones <rjones redhat com>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1440683
> ---
>  src/remote/remote_protocol.x | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 25e62a181..142508713 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -233,7 +233,7 @@ const REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX = 256;
>  const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
>  
>  /* Upper limit on count of parameters returned via bulk stats API */
> -const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
> +const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 262144;
>  
>  /* Upper limit of message size for tunable event. */
>  const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 2048;
> @@ -3264,7 +3264,7 @@ struct remote_domain_event_callback_agent_lifecycle_msg {
>  };
>  
>  struct remote_connect_get_all_domain_stats_ret {
> -    remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>;
> +    remote_domain_stats_record retStats<REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX>;
>  };

There is one retStats entry per guest domain, so using DOMAIN_LIST_MAX
is right here.

The remote_domain_stats_record entry then contains N statistics
per guest. So we should only need the first hunk of this patch

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]