Add batch deletion support to removeRawUsageRecords
#12522
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12522 +/- ##
============================================
+ Coverage 4.03% 16.25% +12.21%
- Complexity 0 13400 +13400
============================================
Files 402 5658 +5256
Lines 32715 499276 +466561
Branches 5831 60601 +54770
============================================
+ Hits 1319 81134 +79815
- Misses 31241 409095 +377854
- Partials 155 9047 +8892
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| public static final ConfigKey<Long> DELETE_QUERY_BATCH_SIZE = new ConfigKey<>("Advanced", Long.class, "delete.query.batch.size", "0", | ||
| "Indicates the limit applied while deleting entries in bulk. With this, the delete query will apply the limit as many times as necessary," + | ||
| " to delete all the entries. This is advised when retaining several days of records, which can lead to slowness. <= 0 means that no limit will " + | ||
| "be applied. Default value is 0. For now, this is used for deletion of vm & volume stats only.", true); | ||
| "be applied. Default value is 0. For now, this is used for deletion of VM stats, volume stats, and usage records.", true); |
There was a problem hiding this comment.
should we make the default some biug value to protect the operator from trying to delete 500000+ records? (i am thinking about 10000)
There was a problem hiding this comment.
Yes, I think it would be good to enable batch deletion by default. 10,000 is too low and might end up slowing down deletion too much though. I've seen vm_stats with over 100 million entries for deletion, which would take too long with batches of 10,000. I will check how some different values behave later.
By the way, I'm putting this in draft because I noticed an issue in the code.
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
clgtm otherwise btw
|
@blueorangutan package |
|
@winterhazel 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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16569 |
|
@blueorangutan test |
|
@borisstoyanov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
5 participants
Description
This change allows the batch removal of usage records in order to prevent query timeouts when there is a huge amount of records to delete. Also, an useless
ORDER BYthat would result in higher response times was removed from the batch expunge query.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
In a local environment, I enabled batch deletion by setting a value greater than 0 to
delete.query.batch.size, calledremoveRawUsageRecords, and debugged to code to validate that the deletion was being correctly performed in batches.