[libvirt] [PATCH] rpc: Allow up to 256K records to be returned from virConnectGetAllDomainStats.
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
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.
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 :|
participants (3)
-
Daniel P. Berrange -
Peter Krempa -
Richard W.M. Jones