Fix query execution in countVgpuVMs#12713
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the execution order of statements in countVgpuVMs to ensure the legacy query is executed and processed before the main query, aligning with the intended behavior in the linked issue.
Changes:
- Reordered prepared statement creation/execution so the legacy query runs first.
- Moved parameter binding for the main query to occur after legacy results are processed.
Comments suppressed due to low confidence (1)
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java:864
- The
ResultSetfrom the legacy query is reassigned (rs = pstmt.executeQuery()) without being closed first, which can leak DB/cursor resources depending on the JDBC driver. Use separateResultSetvariables and/or wrap eachexecuteQuery()in its own try-with-resources block so the legacyResultSetis closed before executing the main query.
ResultSet rs = pstmtLegacy.executeQuery();
while (rs.next()) {
result.put(rs.getString(1).concat(rs.getString(2)), rs.getLong(3));
}
pstmt = txn.prepareAutoCloseStatement(finalQuery.toString());
for (int i = 0; i < resourceIdList.size(); i++) {
pstmt.setLong(1 + i, resourceIdList.get(i));
}
rs = pstmt.executeQuery();
while (rs.next()) {
result.put(rs.getString(1).concat(rs.getString(2)), rs.getLong(3));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12713 +/- ##
============================================
+ Coverage 17.61% 17.92% +0.31%
- Complexity 15662 16155 +493
============================================
Files 5917 5939 +22
Lines 531360 533148 +1788
Branches 64969 65238 +269
============================================
+ Hits 93584 95573 +1989
+ Misses 427224 426834 -390
- Partials 10552 10741 +189
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:
|
|
@blueorangutan package |
|
@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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16960 |
rajujith
left a comment
There was a problem hiding this comment.
LGTM.
Reproduced the issue on 4.22.0 with a standalone MariDB 10.11 with db.cloud.uri=jdbc:mariadb:sequential.
Upgraded to this PR build. The issue is resolved.
Description
This PR fixes #12712
Generated Summary
This pull request makes a minor adjustment to the order of prepared statement execution in the
countVgpuVMsmethod ofVMInstanceDaoImpl.java. The change ensures that the legacy query is executed and processed before preparing and executing the main query, which improves clarity and may help prevent resource conflicts.countVgpuVMsso that the legacy query is prepared, parameters set, and results processed before preparing and executing the main query, potentially improving resource management and code clarity.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?