Skip to content

Use join instead of views - #8321

Merged
weizhouapache merged 30 commits into
apache:4.19from
shapeblue:use-join-instead-of-views
Mar 18, 2024
Merged

Use join instead of views#8321
weizhouapache merged 30 commits into
apache:4.19from
shapeblue:use-join-instead-of-views

Conversation

@vishesh92

@vishesh92vishesh92 commented Dec 6, 2023

Copy link
Copy Markdown
Member

Description

To be merged after #8410 to ensure this doesn't break anything.

This PR contains changes similar to #8012 which is to directly query the tables and make required joins only when there is a requirement to filter on some other field. This will help reduce the query times and thus reduce load on the database as well.

ResourceCountBeforeAfterDifference
volumes74850473
networks1911401319
events7515613130
accounts52100946
domains5135323
hosts101306163143
storage pools10041401
service offerings603534112
disk offerings705483711
templates60315510946

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

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

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

@codecov

codecovBot commented Dec 6, 2023

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 68.29522% with 305 lines in your changes are missing coverage. Please review.

Project coverage is 30.85%. Comparing base (0208e09) to head (61f1382).
Report is 5 commits behind head on 4.19.

FilesPatch %Lines
...ain/java/com/cloud/api/query/QueryManagerImpl.java70.45%138 Missing and 26 partials ⚠️
...c/main/java/com/cloud/utils/db/GenericDaoBase.java70.66%16 Missing and 6 partials ⚠️
...b/src/main/java/com/cloud/utils/db/SearchBase.java47.36%9 Missing and 11 partials ⚠️
...va/com/cloud/api/query/dao/AccountJoinDaoImpl.java53.12%11 Missing and 4 partials ⚠️
...m/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java53.12%11 Missing and 4 partials ⚠️
...ava/com/cloud/api/query/dao/DomainJoinDaoImpl.java53.12%11 Missing and 4 partials ⚠️
...loud/api/query/dao/ServiceOfferingJoinDaoImpl.java53.12%11 Missing and 4 partials ⚠️
.../src/main/java/com/cloud/utils/db/JoinBuilder.java51.72%14 Missing ⚠️
...ava/org/apache/cloudstack/acl/RoleManagerImpl.java56.25%4 Missing and 3 partials ⚠️
.../storage/datastore/db/PrimaryDataStoreDaoImpl.java92.98%2 Missing and 2 partials ⚠️
... and 8 more
Additional details and impacted files
@@ Coverage Diff @@## 4.19 #8321 +/- ##
============================================
- Coverage 30.91% 30.85% -0.07% + Complexity 34249 34183 -66 
============================================
Files 5354 5355 +1 Lines 376094 376619 +525 Branches 54696 54809 +113 ============================================
- Hits 116258 116194 -64 - Misses 244542 245131 +589 
Partials 15294 15294 
FlagCoverage Δ
simulator-marvin-tests24.64% <68.19%> (-0.09%)⬇️
uitests4.39% <ø> (ø)
unit-tests16.56% <5.19%> (-0.03%)⬇️

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.

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

Comment threadserver/src/main/java/com/cloud/api/query/QueryManagerImpl.java Outdated
@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@vishesh92
vishesh92 marked this pull request as ready for review December 15, 2023 09:51
Comment threadengine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java Outdated
@vishesh92

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@vishesh92

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@harikrishna-patnala

Copy link
Copy Markdown
Member

@blueorangutan test matrix

@blueorangutan

Copy link
Copy Markdown

