You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "vishesh92 (via GitHub)" <gi...@apache.org> on 2024/03/04 11:04:55 UTC
[PR] Add indexes for vm_stats [cloudstack]
vishesh92 opened a new pull request, #8737:
URL: https://github.com/apache/cloudstack/pull/8737
### Description
This PR adds the upgrade path for 4.19 to 4.19.1 and creates two missing indexes on vm_stats table:
1. `vm_stats` -> `vm_id`, `timestamp` - To speed up fetching of stats for a VM.
2. `vm_stats` -> `timestamp` - To speed up clean up of old vm_stats.
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ******************************************************************************* -->
<!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ******************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
#### How did you try to break this feature and the system with this change?
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#discussion_r1526291432
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41900to41910.java:
##########
@@ -61,6 +66,25 @@ public InputStream[] getCleanupScripts() {
throw new CloudRuntimeException("Unable to find " + scriptFile);
}
- return new InputStream[] {script};
+ return new InputStream[]{script};
+ }
+
+ private void addIndexes(Connection conn) {
+ DbUpgradeUtils.addIndexIfNeeded(conn, "vm_stats", "vm_id", "timestamp");
Review Comment:
```suggestion
DbUpgradeUtils.addIndexIfNeeded(conn, "vm_stats", "vm_id");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976334509
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976332445
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1979497377
> @vishesh92 could you share a description of the tests that you've done and their results?
Sure. I created a table with 661683 entries for timestamp ranging from `1970-11-29` till `2024-03-07 00:00:00`.
```sql
EXPLAIN ANALYZE SELECT * FROM vm_stats WHERE timestamp < '2000-01-01' ;
```
Query plan with index
```sql
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| EXPLAIN |
|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| -> Filter: (vm_stats.`timestamp` < TIMESTAMP'2000-01-01 00:00:00') (cost=66558 rows=330170) (actual time=0.0104..47.4 rows=282075 loops=1) |
| -> Covering index range scan on vm_stats using temp_idx over (timestamp < '2000-01-01 00:00:00') (cost=66558 rows=330170) (actual time=0.00948..35.4 rows=282075 loops=1) |
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
```
Query plan without index
```sql
+---------------------------------------------------------------------------------------------------------------------------------------------+
| EXPLAIN |
|---------------------------------------------------------------------------------------------------------------------------------------------|
| -> Filter: (vm_stats.`timestamp` < TIMESTAMP'2000-01-01 00:00:00') (cost=66500 rows=220091) (actual time=0.0198..87.5 rows=282075 loops=1) |
| -> Table scan on vm_stats (cost=66500 rows=660340) (actual time=0.0186..64 rows=661683 loops=1) |
+---------------------------------------------------------------------------------------------------------------------------------------------+
```
As you can see the index reduced the query time by ~40%.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1995252882
> > I only noticed this after users complained that listing VMs when creating a port forwarding was taking >30 seconds to load
>
> @kohrar, the way to address an issue like this should use a more rational manner. For instance, if we ask ourselves why metrics storage are affecting the VM listing on the port forwarding view, we might see/look for a different solution. The port forwarding view is calling `listVirtualMachines`, which by default (honestly, I do not know why it does that when we have `listVirtualMachinesMetrics` and `listVirtualMachinesUsageHistory`) makes a join with the statistics table. The problem is that the API `listVirtualMachines` is not working in a optimized fashion (returning VM statistics by default), and the UI is not passing the parameter `details` to not list the statistics. Therefore, the problem in this view is not the lack of indexes; but the API working/being called in a non-optimized way. I already talked with @JoaoJandre to work on a patch for this situation.
I've created #8782 to address the `listVIrtualMachines` listing the VM's metrics by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1981393977
>
> @vishesh92, from the description of your PR:
>
> ```
> This PR adds the upgrade path for 4.19 to 4.19.1 and creates two missing indexes on vm_stats table
> ...
> 2. vm_stats -> timestamp - To speed up clean up of old vm_stats.
> ```
>
> You're proposing the addition of an index on the `timestamp` column to speed up the deletion of old records on `vm_stats`. Why would you test a **SELECT** and not a **DELETE**? Could you share the tests results using a **DELETE** query?
@JoaoJandre We can't run explain analyze with delete commands. Adding the index also helps reduce the load on database while cleaning up the entries. Otherwise the DB will have to go through each record to filter out all the entries which can cause spike in I/O as well. I understand that if the administrator reduces the retention period, it can cause issues but having the index will speed up deletion which runs every minute.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1999668050
> @JoaoJandre, could you make some tests listing and removing data with the proposed index?
@GutoVeronezi, @vishesh92, @rohityadavcloud, @sureshanaparti, @mlsorensen, @kohrar I've made some tests listing and deleting data from the vm_stats table using the proposed index, I've also taken the liberty to reproduce the tests using a index on (`vm_id`) only. I've used the same assumptions from the tests that I've done here: https://github.com/apache/cloudstack/pull/8737#issuecomment-1992241242.
First, I tested two listings: Select all the data on a VM; select the data on a VM between two specific dates.
Without any indexes:
```
MariaDB [teste]> analyze select * from vm_stats where vm_id = 368;
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40198741.00 | 100.00 | 0.10 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (24.905 sec)
```
```
MariaDB [teste]> analyze select * from vm_stats where vm_id = 368 AND (timestamp between '2024-03-02' AND '2024-03-04');
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40198741.00 | 100.00 | 0.03 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (24.528 sec)
```
Using the (`vm_id`,`timestamp`) index:
```
MariaDB [teste]> analyze select * from vm_stats where vm_id = 368;
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------+
| 1 | SIMPLE | vm_stats | ref | index2 | index2 | 8 | const | 87578 | 40866.00 | 100.00 | 100.00 | |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------+
1 row in set (3.687 sec)
```
```
MariaDB [teste]> analyze select * from vm_stats where vm_id = 368 AND (timestamp between '2024-03-02' AND '2024-03-04');
+------+-------------+----------+-------+---------------+--------+---------+------+-------+----------+----------+------------+-----------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+-------+---------------+--------+---------+------+-------+----------+----------+------------+-----------------------+
| 1 | SIMPLE | vm_stats | range | index2 | index2 | 13 | NULL | 23986 | 11581.00 | 0.06 | 100.00 | Using index condition |
+------+-------------+----------+-------+---------------+--------+---------+------+-------+----------+----------+------------+-----------------------+
1 row in set (1.025 sec)
```
Using the (`vm_id`) index:
```
MariaDB [teste]> analyze select * from vm_stats where vm_id = 368;
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------+
| 1 | SIMPLE | vm_stats | ref | index1 | index1 | 8 | const | 87528 | 40866.00 | 100.00 | 100.00 | |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------+
1 row in set (3.381 sec)
```
```
MariaDB [teste]> analyze select * from vm_stats where vm_id = 368 AND (timestamp between '2024-03-02' AND '2024-03-04');
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ref | index1 | index1 | 8 | const | 87528 | 40866.00 | 100.00 | 28.34 | Using where |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------------+
1 row in set (3.365 sec)
```
Again, listing with indexes is much faster, we can see that the select with indexes are an order of magnitude faster then the select without an index. Furthermore, the (`vm_id`,`timestamp`) index is slighty faster than the (`vm_id`) index on the select with a condition on `timestamp`.
Then, I did three delete tests: deleting one minute of data, deleting a whole day of data, and deleting a whole day of data with limit 100000.
Without any indexes:
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-01 00:03:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40190453.00 | 100.00 | 0.01 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (30.399 sec)
```
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-02 00:00:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39281076 | 40186342.00 | 100.00 | 14.26 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (2 min 50.000 sec)
```
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-02 00:00:00' limit 100000;
+------+-------------+----------+------+---------------+------+---------+------+----------+-----------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-----------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 697806.00 | 100.00 | 14.33 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-----------+----------+------------+-------------+
1 row in set (2.023 sec)
```
Using the (`vm_id`,`timestamp`) index:
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-01 00:01:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40198741.00 | 100.00 | 0.01 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (30.866 sec)
```
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-02 00:00:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40198741.00 | 100.00 | 14.29 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (22 min 6.165 sec)
```
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-02 00:00:00' limit 100000;
+------+-------------+----------+------+---------------+------+---------+------+----------+------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39185187 | 1900366.00 | 100.00 | 5.26 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+------------+----------+------------+-------------+
1 row in set (14.068 sec)
```
Using the (`vm_id`) index:
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-01 00:02:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40194609.00 | 100.00 | 0.01 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (31.899 sec)
```
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-02 00:00:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40198741.00 | 100.00 | 14.29 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (7 min 28.040 sec)
```
```
MariaDB [teste]> ANALYZE delete from vm_stats where `timestamp` <= '2024-03-02 00:00:00' limit 100000;
+------+-------------+----------+------+---------------+------+---------+------+----------+------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39185187 | 1299394.00 | 100.00 | 7.70 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+------------+----------+------------+-------------+
1 row in set (5.265 sec)
```
From the delete tests, we can once again see that, when deleting small amounts of data, the index overhead is unnoticeable. However, when deleting larger parts of the table, the indexes make a large difference, with the deletion using (`vm_id`,`timestamp`) being an order of magnitude slower the deletion without any indexes. However, the deletion with the (`vm_id`) index is in the same order of magnitude as the no index deletion, only being about two times slower.
Looking at the test results, we can make the following conclusions:
1. Selects without any indexes take too long (who would've thought?);
2. While the (`vm_id`,`timestamp`) index makes the select faster when filtering for both `vm_id` and `timestamp`, the deletion is much slower;
3. The (`vm_id`) index lowers the select time considerably, being on the same order of magnitude of the (`vm_id`,`timestamp`) speed up, while being only a bit slower when selecting using timestamps. Moreover, the deletion is slower than the no index deletion; however, it's still on the same order of magnitude of the no index deletion.
Therefore, based on these results, I believe that we could change the proposed (`vm_id`,`timestamp`) index to a (`vm_id`) index, as the (`vm_id`) offers a good speed boost for listing, and minimal slowdown when deleting, especially when used with the feature proposed in #8740.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1991683719
> > > @JoaoJandre We can't run explain analyze with delete commands. Adding the index also helps reduce the load on database while cleaning up the entries. Otherwise the DB will have to go through each record to filter out all the entries which can cause spike in I/O as well. I understand that if the administrator reduces the retention period, it can cause issues but having the index will speed up deletion which runs every minute.
> >
> >
> > @vishesh92 you are proposing to add an index to improve DELETE operations; however, you are presenting results for SELECT operations. Indeed, indexes can improve SELECT operations, which only list the data. However, the DELETE operation works different: it removes data, recalculates statistics, reorganizes indexes and more. Please, present some tests and results for DELETE operations; otherwise, it is not feasible to sustain the reason for adding those indexes.
>
> @JoaoJandre Here are the results for a delete operation where the filter doesn't matches any rows.
>
> ```sql
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.001s
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.001s
> ```
>
> query plan
>
> ```sql
> +------------------------------------------------------------------------------------------------+
> | EXPLAIN |
> |------------------------------------------------------------------------------------------------|
> | { |
> | "query_block": { |
> | "select_id": 1, |
> | "table": { |
> | "delete": true, |
> | "table_name": "vm_stats", |
> | "access_type": "range", |
> | "possible_keys": [ |
> | "temp_idx" |
> | ], |
> | "key": "temp_idx", |
> | "used_key_parts": [ |
> | "timestamp" |
> | ], |
> | "key_length": "5", |
> | "ref": [ |
> | "const" |
> | ], |
> | "rows_examined_per_scan": 1, |
> | "filtered": "100.00", |
> | "attached_condition": "(`test`.`vm_stats`.`timestamp` < TIMESTAMP'1950-01-01 00:00:00')" |
> | } |
> | } |
> | } |
> +------------------------------------------------------------------------------------------------+
> ```
>
> After dropping the index
>
> ```sql
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.364s
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.362s
> ```
>
> query plan
>
> ```sql
> +------------------------------------------------------------------------------------------------+
> | EXPLAIN |
> |------------------------------------------------------------------------------------------------|
> | { |
> | "query_block": { |
> | "select_id": 1, |
> | "table": { |
> | "delete": true, |
> | "table_name": "vm_stats", |
> | "access_type": "ALL", |
> | "rows_examined_per_scan": 660340, |
> | "filtered": "100.00", |
> | "attached_condition": "(`test`.`vm_stats`.`timestamp` < TIMESTAMP'1950-01-01 00:00:00')" |
> | } |
> | } |
> | } |
> +------------------------------------------------------------------------------------------------+
> ```
>
> As you can see, without the index it takes around 0.36 seconds because it has to go through each row in the database. It takes 0.001 seconds with the index because it doesn't need to go through all the rows in the table.
@vishesh92 _vm_stats_ table data, timestamp column cardinality in both cases are the same? please check/confirm results with the filter matching rows, and same delete count.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2006202140
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud merged PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2006698671
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1991036038
> > @JoaoJandre We can't run explain analyze with delete commands. Adding the index also helps reduce the load on database while cleaning up the entries. Otherwise the DB will have to go through each record to filter out all the entries which can cause spike in I/O as well. I understand that if the administrator reduces the retention period, it can cause issues but having the index will speed up deletion which runs every minute.
>
> @vishesh92 you are proposing to add an index to improve DELETE operations; however, you are presenting results for SELECT operations. Indeed, indexes can improve SELECT operations, which only list the data. However, the DELETE operation works different: it removes data, recalculates statistics, reorganizes indexes and more. Please, present some tests and results for DELETE operations; otherwise, it is not feasible to sustain the reason for adding those indexes.
@JoaoJandre Here are the results for a delete operation where the filter doesn't matches any rows.
```sql
MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
Query OK, 0 rows affected
Time: 0.001s
MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
Query OK, 0 rows affected
Time: 0.001s
```
query plan
```sql
+------------------------------------------------------------------------------------------------+
| EXPLAIN |
|------------------------------------------------------------------------------------------------|
| { |
| "query_block": { |
| "select_id": 1, |
| "table": { |
| "delete": true, |
| "table_name": "vm_stats", |
| "access_type": "range", |
| "possible_keys": [ |
| "temp_idx" |
| ], |
| "key": "temp_idx", |
| "used_key_parts": [ |
| "timestamp" |
| ], |
| "key_length": "5", |
| "ref": [ |
| "const" |
| ], |
| "rows_examined_per_scan": 1, |
| "filtered": "100.00", |
| "attached_condition": "(`test`.`vm_stats`.`timestamp` < TIMESTAMP'1950-01-01 00:00:00')" |
| } |
| } |
| } |
+------------------------------------------------------------------------------------------------+
```
After dropping the index
```sql
MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
Query OK, 0 rows affected
Time: 0.364s
MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
Query OK, 0 rows affected
Time: 0.362s
```
query plan
```sql
+------------------------------------------------------------------------------------------------+
| EXPLAIN |
|------------------------------------------------------------------------------------------------|
| { |
| "query_block": { |
| "select_id": 1, |
| "table": { |
| "delete": true, |
| "table_name": "vm_stats", |
| "access_type": "ALL", |
| "rows_examined_per_scan": 660340, |
| "filtered": "100.00", |
| "attached_condition": "(`test`.`vm_stats`.`timestamp` < TIMESTAMP'1950-01-01 00:00:00')" |
| } |
| } |
| } |
+------------------------------------------------------------------------------------------------+
```
As you can see, without the index it takes around 0.36 seconds because it has to go through each row in the database.
It takes 0.001 seconds with the index because it doesn't need to go through all the rows in the table.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1995638271
I have removed the index on `timestamp`.
The index on `(vm_id, timestamp)` should help with faster listing of metrics for a vm. As @rohityadavcloud said this index helped bring down the latency from 15s to 1s in one of our environments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1998711554
<b>[SF] Trillian test result (tid-9478)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49903 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8737-t9478-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1983608195
> @JoaoJandre We can't run explain analyze with delete commands. Adding the index also helps reduce the load on database while cleaning up the entries. Otherwise the DB will have to go through each record to filter out all the entries which can cause spike in I/O as well. I understand that if the administrator reduces the retention period, it can cause issues but having the index will speed up deletion which runs every minute.
@vishesh92 you are proposing to add an index to improve DELETE operations; however, you are presenting results for SELECT operations. Indeed, indexes can improve SELECT operations, which only list the data. However, the DELETE operation works different: it removes data, recalculates statistics, reorganizes indexes and more. Please, present some tests and results for DELETE operations; otherwise, it is not feasible to sustain the reason for adding those indexes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2000720694
<b>[SF] Trillian test result (tid-9497)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 48937 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8737-t9497-vmware-67u3.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1999705378
@JoaoJandre Can you share query times for this query?
```sql
SELECT vm_stats.id, vm_stats.vm_id, vm_stats.mgmt_server_id, vm_stats.timestamp, vm_stats.vm_stats_data FROM vm_stats WHERE vm_stats.vm_id = 368 ORDER BY vm_stats.timestamp DESC;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997014413
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#discussion_r1511158717
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41900to41910.java:
##########
@@ -0,0 +1,91 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.upgrade.dao;
+
+import com.cloud.upgrade.SystemVmTemplateRegistration;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.log4j.Logger;
+
+import java.io.InputStream;
+import java.sql.Connection;
+
+public class Upgrade41900to41910 implements DbUpgrade, DbUpgradeSystemVmTemplate {
+ final static Logger LOG = Logger.getLogger(Upgrade41900to41910.class);
+ private SystemVmTemplateRegistration systemVmTemplateRegistration;
+
+ @Override
+ public String[] getUpgradableVersionRange() {
+ return new String[] {"4.19.0.0", "4.19.1.0"};
+ }
+
+ @Override
+ public String getUpgradedVersion() {
+ return "4.19.1.0";
+ }
+
+ @Override
+ public boolean supportsRollingUpgrade() {
+ return false;
+ }
+
+ @Override
+ public InputStream[] getPrepareScripts() {
+ final String scriptFile = "META-INF/db/schema-41900to41910.sql";
+ final InputStream script = Thread.currentThread().getContextClassLoader().getResourceAsStream(scriptFile);
+ if (script == null) {
+ throw new CloudRuntimeException("Unable to find " + scriptFile);
+ }
+
+ return new InputStream[] {script};
+ }
+
+ @Override
+ public void performDataMigration(Connection conn) {
+ addIndexes(conn);
+ }
+
+ @Override
+ public InputStream[] getCleanupScripts() {
+ final String scriptFile = "META-INF/db/schema-41900to41910-cleanup.sql";
+ final InputStream script = Thread.currentThread().getContextClassLoader().getResourceAsStream(scriptFile);
+ if (script == null) {
+ throw new CloudRuntimeException("Unable to find " + scriptFile);
+ }
+
+ return new InputStream[] {script};
+ }
+
+ private void initSystemVmTemplateRegistration() {
+ systemVmTemplateRegistration = new SystemVmTemplateRegistration("");
+ }
+
+ @Override
+ public void updateSystemVmTemplates(Connection conn) {
+ LOG.debug("Updating System Vm template IDs");
+ initSystemVmTemplateRegistration();
+ try {
+ systemVmTemplateRegistration.updateSystemVmTemplates(conn);
+ } catch (Exception e) {
+ throw new CloudRuntimeException("Failed to find / register SystemVM template(s)");
+ }
+ }
+
+ private void addIndexes(Connection conn) {
+ DbUpgradeUtils.addIndexIfNeeded(conn, "vm_stats", "vm_id", "timestamp");
+ DbUpgradeUtils.addIndexIfNeeded(conn, "vm_stats", "timestamp");
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1980809847
> > @vishesh92 could you share a description of the tests that you've done and their results?
>
> Sure. I created a table with 661683 entries for timestamp ranging from `1970-11-29` till `2024-03-07 00:00:00`.
>
> ```sql
> EXPLAIN ANALYZE SELECT * FROM vm_stats WHERE timestamp < '2000-01-01' ;
> ```
>
> Query plan with index
>
> ```sql
> +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
> | EXPLAIN |
> |--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
> | -> Filter: (vm_stats.`timestamp` < TIMESTAMP'2000-01-01 00:00:00') (cost=66558 rows=330170) (actual time=0.0104..47.4 rows=282075 loops=1) |
> | -> Covering index range scan on vm_stats using temp_idx over (timestamp < '2000-01-01 00:00:00') (cost=66558 rows=330170) (actual time=0.00948..35.4 rows=282075 loops=1) |
> +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
> ```
>
> Query plan without index
>
> ```sql
> +---------------------------------------------------------------------------------------------------------------------------------------------+
> | EXPLAIN |
> |---------------------------------------------------------------------------------------------------------------------------------------------|
> | -> Filter: (vm_stats.`timestamp` < TIMESTAMP'2000-01-01 00:00:00') (cost=66500 rows=220091) (actual time=0.0198..87.5 rows=282075 loops=1) |
> | -> Table scan on vm_stats (cost=66500 rows=660340) (actual time=0.0186..64 rows=661683 loops=1) |
> +---------------------------------------------------------------------------------------------------------------------------------------------+
> ```
>
> As you can see the index reduced the query time by ~40%.
@vishesh92, from the description of your PR:
```
This PR adds the upgrade path for 4.19 to 4.19.1 and creates two missing indexes on vm_stats table
...
2. vm_stats -> timestamp - To speed up clean up of old vm_stats.
```
You're proposing the addition of an index on the `timestamp` column to speed up the deletion of old records on `vm_stats`. Why would you test a **SELECT** and not a **DELETE**? Could you share the tests results using a **DELETE** query?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2011636987
Merging based on reviews, smoketests and Bobby's manual testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1978101928
> Hey, @vishesh92
>
> Adding the `vm_stats` -> `timestamp` index to the table might not produce the effect that you want. When deleting very specific rows, adding an index to the table will generally make the deletion much faster. However, when a table has indexes, those must also be updated on deletes (and inserts too); therefore, if you are deleting large parts of a table, such as when deleting all the entries older then x time, your delete query will probably be slower.
>
> I've actually ran into a situation where a user was retaining the last 30 days of VM statistics, but decided that they wanted to start retaining only 7 days. When changing the retain time from 30 days to 7 days, ACS had to delete 23 days worth of data. Because of the number of VMs and MGMT servers on his enviroment, they had about 1 billion entries to delete. ACS had trouble to perform this operation as the delete was being done in a single query that always timed out; thus the table started to grow endlessly. I've tried to add this exact index you are proposing; but, when I tested deleting 10 million rows of `vm_stats`, the index actually made the delete take double the amount of time.
>
> I've found another solution to the `vm_stats` delete problem. You can check it out on #8740
@JoaoJandre I understand what you mean. But for day to day operations, the index on `timestamp` will make the operation much faster. The scenario you mentioned will only occur when the operator changes the stats retention interval which IMO doesn't happen too frequently. And with the changes you are suggesting in #8740, the query won't timeout since we are deleting only a small part of the table.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976344743
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976456644
@vishesh92 a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997138897
@sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "borisstoyanov (via GitHub)" <gi...@apache.org>.
borisstoyanov commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1999321252
@blueorangutan test matrix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1977813109
<b>[SF] Trillian test result (tid-9382)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47963 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8737-t9382-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1995191529
As proved by @JoaoJandre in comment https://github.com/apache/cloudstack/pull/8737#issuecomment-1992241242, the indiscriminate addition of an index for `timestamp` might cause more problems than benefits.
> I only noticed this after users complained that listing VMs when creating a port forwarding was taking >30 seconds to load
@kohrar, the way to address an issue like this should use a more rational manner. For instance, if we ask ourselves why metrics storage are affecting the VM listing on the port forwarding view, we might see/look for a different solution. The port forwarding view is calling `listVirtualMachines`, which by default (honestly, I do not know why it does that when we have `listVirtualMachinesMetrics` and `listVirtualMachinesUsageHistory`) makes a join with the statistics table. The problem is that the API `listVirtualMachines` is not working in a optimized fashion (returning VM statistics by default), and the UI is not passing the parameter `details` to not list the statistics. Therefore, the problem in this view is not the lack of indexes; but the API working/being called in a non-optimized way. I already talked with @JoaoJandre to work on a patch for this situation.
> Yes I think we can all agree that the big one here is index on vm_id.
Indeed, the index for `vm_id` might be good to have; however, we need some more testing as well.
> if it's so slow to delete that it is impossible to do so, or breaks something, then that needs to be resolved, maybe by batch deletion or something.
@mlsorensen, you can check @JoaoJandre's PR #8740 regarding the batch deletion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997133428
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8934
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997344952
fwiw @borisstoyanov is helping to test this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2006406250
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8969
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2008291380
<b>[SF] Trillian test result (tid-9524)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43389 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8737-t9524-kvm-centos7.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_trigger_shutdown | `Failure` | 336.56 | test_safe_shutdown.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976576132
Hey, @vishesh92
Adding the `vm_stats` -> `timestamp` index to the table might not produce the effect that you want. When deleting very specific rows, adding an index to the table will generally make the deletion much faster. However, when a table has indexes, those must also be updated on deletes (and inserts too); therefore, if you are deleting large parts of a table, such as when deleting all the entries older then x time, your delete query will probably be slower.
I've actually ran into a situation where a user was retaining the last 30 days of VM statistics, but decided that they wanted to start retaining only 7 days. When changing the retain time from 30 days to 7 days, ACS had to delete 23 days worth of data. Because of the number of VMs and MGMT servers on his enviroment, they had about 1 billion entries to delete. ACS had trouble to perform this operation as the delete was being done in a single query that always timed out; thus the table started to grow endlessly. I've tried to add this exact index you are proposing; but, when I tested deleting 10 million rows of `vm_stats`, the index actually made the delete take double the amount of time.
I've found another solution to the `vm_stats` delete problem. You can check it out on #8740
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1979253335
> > Hey, @vishesh92
> > Adding the `vm_stats` -> `timestamp` index to the table might not produce the effect that you want. When deleting very specific rows, adding an index to the table will generally make the deletion much faster. However, when a table has indexes, those must also be updated on deletes (and inserts too); therefore, if you are deleting large parts of a table, such as when deleting all the entries older then x time, your delete query will probably be slower.
> > I've actually ran into a situation where a user was retaining the last 30 days of VM statistics, but decided that they wanted to start retaining only 7 days. When changing the retain time from 30 days to 7 days, ACS had to delete 23 days worth of data. Because of the number of VMs and MGMT servers on his enviroment, they had about 1 billion entries to delete. ACS had trouble to perform this operation as the delete was being done in a single query that always timed out; thus the table started to grow endlessly. I've tried to add this exact index you are proposing; but, when I tested deleting 10 million rows of `vm_stats`, the index actually made the delete take double the amount of time.
> > I've found another solution to the `vm_stats` delete problem. You can check it out on #8740
>
> @JoaoJandre I understand what you mean. But for day to day operations, the index on `timestamp` will make the operation much faster. The scenario you mentioned will only occur when the operator changes the stats retention interval which IMO doesn't happen too frequently. And with the changes you are suggesting in #8740, the query won't timeout since we are deleting only a small part of the table.
@vishesh92 could you share a description of the tests that you've done and their results?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2000697873
<b>[SF] Trillian test result (tid-9496)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48441 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8737-t9496-xenserver-71.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_cancel_host_maintenace_with_no_migration_jobs | `Error` | 1040.48 | test_host_maintenance.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2000762113
<b>[SF] Trillian test result (tid-9498)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49936 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8737-t9498-kvm-centos7.zip
Smoke tests completed. 123 look OK, 6 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_unsecure_vm_migration | `Error` | 224.53 | test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration | `Error` | 273.24 | test_vm_life_cycle.py
test_01_create_vm_snapshots | `Failure` | 810.66 | test_vm_snapshots.py
test_02_revert_vm_snapshots | `Failure` | 810.79 | test_vm_snapshots.py
test_03_delete_vm_snapshots | `Failure` | 0.02 | test_vm_snapshots.py
test_04_deploy_vnf_appliance | `Error` | 102.44 | test_vnf_templates.py
test_04_deploy_vnf_appliance | `Error` | 102.44 | test_vnf_templates.py
test_05_delete_vnf_template | `Error` | 1.07 | test_vnf_templates.py
ContextSuite context=TestVnfTemplates>:teardown | `Error` | 1.15 | test_vnf_templates.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 372.69 | test_vpc_redundant.py
test_02_redundant_VPC_default_routes | `Error` | 362.60 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Error` | 240.13 | test_vpc_redundant.py
test_04_rvpc_network_garbage_collector_nics | `Error` | 186.84 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 277.02 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 277.04 | test_vpc_redundant.py
test_01_VPC_nics_after_destroy | `Error` | 185.27 | test_vpc_router_nics.py
test_02_VPC_default_routes | `Error` | 193.49 | test_vpc_router_nics.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 423.11 | test_vpc_vpn.py
test_01_vpc_site2site_vpn_multiple_options | `Failure` | 245.56 | test_vpc_vpn.py
test_01_vpc_site2site_vpn | `Failure` | 271.98 | test_vpc_vpn.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1992241242
> > > @JoaoJandre We can't run explain analyze with delete commands. Adding the index also helps reduce the load on database while cleaning up the entries. Otherwise the DB will have to go through each record to filter out all the entries which can cause spike in I/O as well. I understand that if the administrator reduces the retention period, it can cause issues but having the index will speed up deletion which runs every minute.
> >
> >
> > @vishesh92 you are proposing to add an index to improve DELETE operations; however, you are presenting results for SELECT operations. Indeed, indexes can improve SELECT operations, which only list the data. However, the DELETE operation works different: it removes data, recalculates statistics, reorganizes indexes and more. Please, present some tests and results for DELETE operations; otherwise, it is not feasible to sustain the reason for adding those indexes.
>
> @JoaoJandre Here are the results for a delete operation where the filter doesn't matches any rows.
>
> ```sql
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.001s
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.001s
> ```
>
> query plan
>
> ```sql
> +------------------------------------------------------------------------------------------------+
> | EXPLAIN |
> |------------------------------------------------------------------------------------------------|
> | { |
> | "query_block": { |
> | "select_id": 1, |
> | "table": { |
> | "delete": true, |
> | "table_name": "vm_stats", |
> | "access_type": "range", |
> | "possible_keys": [ |
> | "temp_idx" |
> | ], |
> | "key": "temp_idx", |
> | "used_key_parts": [ |
> | "timestamp" |
> | ], |
> | "key_length": "5", |
> | "ref": [ |
> | "const" |
> | ], |
> | "rows_examined_per_scan": 1, |
> | "filtered": "100.00", |
> | "attached_condition": "(`test`.`vm_stats`.`timestamp` < TIMESTAMP'1950-01-01 00:00:00')" |
> | } |
> | } |
> | } |
> +------------------------------------------------------------------------------------------------+
> ```
>
> After dropping the index
>
> ```sql
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.364s
> MySQL root@(none):test> DELETE FROM vm_stats WHERE timestamp < '1950-01-01';
> Query OK, 0 rows affected
> Time: 0.362s
> ```
>
> query plan
>
> ```sql
> +------------------------------------------------------------------------------------------------+
> | EXPLAIN |
> |------------------------------------------------------------------------------------------------|
> | { |
> | "query_block": { |
> | "select_id": 1, |
> | "table": { |
> | "delete": true, |
> | "table_name": "vm_stats", |
> | "access_type": "ALL", |
> | "rows_examined_per_scan": 660340, |
> | "filtered": "100.00", |
> | "attached_condition": "(`test`.`vm_stats`.`timestamp` < TIMESTAMP'1950-01-01 00:00:00')" |
> | } |
> | } |
> | } |
> +------------------------------------------------------------------------------------------------+
> ```
>
> As you can see, without the index it takes around 0.36 seconds because it has to go through each row in the database. It takes 0.001 seconds with the index because it doesn't need to go through all the rows in the table.
@vishesh92, I'm sorry, but I don't understand the point of a DELETE test where no rows are deleted.
I've decided to test/compare the performance of deletion on the vm_stats table using a more realistic scenario. Consider the following:
We have 1000 VMs, 2 MGMT servers each collecting stats every 30 seconds, and we have a retention time of 7 days, this will give us a table size of around 40 million rows. Thus, I've created a mock vm_stats table with that many rows, using random dates between `2024-03-01 00:00:00` and `2024-03-08 00:00:00` and made the following tests (I've regenerated the table when needed):
Without adding the proposed indexes, I deleted rows that had a timestamp lower then `2024-03-01 00:01:00` :
```
MariaDB [teste]> ANALYZE DELETE FROM vm_stats WHERE timestamp < '2024-03-01 00:01:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39379728 | 40198741.00 | 100.00 | 0.01 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (31.688 sec)
```
After, I added the indexes and deleted all the rows with timestamp lower then `2024-03-01 00:02:00` (the ones before 00:01:00 were deleted already by the earlier test):
```
MariaDB [teste]> ANALYZE DELETE FROM vm_stats WHERE timestamp < '2024-03-01 00:02:00';
+------+-------------+----------+-------+---------------+--------+---------+------+------+---------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+-------+---------------+--------+---------+------+------+---------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | range | index1 | index1 | 5 | NULL | 4148 | 4148.00 | 100.00 | 100.00 | Using where |
+------+-------------+----------+-------+---------------+--------+---------+------+------+---------+----------+------------+-------------+
1 row in set (0.819 sec)
```
Great! the indexes made it much faster! roughly 30 times faster! Let's test deleting a whole day's worth of rows then:
Without indexes:
```
MariaDB [teste]> ANALYZE DELETE FROM vm_stats WHERE timestamp < '2024-03-02 00:00:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40198741.00 | 100.00 | 14.29 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (2 min 57.537 sec)
```
With indexes:
```
MariaDB [teste]> ANALYZE DELETE FROM vm_stats WHERE timestamp < '2024-03-02 00:00:00';
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | index1 | NULL | NULL | NULL | 39371512 | 40190525.00 | 100.00 | 14.27 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-------------+----------+------------+-------------+
1 row in set (45 min 50.800 sec)
```
The performance was about 15 times worst this time with indexes. This happens because of what I've explained in this comment: https://github.com/apache/cloudstack/pull/8737#issuecomment-1983608195. When deleting a small amount of the data (0.01% of it), we can find the specific rows we need to delete way faster using the indexes and all the overhead of updating them is worth it. However, when deleting a bigger amount of data (14% of the table) we can see the impact of the indexes' overhead, they make the deletion much slower.
The above tests only consider the proposed PR, but I've also tested the performance of the indexes alongside what's proposed on #8740. I've tested making a query to delete a whole day's worth of rows with a limit of 100000 :
Without indexes:
```
MariaDB [teste]> ANALYZE DELETE FROM vm_stats WHERE timestamp < '2024-03-02 00:00:00' limit 100000;
+------+-------------+----------+------+---------------+------+---------+------+----------+-----------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+-----------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 697806.00 | 100.00 | 14.33 | Using where |
+------+-------------+----------+------+---------------+------+---------+------+----------+-----------+----------+------------+-------------+
1 row in set (2.075 sec)
```
With indexes:
```
MariaDB [teste]> ANALYZE DELETE FROM vm_stats WHERE timestamp < '2024-03-02 00:00:00' limit 100000;
+------+-------------+----------+-------+---------------+--------+---------+------+----------+-----------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+-------+---------------+--------+---------+------+----------+-----------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | range | index1 | index1 | 5 | NULL | 12540998 | 100000.00 | 100.00 | 100.00 | Using where |
+------+-------------+----------+-------+---------------+--------+---------+------+----------+-----------+----------+------------+-------------+
1 row in set (21.133 sec)
```
Deleting without indexes was 10 times faster then with indexes.
Looking at the tests that I've done, I can agree with you that, for the most common case (deleting 1 minute worth of data), the index addition is worth it. However, if at any time the user decides that they want to retain less stats, the query to delete the excess stats might take much longer, and eventually timeout, leading to a snowball where ACS is unable to clean the `vm_stats` table and it keeps growing, making the problem worse.
Another point is that, while the feature on #8740 is optional, and by default turned off, the indexes proposed here are not optional (at least not without manual DB intervention). Taking this into consideration, I really think that adding the `timestamp` index is not the best approach, as it might lead to the problems that I've described both here and on #8740.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1999324607
@borisstoyanov a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2003875216
Sure @vishesh92 , here are the results:
No index:
```
MariaDB [teste]> ANALYZE SELECT vm_stats.id, vm_stats.vm_id, vm_stats.mgmt_server_id, vm_stats.timestamp, vm_stats.vm_stats_data FROM vm_stats WHERE vm_stats.vm_id = 368 ORDER BY vm_stats.timestamp DESC;
+------+-------------+----------+------+---------------+------+---------+------+----------+----------+----------+------------+-----------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+------+---------+------+----------+----------+----------+------------+-----------------------------+
| 1 | SIMPLE | vm_stats | ALL | NULL | NULL | NULL | NULL | 39285187 | 40866.00 | 100.00 | 100.00 | Using where; Using filesort |
+------+-------------+----------+------+---------------+------+---------+------+----------+----------+----------+------------+-----------------------------+
1 row in set (27.276 sec)
```
Index on (`vm_id`, `timestamp`):
```
MariaDB [teste]> ANALYZE SELECT vm_stats.id, vm_stats.vm_id, vm_stats.mgmt_server_id, vm_stats.timestamp, vm_stats.vm_stats_data FROM vm_stats WHERE vm_stats.vm_id = 368 ORDER BY vm_stats.timestamp DESC;
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------------+
| 1 | SIMPLE | vm_stats | ref | index2 | index2 | 8 | const | 87578 | 40866.00 | 100.00 | 100.00 | Using where |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-------------+
1 row in set (4.544 sec)
```
Index on (`vm_id`):
```
MariaDB [teste]> ANALYZE SELECT vm_stats.id, vm_stats.vm_id, vm_stats.mgmt_server_id, vm_stats.timestamp, vm_stats.vm_stats_data FROM vm_stats WHERE vm_stats.vm_id = 368 ORDER BY vm_stats.timestamp DESC;
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-----------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | r_rows | filtered | r_filtered | Extra |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-----------------------------+
| 1 | SIMPLE | vm_stats | ref | index2 | index2 | 8 | const | 87528 | 40866.00 | 100.00 | 100.00 | Using where; Using filesort |
+------+-------------+----------+------+---------------+--------+---------+-------+-------+----------+----------+------------+-----------------------------+
1 row in set (8.123 sec)
```
The results/conclusions are the same: No index is very slow; the (`vm_id`, `timestamp`) index is the fastest for listing when considering the `timestamp`, and the (`vm_id`) index is much faster than without index, but a bit slower than the (`vm_id`, `timestamp`) index.
One point to consider is that this query is returning one whole week's worth of data for a VM and ordering it. That amounts to 40866 rows in this example (that have to be selected from 40 million rows in the table); It's expected that it should take some time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2006204880
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976353616
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8737?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 4.39%. Comparing base [(`a7ec873`)](https://app.codecov.io/gh/apache/cloudstack/commit/a7ec8738a27aaa7fae7a03f1f1aa8bdbaea1def3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`5faf462`)](https://app.codecov.io/gh/apache/cloudstack/pull/8737?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.19 #8737 +/- ##
============================================
- Coverage 30.93% 4.39% -26.54%
============================================
Files 5353 361 -4992
Lines 376055 28630 -347425
Branches 54691 4996 -49695
============================================
- Hits 116317 1258 -115059
+ Misses 244443 27233 -217210
+ Partials 15295 139 -15156
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8737/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8737/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8737/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.39% <ø> (ø)` | |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8737/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8737?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976453050
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8846
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#discussion_r1531932222
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41900to41910.java:
##########
@@ -61,6 +66,25 @@ public InputStream[] getCleanupScripts() {
throw new CloudRuntimeException("Unable to find " + scriptFile);
}
- return new InputStream[] {script};
+ return new InputStream[]{script};
+ }
+
+ private void addIndexes(Connection conn) {
+ DbUpgradeUtils.addIndexIfNeeded(conn, "vm_stats", "vm_id");
+ }
+
+ @Override
+ public void updateSystemVmTemplates(Connection conn) {
+ LOG.debug("Updating System Vm template IDs");
+ initSystemVmTemplateRegistration();
+ try {
+ systemVmTemplateRegistration.updateSystemVmTemplates(conn);
+ } catch (Exception e) {
+ throw new CloudRuntimeException("Failed to find / register SystemVM template(s)");
+ }
+ }
+
+ private void initSystemVmTemplateRegistration() {
+ systemVmTemplateRegistration = new SystemVmTemplateRegistration("");
Review Comment:
Is this relevant to the PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on code in PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#discussion_r1533432581
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41900to41910.java:
##########
@@ -61,6 +66,25 @@ public InputStream[] getCleanupScripts() {
throw new CloudRuntimeException("Unable to find " + scriptFile);
}
- return new InputStream[] {script};
+ return new InputStream[]{script};
+ }
+
+ private void addIndexes(Connection conn) {
+ DbUpgradeUtils.addIndexIfNeeded(conn, "vm_stats", "vm_id");
+ }
+
+ @Override
+ public void updateSystemVmTemplates(Connection conn) {
+ LOG.debug("Updating System Vm template IDs");
+ initSystemVmTemplateRegistration();
+ try {
+ systemVmTemplateRegistration.updateSystemVmTemplates(conn);
+ } catch (Exception e) {
+ throw new CloudRuntimeException("Failed to find / register SystemVM template(s)");
+ }
+ }
+
+ private void initSystemVmTemplateRegistration() {
+ systemVmTemplateRegistration = new SystemVmTemplateRegistration("");
Review Comment:
These are common pattern of changes, without this CloudStack doesn't check/auto-configure/install/upgrade systemvmtemplates.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1991877845
> In case of a delete operation with some filter, before deleting the records database needs to fetch the records from the database. If the index is not present, this fetching of records from the database becomes slow and leads to scanning of all rows in the table resulting in high I/O on disk.
thanks for sharing the results @vishesh92 , seems there is some improvement with the index.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997135543
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "mlsorensen (via GitHub)" <gi...@apache.org>.
mlsorensen commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1994732867
Yes I think we can all agree that the big one here is index on vm_id. Missing this is absolutely crushing for default listVirtualMachines calls (where stats are included). Maybe we can at least get that index in place and discuss further on whether or not the timestamp index is beneficial overall.
FWIW I don't see a problem with an odd deletion of stats taking a long time occasionally, if an index improves the majority use cases. However, if it's so slow to delete that it is impossible to do so, or breaks something, then that needs to be resolved, maybe by batch deletion or something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976455722
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1976343333
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "kohrar (via GitHub)" <gi...@apache.org>.
kohrar commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1992300251
Thanks for the detailed breakdown on your deletion performance @JoaoJandre. Since you have so many rows in the vm_stats table, what's the performance like when you run a list VM command (eg. `list virtualmachines listall=true`)? Without an index on timestamp, are you noticing any significant slowdowns with these commands? I only noticed this after users complained that listing VMs when creating a port forwarding was taking >30 seconds to load. Our instance did have `vm.stats.increment.metrics` enabled which we've subsequently disabled to help speed up these types of API calls.
> However, if at any time the user decides that they want to retain less stats, the query to delete the excess stats might take much longer, and eventually timeout, leading to a snowball where ACS is unable to clean the vm_stats table and it keeps growing, making the problem worse.
Where is the timeout happening and could the timeout be increased? A slow deletion happening in the background isn't as impactful as a slow or unresponsive user interface, so I'd still rather have the index in place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997337903
@JoaoJandre, could you make some tests listing and removing data with the proposed index?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1996999952
Thanks @vishesh92 @JoaoJandre & others involved. Good to see your insights / results on indexing the `timestamp`. I think, more tests are required with varied data and deletion checks with different conditions (and percentage of data deleted) to conclude if it's useful or not for most of the use cases. Maybe, we can park this now, and re-visit later in case any need of it for major use cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1991855141
> @vishesh92 _vm_stats_ table data, timestamp column cardinality in both cases are the same? please check/confirm results with the filter matching rows, and same delete count.
Here are the stats.
```sql
MySQL root@(none):test> DELETE FROM vm_stats_without_index WHERE timestamp < '1972-05-01'; -- without index
Query OK, 9213 rows affected
Time: 0.414s
MySQL root@(none):test> DELETE FROM vm_stats_with_index WHERE timestamp < '1972-05-01'; -- with index
Query OK, 9213 rows affected
Time: 0.100s
```
```sql
MySQL root@(none):test> DELETE FROM vm_stats_without_index WHERE timestamp < '1980-01-01';
You're about to run a destructive command.
Do you want to proceed? (y/n): y
Your call!
Query OK, 84840 rows affected
Time: 0.438s
MySQL root@(none):test> DELETE FROM vm_stats_with_index WHERE timestamp < '1980-01-01';
You're about to run a destructive command.
Do you want to proceed? (y/n): y
Your call!
Query OK, 84840 rows affected
Time: 0.375s
```
In case of a delete operation with some filter, before deleting the records database needs to fetch the records from the database. If the index is not present, this fetching of records from the database becomes slow and leads to scanning of all rows in the table resulting in high I/O on disk.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-1997017823
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add indexes for vm_stats [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8737:
URL: https://github.com/apache/cloudstack/pull/8737#issuecomment-2006702290
@rohityadavcloud a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org