Change vm.stats.remove.batch.size to delete.batch.query.size & allow delete of volume_stats in batches #9283

vishesh92 · 2024-06-21T19:46:49Z

Description

This PR changes vm.stats.remove.batch.size configuration parameter to delete.batch.query.size to make it more generic.

Changes also allow deletion of volume_stats in batches using the delete.batch.query.size global setting.

For context, please check the conversation in PR: #8740

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)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

vishesh92 · 2024-06-21T19:47:54Z

@blueorangutan package

blueorangutan · 2024-06-21T19:50:03Z

@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.

blueorangutan · 2024-06-21T20:56:10Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10068

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

shwstppr

Code LGTM

codecov · 2024-06-24T10:10:05Z

Codecov Report

Attention: Patch coverage is 11.76471% with 15 lines in your changes missing coverage. Please review.

Project coverage is 14.97%. Comparing base (0b857de) to head (745b212).
Report is 160 commits behind head on 4.19.

Files	Patch %	Lines
...java/com/cloud/storage/dao/VolumeStatsDaoImpl.java	0.00%	12 Missing ⚠️
...src/main/java/com/cloud/server/StatsCollector.java	33.33%	2 Missing ⚠️
.../cloud/configuration/ConfigurationManagerImpl.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##               4.19    #9283       +/-   ##
=============================================
- Coverage     30.70%   14.97%   -15.74%     
+ Complexity    34232    11046    -23186     
=============================================
  Files          5376     5390       +14     
  Lines        378222   470973    +92751     
  Branches      55053    57438     +2385     
=============================================
- Hits         116135    70512    -45623     
- Misses       246719   392627   +145908     
+ Partials      15368     7834     -7534

