Skip to content

Conversation

@shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Aug 13, 2024

Description

Allows configuring connection pool library for database connection. As default, replaces dbcp2 connection pool library with more performant HikariCP.

db.<DATABASE>.connectionPoolLib property can be set in the db.properties to use the desired library.
Set dbcp for using DBCP2
Set hikaricp or for using HikariCP

Per docs, if the mysql connector is JDBC2 compliant then it should use the Connection.isValid API to test a connection.
(https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#isValid-int-)

This would significantly reduce query lags and API throughput, as for every SQL query one or two SELECT 1 are performed every time a Connection is given to application logic.

This should only be accepted when the driver is JDBC4 complaint.

As per the docs, the connector-j can use /* ping */ before calling SELECT 1 to have light weight application pings to the server:
https://dev.mysql.com/doc/connector-j/en/connector-j-usagenotes-j2ee-concepts-connection-pooling.html

Related doc PR: apache/cloudstack-documentation#431

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

  • 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?

Per docs, if the mysql connector is JDBC2 compliant then it should use
the Connection.isValid API to test a connection.
(https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#isValid-int-)

This would significantly reduce query lags and API throughput, as for
every SQL query one or two SELECT 1 are performed everytime a Connection
is given to application logic.

This should only be accepted when the driver is JDBC4 complaint.

Signed-off-by: Rohit Yadav <[email protected]>
As per the docs, the connector-j can use /* ping */ before calling
SELECT 1 to have light weight application pings to the server:
https://dev.mysql.com/doc/connector-j/en/connector-j-usagenotes-j2ee-concepts-connection-pooling.html

Signed-off-by: Rohit Yadav <[email protected]>
Replaces dbcp2 connection pool library with more performant HikariCP.
With this unit tests are failing but build is passing.

Signed-off-by: Rohit Yadav <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

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

@codecov
Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 53.50877% with 53 lines in your changes missing coverage. Please review.

Project coverage is 15.57%. Comparing base (d7ca05e) to head (a2d8f96).
Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
...ain/java/com/cloud/utils/db/TransactionLegacy.java 55.45% 45 Missing and 4 partials ⚠️
...c/main/java/com/cloud/upgrade/DatabaseCreator.java 0.00% 3 Missing ⚠️
...n/java/com/cloud/utils/db/ConnectionConcierge.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9518      +/-   ##
============================================
- Coverage     15.57%   15.57%   -0.01%     
- Complexity    12037    12055      +18     
============================================
  Files          5501     5506       +5     
  Lines        482219   482880     +661     
  Branches      62044    59617    -2427     
============================================
+ Hits          75125    75217      +92     
- Misses       398791   399351     +560     
- Partials       8303     8312       +9     
Flag Coverage Δ
uitests 4.17% <ø> (+<0.01%) ⬆️
unittests 16.35% <53.50%> (-0.01%) ⬇️

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.

@blueorangutan
Copy link

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

@rohityadavcloud rohityadavcloud added this to the 4.20.0.0 milestone Aug 21, 2024
@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@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

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

@shwstppr
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File
all_test_vm_strict_host_tags Skipped --- test_vm_strict_host_tags.py
all_test_vnf_templates Skipped --- test_vnf_templates.py
all_test_volumes Skipped --- test_volumes.py
all_test_vpc_ipv6 Skipped --- test_vpc_ipv6.py
all_test_vpc_redundant Skipped --- test_vpc_redundant.py
all_test_vpc_router_nics Skipped --- test_vpc_router_nics.py
all_test_vpc_vpn Skipped --- test_vpc_vpn.py
all_test_webhook_delivery Skipped --- test_webhook_delivery.py
all_test_webhook_lifecycle Skipped --- test_webhook_lifecycle.py
all_test_host_maintenance Skipped --- test_host_maintenance.py
all_test_hostha_kvm Skipped --- test_hostha_kvm.py

@github-actions
Copy link

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

@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

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

@shwstppr
Copy link
Contributor Author

Created a documentation PR: apache/cloudstack-documentation#431

@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_03_ping_in_ssvm_success Failure 15.53 test_diagnostics.py
test_05_ping_in_cpvm_success Failure 15.53 test_diagnostics.py

@blueorangutan
Copy link

[SF] Trillian test result (tid-11252)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 59468 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9518-t11252-vmware-70u3.zip
Smoke tests completed. 137 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_deployVMInSharedNetwork Error 132.29 test_network.py
test_02_restore_vm_with_disk_offering Error 58.09 test_restore_vm.py
test_03_restore_vm_with_disk_offering_custom_size Error 61.17 test_restore_vm.py

@blueorangutan
Copy link

[SF] Trillian test result (tid-11250)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 59833 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9518-t11250-kvm-ol8.zip
Smoke tests completed. 136 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestClusterDRS>:setup Error 0.00 test_cluster_drs.py
test_01_secure_vm_migration Error 378.52 test_vm_life_cycle.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL Failure 406.40 test_vpc_redundant.py

@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 1, 2024

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

[SF] Trillian test result (tid-11276)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 62482 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9518-t11276-kvm-ol8.zip
Smoke tests completed. 134 look OK, 1 have errors, 4 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestRVPCSite2SiteVpn>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:setup Error 0.00 test_vpc_vpn.py
all_test_webhook_delivery Skipped --- test_webhook_delivery.py
all_test_webhook_lifecycle Skipped --- test_webhook_lifecycle.py
all_test_host_maintenance Skipped --- test_host_maintenance.py
all_test_hostha_kvm Skipped --- test_hostha_kvm.py

@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 3, 2024

@blueorangutan package

@blueorangutan
Copy link

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

Copy link
Member

@rohityadavcloud rohityadavcloud left a comment

Choose a reason for hiding this comment

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

LGTM, left a minor comment.

@blueorangutan
Copy link

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

@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 3, 2024

@blueorangutan test

@blueorangutan
Copy link

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

@rohityadavcloud rohityadavcloud marked this pull request as ready for review September 4, 2024 06:52
@rohityadavcloud
Copy link
Member

@blueorangutan test ol8 vmware-70u3

@blueorangutan
Copy link

@rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-70u3) has been kicked to run smoke tests

@weizhouapache
Copy link
Member

@shwstppr
anything to do for upgrade ?

does hikaricp support most popular databases ?

@shwstppr
Copy link
Contributor Author

shwstppr commented Sep 4, 2024

@shwstppr anything to do for upgrade ?

No, I was able touse either library just by changing db.properties. But will still do some upgrade tests to rule out any regressions

does hikaricp support most popular databases ?

Yes, https://github.com/brettwooldridge/HikariCP?tab=readme-ov-file#popular-datasource-class-names

Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

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

thanks @shwstppr
code lgtm

@blueorangutan
Copy link

[SF] Trillian test result (tid-11316)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 64859 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9518-t11316-kvm-ol8.zip
Smoke tests completed. 133 look OK, 2 have errors, 4 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_04_nonsecured_to_secured_vm_migration Error 376.97 test_vm_life_cycle.py
test_01_vpc_remote_access_vpn Error 309.28 test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:teardown Error 1065.21 test_vpc_vpn.py
test_01_vpc_site2site_vpn Failure 1621.20 test_vpc_vpn.py
test_01_vpc_site2site_vpn Error 1621.21 test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:teardown Error 1759.09 test_vpc_vpn.py
all_test_webhook_delivery Skipped --- test_webhook_delivery.py
all_test_webhook_lifecycle Skipped --- test_webhook_lifecycle.py
all_test_host_maintenance Skipped --- test_host_maintenance.py
all_test_hostha_kvm Skipped --- test_hostha_kvm.py

@apache apache deleted a comment from blueorangutan Sep 5, 2024
@rohityadavcloud
Copy link
Member

LGTM, considering this and previous smoketests - intermittent failures observed, not likely from this PR. Merging based on this & the review lgtms.

@rohityadavcloud rohityadavcloud merged commit 7e085d5 into apache:main Sep 5, 2024
@rohityadavcloud rohityadavcloud deleted the add-hikaricp branch September 5, 2024 04:07
DaanHoogland pushed a commit to apache/cloudstack-documentation that referenced this pull request Sep 5, 2024
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 6, 2024
Per docs, if the mysql connector is JDBC2 compliant then it should use
the Connection.isValid API to test a connection.
(https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#isValid-int-)

This would significantly reduce query lags and API throughput, as for
every SQL query one or two SELECT 1 are performed everytime a Connection
is given to application logic.

This should only be accepted when the driver is JDBC4 complaint.

As per the docs, the connector-j can use /* ping */ before calling
SELECT 1 to have light weight application pings to the server:
https://dev.mysql.com/doc/connector-j/en/connector-j-usagenotes-j2ee-concepts-connection-pooling.html

Replaces dbcp2 connection pool library with more performant HikariCP.
With this unit tests are failing but build is passing.

Signed-off-by: Rohit Yadav <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
Co-authored-by: Rohit Yadav <[email protected]>
yildirimomer12 pushed a commit to yildirimomer12/cloudstack that referenced this pull request Jun 12, 2025
Allows specifying connection pooling library. Default is HikariCP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants