Skip to content

Commit 19576f2

Browse files
yadvrdhslove
authored andcommitted
api: listVM API improvement followup, change returning of stats detail (apache#9177)
- Changes behaviour of details param handling via global setting: - listVirtualMachines API: when the details param is not provided, it returns whether stats are returned controlled 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 slow performance of the listVirtualMachines API response time can set `list.vm.default.details.stats` to `false` - Remove ConfigKey vm.stats.increment.metrics.in.memory which was renamed to `vm.stats.increment.metrics` in apache#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 apache#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. Any costly operations such as summing of stats shouldn't be done during the course of a synchronous API, such as the list VM API. - Fix UI that uses listVirtualMachinesMetrics to not call `stats` detail when in list view without metrics selected. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
1 parent 7a4f265 commit 19576f2

6 files changed

Lines changed: 13 additions & 13 deletions

File tree

api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements
9797
collectionType = CommandType.STRING,
9898
description = "comma separated list of vm details requested, "
9999
+ "value can be a list of [all, group, nics, stats, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]."
100-
+ " If no parameter is passed in, the details will be defaulted to all. When return.vm.stats.on.vm.list is true, the default" +
101-
"details change to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp], thus the stats will not be returned. ")
100+
+ " When no parameters are passed, all the details are returned if list.vm.default.details.stats is true (default),"
101+
+ " otherwise when list.vm.default.details.stats is false the API response will exclude the stats details.")
102102
private List<String> viewDetails;
103103

104104
@Parameter(name = ApiConstants.TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list vms by template")

api/src/main/java/org/apache/cloudstack/query/QueryService.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,8 @@ public interface QueryService {
125125
static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
126126
"If false, templates of this domain will not show up in the list templates of other domains.", true, ConfigKey.Scope.Domain);
127127

128-
ConfigKey<Boolean> ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "return.vm.stats.on.vm.list", "true",
129-
"If false, changes the listVirtualMachines default details to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp], so that the VMs' stats" +
130-
" are not returned by default when listing VMs; only when the 'stats' or 'all' detail is informed.", true, ConfigKey.Scope.Global);
128+
ConfigKey<Boolean> ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "true",
129+
"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);
131130

132131
ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;
133132

engine/schema/src/main/resources/META-INF/db/schema-41900to41910-cleanup.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,7 @@
1818
--;
1919
-- Schema upgrade cleanup from 4.19.0.0 to 4.19.1.0
2020
--;
21+
22+
-- List VMs response optimisation, don't sum during API handling
23+
UPDATE cloud.configuration set value='false' where name='vm.stats.increment.metrics';
24+
DELETE from cloud.configuration where name='vm.stats.increment.metrics.in.memory';

server/src/main/java/com/cloud/server/StatsCollector.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,9 @@ public String toString() {
299299
private static final ConfigKey<String> statsOutputUri = new ConfigKey<>("Advanced", String.class, "stats.output.uri", "",
300300
"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.",
301301
true);
302-
protected static ConfigKey<Boolean> vmStatsIncrementMetrics = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics", "true",
302+
protected static ConfigKey<Boolean> vmStatsIncrementMetrics = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics", "false",
303303
"When set to 'true', VM metrics(NetworkReadKBs, NetworkWriteKBs, DiskWriteKBs, DiskReadKBs, DiskReadIOs and DiskWriteIOs) that are collected from the hypervisor are summed before being returned."
304304
+ "On the other hand, when set to 'false', the VM metrics API will just display the latest metrics collected.", true);
305-
private static final ConfigKey<Boolean> VM_STATS_INCREMENT_METRICS_IN_MEMORY = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.increment.metrics.in.memory", "true",
306-
"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. "
307-
+ "On the other hand, when set to 'false', the VM metrics API will just display the latest metrics collected.", true);
308305
protected static ConfigKey<Integer> vmStatsMaxRetentionTime = new ConfigKey<>("Advanced", Integer.class, "vm.stats.max.retention.time", "60",
309306
"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);
310307

@@ -2332,8 +2329,7 @@ public String getConfigComponentName() {
23322329
@Override
23332330
public ConfigKey<?>[] getConfigKeys() {
23342331
return new ConfigKey<?>[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri,
2335-
vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime,
2336-
VM_STATS_INCREMENT_METRICS_IN_MEMORY,
2332+
vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime,
23372333
MANAGEMENT_SERVER_STATUS_COLLECTION_INTERVAL,
23382334
DATABASE_SERVER_STATUS_COLLECTION_INTERVAL,
23392335
DATABASE_SERVER_LOAD_HISTORY_RETENTION_NUMBER};

ui/src/config/section/compute.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ export default {
3232
getApiToCall: () => store.getters.metrics ? 'listVirtualMachinesMetrics' : 'listVirtualMachines',
3333
resourceType: 'UserVm',
3434
params: () => {
35-
var params = { details: 'servoff,tmpl,iso,nics,backoff' }
35+
var params = { details: 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp' }
3636
if (store.getters.metrics) {
37-
params = { details: 'servoff,tmpl,iso,nics,backoff,stats' }
37+
params = { details: 'all,stats' }
3838
}
3939
params.isvnf = false
4040
return params

ui/src/views/AutogenView.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,6 +1200,7 @@ export default {
12001200
if (['listVirtualMachinesMetrics'].includes(this.apiName) && this.dataView) {
12011201
delete params.details
12021202
delete params.isvnf
1203+
params.details = 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp'
12031204
}
12041205
12051206
this.loading = true

0 commit comments

Comments
 (0)