From 68b177978b7359e48bf9c6bb493a6b12ccd604d6 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 5 Jun 2024 17:56:48 +0530 Subject: [PATCH 1/2] api: listVM API improvement followup, change returning of stats detail - Changes behaviour of details param handling via global setting: - listVirtualMachines API: when the details param is not provided, it returns `all` details excluding/including `stats` which is controllable by a new global setting `list.vm.default.details.stats` - listVirtualMachinesMetrics API: when the details param is not provided, it uses `all` details including `stats` - Users who are affected by the stats related change, can have backward compatibility at the higher-cost of listVirtualMachines API response time by setting `list.vm.default.details.stats` to true - Remove ConfigKey vm.stats.increment.metrics.in.memory which was renamed to `vm.stats.increment.metrics` in #5984 and also remove unused/unnecessary global settings via upgrade path - Changes default value of VM stats accumulation setting `vm.stats.increment.metrics` to false until a better solution emerges. Since #5984, this is true and during the execution of listVM APIs the stats are clubbed/calculated which can immensely slow down list VM API calls. - Fix UI that uses listVirtualMachinesMetrics to not call `stats` detail when in list view without metrics selected. Signed-off-by: Rohit Yadav --- .../apache/cloudstack/api/command/user/vm/ListVMsCmd.java | 4 ++-- .../main/java/org/apache/cloudstack/query/QueryService.java | 5 ++--- .../resources/META-INF/db/schema-41900to41910-cleanup.sql | 4 ++++ server/src/main/java/com/cloud/server/StatsCollector.java | 4 ---- ui/src/config/section/compute.js | 4 ++-- ui/src/views/AutogenView.vue | 1 + 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java index 4abccfd1da8d..8854ff451a11 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java @@ -99,8 +99,8 @@ public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements collectionType = CommandType.STRING, description = "comma separated list of vm details requested, " + "value can be a list of [all, group, nics, stats, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]." - + " If no parameter is passed in, the details will be defaulted to all. When return.vm.stats.on.vm.list is true, the default" + - "details change to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp], thus the stats will not be returned. ") + + " If no parameter is passed in, the details will be defaulted to all details except stats when list.vm.default.details.stats is false (default)." + + " When list.vm.default.details.stats is true, all details including stats are returned") private List viewDetails; @Parameter(name = ApiConstants.TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list vms by template") diff --git a/api/src/main/java/org/apache/cloudstack/query/QueryService.java b/api/src/main/java/org/apache/cloudstack/query/QueryService.java index dd5aaf4e6d7c..dad71044c870 100644 --- a/api/src/main/java/org/apache/cloudstack/query/QueryService.java +++ b/api/src/main/java/org/apache/cloudstack/query/QueryService.java @@ -125,9 +125,8 @@ public interface QueryService { static final ConfigKey SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true", "If false, templates of this domain will not show up in the list templates of other domains.", true, ConfigKey.Scope.Domain); - ConfigKey ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "return.vm.stats.on.vm.list", "true", - "If false, changes the listVirtualMachines default details to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp], so that the VMs' stats" + - " are not returned by default when listing VMs; only when the 'stats' or 'all' detail is informed.", true, ConfigKey.Scope.Global); + ConfigKey ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "false", + "Determines whether VM stats should be returned when details are not specified in listVirtualMachines API request. When false, details default to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]. When true, all details are returned including 'stats'.", true, ConfigKey.Scope.Global); ListResponse searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException; diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql b/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql index b580d42686f0..e35ea89fb88a 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql @@ -18,3 +18,7 @@ --; -- Schema upgrade cleanup from 4.19.0.0 to 4.19.1.0 --; + +-- List VMs response optimisation +UPDATE cloud.configuration set value='false' where name='vm.stats.increment.metrics'; +DELETE from cloud.configuration where name='vm.stats.increment.metrics.in.memory'; diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 2467416155a1..95518cab12fe 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -281,9 +281,6 @@ public String toString() { protected static ConfigKey vmStatsIncrementMetrics = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics", "true", "When set to 'true', VM metrics(NetworkReadKBs, NetworkWriteKBs, DiskWriteKBs, DiskReadKBs, DiskReadIOs and DiskWriteIOs) that are collected from the hypervisor are summed before being returned." + "On the other hand, when set to 'false', the VM metrics API will just display the latest metrics collected.", true); - private static final ConfigKey VM_STATS_INCREMENT_METRICS_IN_MEMORY = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics.in.memory", "true", - "When set to 'true', VM metrics(NetworkReadKBs, NetworkWriteKBs, DiskWriteKBs, DiskReadKBs, DiskReadIOs and DiskWriteIOs) that are collected from the hypervisor are summed and stored in memory. " - + "On the other hand, when set to 'false', the VM metrics API will just display the latest metrics collected.", true); protected static ConfigKey vmStatsMaxRetentionTime = new ConfigKey<>("Advanced", Integer.class, "vm.stats.max.retention.time", "720", "The maximum time (in minutes) for keeping VM stats records in the database. The VM stats cleanup process will be disabled if this is set to 0 or less than 0.", true); @@ -2131,7 +2128,6 @@ public String getConfigComponentName() { public ConfigKey[] getConfigKeys() { return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime, - VM_STATS_INCREMENT_METRICS_IN_MEMORY, MANAGEMENT_SERVER_STATUS_COLLECTION_INTERVAL, DATABASE_SERVER_STATUS_COLLECTION_INTERVAL, DATABASE_SERVER_LOAD_HISTORY_RETENTION_NUMBER}; diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index 4893cfc10533..57177d0da932 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -32,9 +32,9 @@ export default { getApiToCall: () => store.getters.metrics ? 'listVirtualMachinesMetrics' : 'listVirtualMachines', resourceType: 'UserVm', params: () => { - var params = { details: 'servoff,tmpl,iso,nics,backoff' } + var params = { details: 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp' } if (store.getters.metrics) { - params = { details: 'servoff,tmpl,iso,nics,backoff,stats' } + params = { details: 'all,stats' } } params.isvnf = false return params diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue index f9c4edd36b09..480b2f86f03d 100644 --- a/ui/src/views/AutogenView.vue +++ b/ui/src/views/AutogenView.vue @@ -904,6 +904,7 @@ export default { if (['listVirtualMachinesMetrics'].includes(this.apiName) && this.dataView) { delete params.details delete params.isvnf + params.details = 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp' } this.loading = true From 1efbefce7b81f81be9ddd2a0f97ec8399d9b8615 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 13 Jun 2024 11:36:30 +0530 Subject: [PATCH 2/2] keep old behaviour, have list.vm.default.details.stats to true (default) Signed-off-by: Rohit Yadav --- .../org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java | 4 ++-- .../main/java/org/apache/cloudstack/query/QueryService.java | 4 ++-- .../resources/META-INF/db/schema-41900to41910-cleanup.sql | 2 +- server/src/main/java/com/cloud/server/StatsCollector.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java index 8854ff451a11..37b702e166af 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java @@ -99,8 +99,8 @@ public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements collectionType = CommandType.STRING, description = "comma separated list of vm details requested, " + "value can be a list of [all, group, nics, stats, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]." - + " If no parameter is passed in, the details will be defaulted to all details except stats when list.vm.default.details.stats is false (default)." - + " When list.vm.default.details.stats is true, all details including stats are returned") + + " When no parameters are passed, all the details are returned if list.vm.default.details.stats is true (default)," + + " otherwise when list.vm.default.details.stats is false the API response will exclude the stats details.") private List viewDetails; @Parameter(name = ApiConstants.TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list vms by template") diff --git a/api/src/main/java/org/apache/cloudstack/query/QueryService.java b/api/src/main/java/org/apache/cloudstack/query/QueryService.java index dad71044c870..4c53314aef59 100644 --- a/api/src/main/java/org/apache/cloudstack/query/QueryService.java +++ b/api/src/main/java/org/apache/cloudstack/query/QueryService.java @@ -125,8 +125,8 @@ public interface QueryService { static final ConfigKey SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true", "If false, templates of this domain will not show up in the list templates of other domains.", true, ConfigKey.Scope.Domain); - ConfigKey ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "false", - "Determines whether VM stats should be returned when details are not specified in listVirtualMachines API request. When false, details default to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]. When true, all details are returned including 'stats'.", true, ConfigKey.Scope.Global); + ConfigKey ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "true", + "Determines whether VM stats should be returned when details are not explicitly specified in listVirtualMachines API request. When false, details default to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]. When true, all details are returned including 'stats'.", true, ConfigKey.Scope.Global); ListResponse searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException; diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql b/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql index e35ea89fb88a..2d57db2b778e 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql @@ -19,6 +19,6 @@ -- Schema upgrade cleanup from 4.19.0.0 to 4.19.1.0 --; --- List VMs response optimisation +-- List VMs response optimisation, don't sum during API handling UPDATE cloud.configuration set value='false' where name='vm.stats.increment.metrics'; DELETE from cloud.configuration where name='vm.stats.increment.metrics.in.memory'; diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 95518cab12fe..7f4cc2f757e4 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -278,7 +278,7 @@ public String toString() { private static final ConfigKey statsOutputUri = new ConfigKey<>("Advanced", String.class, "stats.output.uri", "", "URI to send StatsCollector statistics to. The collector is defined on the URI scheme. Example: graphite://graphite-hostaddress:port or influxdb://influxdb-hostaddress/dbname. Note that the port is optional, if not added the default port for the respective collector (graphite or influxdb) will be used. Additionally, the database name '/dbname' is also optional; default db name is 'cloudstack'. You must create and configure the database if using influxdb.", true); - protected static ConfigKey vmStatsIncrementMetrics = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics", "true", + protected static ConfigKey vmStatsIncrementMetrics = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics", "false", "When set to 'true', VM metrics(NetworkReadKBs, NetworkWriteKBs, DiskWriteKBs, DiskReadKBs, DiskReadIOs and DiskWriteIOs) that are collected from the hypervisor are summed before being returned." + "On the other hand, when set to 'false', the VM metrics API will just display the latest metrics collected.", true); protected static ConfigKey vmStatsMaxRetentionTime = new ConfigKey<>("Advanced", Integer.class, "vm.stats.max.retention.time", "720",