Flag	Coverage Δ
simulator-marvin-tests	`?`
uitests	`4.27% <ø> (-0.04%)`	⬇️
unittests	`15.68% <11.76%> (-0.97%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

vishesh92 · 2024-06-24T10:10:31Z

@blueorangutan package

blueorangutan · 2024-06-24T10:12:04Z

@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.

blueorangutan · 2024-06-24T11:13:30Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10094

JoaoJandre · 2024-06-24T12:19:26Z

Hey @vishesh92,
As I said on #8740, I do not think that it makes sense to have a configuration with a generic name that only applies to a single scenario. This misleads the users into thinking that ACS will apply it everywhere, even if you add a note at the end of the description saying otherwise. My point on #8740 was that if we are applying the same concept on other deletes, then we can create a generic configuration. Just renaming a specific configuration to be generic and changing nothing else does not seem to make much sense.

vishesh92 · 2024-06-24T12:38:13Z

Hey @vishesh92, As I said on #8740, I do not think that it makes sense to have a configuration with a generic name that only applies to a single scenario. This misleads the users into thinking that ACS will apply it everywhere, even if you add a note at the end of the description saying otherwise. My point on #8740 was that if we are applying the same concept on other deletes, then we can create a generic configuration. Just renaming a specific configuration to be generic and changing nothing else does not seem to make much sense.

@JoaoJandre I think I misunderstood what you meant. I thought you didn't want to increase the scope of your PR. As I mentioned earlier in PR #8740, I know it doesn't make sense now. But I am assuming we will be doing this for other tables as well in the future. Do you have any plan on how to deprecate the global settings in the future when we have similar global setting for different tables?

vishesh92 · 2024-06-25T05:01:31Z

@blueorangutan package

blueorangutan · 2024-06-25T05:02:03Z

@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.

sureshanaparti · 2024-06-25T05:15:43Z

Hi @JoaoJandre Can you review/check if your comments are addressed. It seems the config is applied for vm and volume stats now, this can be extended to other stats/tables later if required without any new config.

blueorangutan · 2024-06-25T06:04:21Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10108

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

…gerImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

…gerImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

sureshanaparti · 2024-06-27T10:55:35Z

@blueorangutan package

blueorangutan · 2024-06-27T10:56:03Z

@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.

blueorangutan · 2024-06-27T11:55:57Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10162

sureshanaparti · 2024-06-27T12:16:22Z

@blueorangutan test

blueorangutan · 2024-06-27T12:18:04Z

@sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2024-06-27T12:43:59Z

[SF] Trillian Build Failed (tid-10657)

JoaoJandre

LGTM, but I think we should change the name to delete.query.batch.size instead of delete.batch.query.size. I did not test it.

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

vishesh92 · 2024-06-27T14:00:27Z

@blueorangutan package

blueorangutan · 2024-06-27T14:02:06Z

@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.

blueorangutan · 2024-06-27T15:04:22Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10172

sureshanaparti · 2024-06-27T16:16:29Z

@blueorangutan test

blueorangutan · 2024-06-27T16:18:03Z

@sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2024-06-28T02:27:29Z

[SF] Trillian test result (tid-10664)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35009 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9283-t10664-kvm-centos7.zip
Smoke tests completed. 97 look OK, 34 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestTemplateHierarchy>:setup	`Error`	8.40	test_accounts.py
test_01_events_resource	`Failure`	0.02	test_events_resource.py
ContextSuite context=TestDeployVmWithAffinityGroup>:setup	`Error`	0.00	test_affinity_groups.py
ContextSuite context=TestMultipleVolumeAttach>:setup	`Error`	0.00	test_attach_multiple_volumes.py
ContextSuite context=TestDummyBackupAndRecovery>:setup	`Error`	0.00	test_backup_recovery_dummy.py
ContextSuite context=TestVeeamBackupAndRecovery>:setup	`Error`	0.00	test_backup_recovery_veeam.py
test_01_condensed_drs_algorithm	`Error`	0.00	test_cluster_drs.py
test_02_balanced_drs_algorithm	`Error`	0.00	test_cluster_drs.py
ContextSuite context=TestConsoleEndpoint>:setup	`Error`	0.00	test_console_endpoint.py
test_01_deploy_vm_with_extraconfig_throws_exception_kvm	`Error`	0.09	test_deploy_vm_extra_config_data.py
test_02_deploy_vm_with_extraconfig_kvm	`Error`	0.07	test_deploy_vm_extra_config_data.py
test_03_update_vm_with_extraconfig_kvm	`Error`	0.07	test_deploy_vm_extra_config_data.py
ContextSuite context=TestDeployVmRootSize>:setup	`Error`	0.00	test_deploy_vm_root_resize.py
ContextSuite context=TestHostControlState>:setup	`Error`	27.26	test_host_control_state.py
ContextSuite context=TestImportAndUnmanageVolumes>:setup	`Error`	0.00	test_import_unmanage_volumes.py
test_11_test_unmanaged_cluster_lifecycle	`Error`	3.26	test_kubernetes_clusters.py
ContextSuite context=TestListVolumes>:setup	`Error`	0.00	test_list_volumes.py
ContextSuite context=TestNetworkMigration>:setup	`Error`	0.00	test_migration.py
test_03_network_operations_on_created_vm_of_otheruser	`Error`	1.22	test_network_permissions.py
test_04_deploy_vm_for_other_user_and_test_vm_operations	`Failure`	0.05	test_network_permissions.py
ContextSuite context=TestSharedNetwork>:setup	`Error`	46.45	test_network.py
test_01_nic	`Error`	0.06	test_nic.py
test_01_non_strict_host_anti_affinity	`Error`	2.25	test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity	`Error`	1.16	test_nonstrict_affinity_group.py
ContextSuite context=TestL2PersistentNetworks>:setup	`Error`	0.00	test_persistent_network.py
test_01_add_primary_storage_disabled_host	`Failure`	0.06	test_primary_storage.py
test_01_primary_storage_iscsi	`Failure`	0.06	test_primary_storage.py
test_01_primary_storage_nfs	`Failure`	0.06	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.13	test_primary_storage.py
ContextSuite context=TestPrivateGwACLOvsGRE>:setup	`Error`	0.00	test_privategw_acl_ovs_gre.py
ContextSuite context=TestTemplates>:setup	`Error`	34.65	test_templates.py
test_01_positive_tests_usage	`Failure`	3.07	test_usage_events.py
ContextSuite context=TestLBRuleUsage>:setup	`Error`	2.27	test_usage.py
ContextSuite context=TestNatRuleUsage>:setup	`Error`	2.33	test_usage.py
ContextSuite context=TestPublicIPUsage>:setup	`Error`	2.39	test_usage.py
ContextSuite context=TestSnapshotUsage>:setup	`Error`	2.45	test_usage.py
ContextSuite context=TestTemplateUsage>:setup	`Error`	2.51	test_usage.py
ContextSuite context=TestVmUsage>:setup	`Error`	2.56	test_usage.py
ContextSuite context=TestVolumeUsage>:setup	`Error`	2.67	test_usage.py
ContextSuite context=TestVpnUsage>:setup	`Error`	2.73	test_usage.py
ContextSuite context=TestVmAutoScaling>:setup	`Error`	0.00	test_vm_autoscaling.py
test_01_deploy_vm_on_specific_host	`Error`	0.04	test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster	`Error`	0.03	test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod	`Error`	0.07	test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster	`Error`	0.09	test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod	`Error`	0.06	test_vm_deployment_planner.py
ContextSuite context=TestDeployVM>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestKVMLiveMigration>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestMigrateVMwithVolume>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestSecuredVmMigration>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=TestVMLifeCycle>:setup	`Error`	0.06	test_vm_life_cycle.py
ContextSuite context=TestUnmanageVM>:setup	`Error`	0.00	test_vm_lifecycle_unmanage_import.py
ContextSuite context=TestVMSchedule>:setup	`Error`	0.00	test_vm_schedule.py
ContextSuite context=TestVmSnapshot>:setup	`Error`	0.06	test_vm_snapshots.py
ContextSuite context=TestVnfTemplates>:setup	`Error`	0.00	test_vnf_templates.py
ContextSuite context=TestCreateVolume>:setup	`Error`	0.00	test_volumes.py
ContextSuite context=TestVolumeEncryption>:setup	`Error`	0.00	test_volumes.py
ContextSuite context=TestVolumes>:setup	`Error`	0.00	test_volumes.py
test_02_cancel_host_maintenace_with_migration_jobs	`Failure`	0.12	test_host_maintenance.py
test_03_cancel_host_maintenace_with_migration_jobs_failure	`Error`	0.20	test_host_maintenance.py
ContextSuite context=TestHostMaintenanceAgents>:setup	`Error`	0.28	test_host_maintenance.py

sureshanaparti · 2024-06-28T04:59:58Z

@blueorangutan LLtest

blueorangutan · 2024-06-28T05:02:02Z

@sureshanaparti a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

sureshanaparti · 2024-06-28T10:02:00Z

[SF] Trillian test result (tid-10664) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 35009 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9283-t10664-kvm-centos7.zip Smoke tests completed. 97 look OK, 34 have errors, 0 did not run

these failures are due to templates listing issue & builtin template deleted during the tests.

sureshanaparti · 2024-06-28T10:02:27Z

Manually verified the config change.

…delete of volume_stats in batches (apache#9283) * Change vm.stats.remove.batch.size to delete.batch.query.size * Add support for deletion of volume stats in batches * Update server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com> * Update server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com> * Update configkey description * Address comments Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

boring-cyborg bot added the component:management-server label Jun 21, 2024

vishesh92 added this to the 4.19.1.0 milestone Jun 21, 2024

vishesh92 requested review from JoaoJandre and sureshanaparti June 21, 2024 19:52

sureshanaparti reviewed Jun 23, 2024

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Show resolved Hide resolved

yadvr approved these changes Jun 24, 2024

View reviewed changes

shwstppr approved these changes Jun 24, 2024

View reviewed changes

Change vm.stats.remove.batch.size to delete.batch.query.size

e562cb0

vishesh92 force-pushed the make-vm-stat-remove-config-generic branch from d42560b to e562cb0 Compare June 24, 2024 10:09

sureshanaparti approved these changes Jun 24, 2024

View reviewed changes

boring-cyborg bot added the component:storage label Jun 24, 2024

vishesh92 force-pushed the make-vm-stat-remove-config-generic branch from 22060c3 to 96c3a08 Compare June 24, 2024 12:57

Add support for deletion of volume stats in batches

dea2467

vishesh92 force-pushed the make-vm-stat-remove-config-generic branch from 96c3a08 to dea2467 Compare June 24, 2024 13:06

vishesh92 changed the title ~~Change vm.stats.remove.batch.size to delete.batch.query.size~~ Change vm.stats.remove.batch.size to delete.batch.query.size & allow delete of volume_stats in batches Jun 24, 2024

vishesh92 closed this Jun 25, 2024

vishesh92 reopened this Jun 25, 2024

apache deleted a comment from blueorangutan Jun 27, 2024

apache deleted a comment from vishesh92 Jun 27, 2024

sureshanaparti reviewed Jun 27, 2024

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated Show resolved Hide resolved

Update server/src/main/java/com/cloud/configuration/ConfigurationMana…

8b39a51

…gerImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

sureshanaparti reviewed Jun 27, 2024

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated Show resolved Hide resolved

Update server/src/main/java/com/cloud/configuration/ConfigurationMana…

9d89f17

…gerImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

sureshanaparti reviewed Jun 27, 2024

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated Show resolved Hide resolved

Update configkey description

beea194

apache deleted a comment from blueorangutan Jun 27, 2024

JoaoJandre approved these changes Jun 27, 2024

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Outdated Show resolved Hide resolved

Address comments

745b212

sureshanaparti merged commit a4e9d7f into apache:4.19 Jun 28, 2024

vishesh92 deleted the make-vm-stat-remove-config-generic branch June 28, 2024 10:02

Change vm.stats.remove.batch.size to delete.batch.query.size & allow delete of volume_stats in batches #9283

Change vm.stats.remove.batch.size to delete.batch.query.size & allow delete of volume_stats in batches #9283

Uh oh!

Conversation

vishesh92 commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

vishesh92 commented Jun 21, 2024

Uh oh!

blueorangutan commented Jun 21, 2024

Uh oh!

blueorangutan commented Jun 21, 2024

Uh oh!

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vishesh92 commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

JoaoJandre commented Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vishesh92 commented Jun 24, 2024

Uh oh!

vishesh92 commented Jun 25, 2024

Uh oh!

blueorangutan commented Jun 25, 2024

Uh oh!

sureshanaparti commented Jun 25, 2024

Uh oh!

blueorangutan commented Jun 25, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sureshanaparti commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

sureshanaparti commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

JoaoJandre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vishesh92 commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

sureshanaparti commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 27, 2024

Uh oh!

blueorangutan commented Jun 28, 2024

Uh oh!

sureshanaparti commented Jun 28, 2024

vishesh92 commented Jun 21, 2024 •

edited

Loading

codecov bot commented Jun 24, 2024 •

edited

Loading

JoaoJandre commented Jun 24, 2024 •

edited

Loading