rpc: Allow up to 256K records to be returned from virConnectGetAllDomainStats.

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

Posted by Richard W.M. Jones 6 years, 11 months ago

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).

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>;
 };
 
 struct remote_domain_fsinfo {
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Peter Krempa 6 years, 11 months ago

On Fri, May 26, 2017 at 13:29:20 +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).
> 
> 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;

ACK to this.

>  
>  /* 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>;

But this should stay as it is now, this is basically the number of VMs
you can request stats for. You want to increase only the number of stats
per VM which should be okay.

In other places returning VM lists we use the 16k limit named
REMOTE_DOMAIN_LIST_MAX, so it should stay that way.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Daniel P. Berrange 6 years, 11 months ago

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list