@harikrishna-patnala a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-8765)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44377 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8321-t8765-xenserver-71.zip
Smoke tests completed. 119 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File
test_04_change_offering_smallError100.44test_service_offerings.py
test_06_disk_offering_strictness_falseError148.42test_service_offerings.py
test_change_service_offering_for_vm_with_snapshotsError98.27test_vm_snapshots.py
test_01_create_vm_snapshotsError90.24test_vm_snapshots.py
test_02_revert_vm_snapshotsFailure90.12test_vm_snapshots.py
test_03_delete_vm_snapshotsFailure0.01test_vm_snapshots.py

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-8766)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 49277 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8321-t8766-vmware-67u3.zip
Smoke tests completed. 115 look OK, 6 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File
test_01_snapshot_root_diskError0.07test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:setupError213.28test_snapshots.py
test_01_snapshot_usageError0.07test_usage.py
test_02_balanced_drs_algorithmFailure130.02test_cluster_drs.py
ContextSuite context=TestListIdsParams>:setupError0.00test_list_ids_parameter.py
test_04_change_offering_smallError97.46test_service_offerings.py
test_06_disk_offering_strictness_falseError150.06test_service_offerings.py
test_change_service_offering_for_vm_with_snapshotsError144.69test_vm_snapshots.py
test_01_create_vm_snapshotsError123.73test_vm_snapshots.py
test_02_revert_vm_snapshotsFailure90.22test_vm_snapshots.py
test_03_delete_vm_snapshotsFailure0.02test_vm_snapshots.py

@vishesh92

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@vishesh92

Copy link
Copy Markdown
MemberAuthor

@blueorangutan test matrix

@blueorangutan

Copy link
Copy Markdown

@vishesh92 a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-8788)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44397 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8321-t8788-xenserver-71.zip
Smoke tests completed. 118 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File
test_02_trigger_shutdownFailure341.81test_safe_shutdown.py
test_04_change_offering_smallError100.42test_service_offerings.py
test_06_disk_offering_strictness_falseError158.00test_service_offerings.py
test_change_service_offering_for_vm_with_snapshotsError211.21test_vm_snapshots.py

@blueorangutan

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@vishesh92vishesh92 mentioned this pull request Feb 21, 2024
13 tasks
@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan package

@weizhouapacheweizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

@yadvr

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

Copy link
Copy Markdown

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

@vishesh92

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@yadvr

Copy link
Copy Markdown
Member

@blueorangutan test matrix

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-9482)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40129 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8321-t9482-xenserver-71.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-9483)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 45959 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8321-t9483-vmware-67u3.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File

@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_02_trigger_shutdownFailure336.69test_safe_shutdown.py

@vladimirpetrov

Copy link
Copy Markdown
Contributor

Tested on a simulator hypervisor, with a separate MS (4 CPUs, 8 GB RAM) and DB VM (8 CPUs, 16 GB RAM), using Apache Benchmark tool (ab), measuring mean response time from 100 executions per list operation.

Number of resources:
Hosts: 1001
Storage pools: 100
Volumes: 25110
Events: 155118
Instances: 8370

Results, mean time in milliseconds

list hosts

Page=20Page=50Page=100
Unpatched3046761052
Patched2596471246
Difference-15%-4%18%

list storage pools

Page=20Page=50Page=100
Unpatched204487972
Patched193381843
Difference-5%-22%-13%

list volumes

Page=20Page=50Page=100
Unpatched6638781205
Patched389558961
Difference-41%-36%-20%

list volume metrics

Page=20Page=50Page=100
Unpatched69910571329
Patched3747321299
Difference-46%-31%-2%

list events

Page=20Page=50Page=100
Unpatched260927502748
Patched396522457
Difference-85%-81%-83%

list instance metrics

Page=20Page=50Page=100
Unpatched91418043273
Patched89816503255
Difference-2%-9%-1%

@vladimirpetrovvladimirpetrov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on performance measurements with and without the patch

@weizhouapache

Copy link
Copy Markdown
Member

Merging based on approvals and CI/manual test results
Thanks @vishesh92@vladimirpetrov

@weizhouapache
weizhouapache merged commit 0043540 into apache:4.19Mar 18, 2024
@vishesh92
vishesh92 deleted the use-join-instead-of-views branch March 19, 2024 07:36
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Mar 21, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants

@vishesh92@blueorangutan@harikrishna-patnala@yadvr@DaanHoogland@weizhouapache@vladimirpetrov