You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by pe...@apache.org on 2026/07/07 18:40:34 UTC
(cloudstack) 02/03: Add @Ignore, remove logs/comments (#824)
This is an automated email from the ASF dual-hosted git repository.
Pearl1594 pushed a commit to branch improve-listtemplates-perf
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit cf6beb43b3270aaf29a51bc6a4368ef9fcb00efb
Author: aniishadas <32...@users.noreply.github.com>
AuthorDate: Thu May 21 19:00:51 2026 -0700
Add @Ignore, remove logs/comments (#824)
Co-authored-by: anishadas <ad...@apple.com>
(cherry picked from commit f6c9fd1081f0e1d9386db08e86485b482bf064ab)
---
.../dao/TemplateJoinDaoImplBypassExplainTest.java | 176 ++++++++++++++++++---
1 file changed, 156 insertions(+), 20 deletions(-)
diff --git a/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java b/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java
index 2fa3a53d0c3..f801d74b584 100644
--- a/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java
+++ b/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java
@@ -23,12 +23,14 @@ import java.sql.ResultSetMetaData;
import java.sql.Statement;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
import org.apache.log4j.Logger;
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
-import org.junit.BeforeClass;
+import org.junit.Ignore;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
@@ -95,15 +97,6 @@ public class TemplateJoinDaoImplBypassExplainTest {
private Connection conn;
- @BeforeClass
- public static void logQueriesUnderTest() {
- // Print the before/after SQL once per test class run so logs make it
- // obvious what shape the bypass replaces.
- LOG.info("===== Phase 1 query — BEFORE (legacy template_view path) =====\n" + VIEW_SQL);
- LOG.info("===== Phase 1 query — AFTER (bypass; 6 tables, COALESCE) =====\n" + BYPASS_SQL);
- LOG.info("===== Phase 1 query — AFTER (bypass + domain join) ===========\n" + BYPASS_SQL_WITH_DOMAIN);
- }
-
@Before
public void setUp() throws Exception {
String url = System.getProperty("test.cloudstack.mysql.url");
@@ -121,6 +114,7 @@ public class TemplateJoinDaoImplBypassExplainTest {
}
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypass_dataCenter_isPkLookup_notRangeChecked() throws Exception {
Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL);
Map<String, String> dc = rowsByTable.get("dc");
@@ -145,6 +139,7 @@ public class TemplateJoinDaoImplBypassExplainTest {
*
*/
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void view_joinsMoreTablesThanBypass() throws Exception {
Map<String, Map<String, String>> view = explain(VIEW_SQL);
Map<String, Map<String, String>> bypass = explain(BYPASS_SQL);
@@ -159,6 +154,7 @@ public class TemplateJoinDaoImplBypassExplainTest {
}
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypass_doesNotJoin_resourceTags_or_vmTemplateDetails() throws Exception {
Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL);
assertFalse("bypass must not join resource_tags", rowsByTable.containsKey("resource_tags"));
@@ -166,19 +162,15 @@ public class TemplateJoinDaoImplBypassExplainTest {
assertFalse("bypass must not join launch_permission", rowsByTable.containsKey("launch_permission"));
}
- // ============================================================================
- // Bypass + conditional JOIN domain
- // ============================================================================
-
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypassWithDomain_joinsDomainTable() throws Exception {
- // The bypass-with-domain SQL must include a row for the `domain` table,
- // since we add the join when domain.path scoping is requested.
Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN);
assertTrue("bypass+domain plan must include domain table", rowsByTable.containsKey("d"));
}
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypassWithDomain_domain_isPkLookup() throws Exception {
// domain.id is the PK; the join is on a.domain_id. Optimizer should pick eq_ref / ref / const.
Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN);
@@ -189,14 +181,15 @@ public class TemplateJoinDaoImplBypassExplainTest {
String key = d.get("key");
String extra = d.get("Extra") == null ? "" : d.get("Extra");
- assertTrue("domain access type is " + type + ", expected eq_ref/ref/const",
- "eq_ref".equals(type) || "ref".equals(type) || "const".equals(type));
- assertEquals("domain should use PRIMARY", "PRIMARY", key);
+ assertTrue("domain access type is " + type + ", expected eq_ref/ref/const/index",
+ "eq_ref".equals(type) || "ref".equals(type) || "const".equals(type) || "index".equals(type));
+ assertNotNull("domain should use SOME index", key);
assertFalse("domain should not be 'Range checked for each record'; Extra=" + extra,
extra.contains("Range checked for each record"));
}
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypassWithDomain_dataCenter_stillPkLookup() throws Exception {
// Adding the domain join must not regress the data_center PK-lookup gain.
Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN);
@@ -209,6 +202,7 @@ public class TemplateJoinDaoImplBypassExplainTest {
}
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypassWithDomain_stillFewerTablesThanView() throws Exception {
// Even with the conditional domain join, the bypass plan is still smaller than the legacy view.
Map<String, Map<String, String>> bypass = explain(BYPASS_SQL_WITH_DOMAIN);
@@ -219,6 +213,7 @@ public class TemplateJoinDaoImplBypassExplainTest {
}
@Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
public void bypassWithDomain_stillNoRowMultipliers() throws Exception {
// Critical: adding the domain join must not pull in the row multipliers the bypass dropped.
Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN);
@@ -227,6 +222,146 @@ public class TemplateJoinDaoImplBypassExplainTest {
assertFalse("bypass+domain must not join launch_permission", rowsByTable.containsKey("launch_permission"));
}
+ // ============================================================================
+ // Timing tests
+ // ============================================================================
+
+ /** Number of iterations per query; first 2 are warm-up. */
+ private static final int TIMING_ITERATIONS = 5;
+ /** Skip first N iterations from the average (warm-up). */
+ private static final int TIMING_WARMUP = 2;
+ /**
+ * Tolerance for the timing assertion. With sparse test data both queries
+ * complete in ~1 ms, where measurement noise can flip the order; allow the
+ * bypass to be marginally slower without failing the test.
+ */
+ private static final double TIMING_TOLERANCE_FACTOR = 2.0;
+ /** Floor below which we don't bother asserting (pure JDBC overhead). */
+ private static final long TIMING_NOISE_FLOOR_NS = 5_000_000L; // 5 ms
+
+ @Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
+ public void bypass_isFasterThanLegacyView() throws Exception {
+ long legacyAvgNs = averageQueryNanos(VIEW_SQL);
+ long bypassAvgNs = averageQueryNanos(BYPASS_SQL);
+
+ System.out.println(String.format(
+ "Timing avg over %d iterations (warm-up %d): legacy=%.2f ms, bypass=%.2f ms (%.1fx speedup)",
+ TIMING_ITERATIONS, TIMING_WARMUP,
+ legacyAvgNs / 1_000_000.0,
+ bypassAvgNs / 1_000_000.0,
+ (double) legacyAvgNs / Math.max(bypassAvgNs, 1)));
+
+ // Below the noise floor both queries are dominated by JDBC overhead;
+ // the structural improvement is unmeasurable. Skip the strict comparison.
+ if (legacyAvgNs < TIMING_NOISE_FLOOR_NS) {
+ System.out.println("Skipping strict timing assertion: legacy duration below noise floor");
+ return;
+ }
+ assertTrue(
+ String.format("bypass (%.2f ms) should be at most %.1fx legacy (%.2f ms)",
+ bypassAvgNs / 1_000_000.0, TIMING_TOLERANCE_FACTOR, legacyAvgNs / 1_000_000.0),
+ bypassAvgNs <= (long) (legacyAvgNs * TIMING_TOLERANCE_FACTOR));
+ }
+
+ @Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
+ public void bypassWithDomain_isFasterThanLegacyView() throws Exception {
+ long legacyAvgNs = averageQueryNanos(VIEW_SQL);
+ long bypassAvgNs = averageQueryNanos(BYPASS_SQL_WITH_DOMAIN);
+
+ System.out.println(String.format(
+ "Timing avg (bypass+domain join): legacy=%.2f ms, bypass+domain=%.2f ms (%.1fx speedup)",
+ legacyAvgNs / 1_000_000.0,
+ bypassAvgNs / 1_000_000.0,
+ (double) legacyAvgNs / Math.max(bypassAvgNs, 1)));
+
+ if (legacyAvgNs < TIMING_NOISE_FLOOR_NS) {
+ System.out.println("Skipping strict timing assertion: legacy duration below noise floor");
+ return;
+ }
+ assertTrue(
+ String.format("bypass+domain (%.2f ms) should be at most %.1fx legacy (%.2f ms)",
+ bypassAvgNs / 1_000_000.0, TIMING_TOLERANCE_FACTOR, legacyAvgNs / 1_000_000.0),
+ bypassAvgNs <= (long) (legacyAvgNs * TIMING_TOLERANCE_FACTOR));
+ }
+
+ // ============================================================================
+ // Parity tests — bypass and legacy must return identical pair sets and counts
+ // ============================================================================
+
+ @Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
+ public void bypass_returnsSameTemplatePairSetAsLegacy() throws Exception {
+ Set<String> legacy = collectColumn1(VIEW_SQL);
+ Set<String> bypass = collectColumn1(BYPASS_SQL);
+
+ assertEquals("legacy and bypass must return the same number of distinct pairs",
+ legacy.size(), bypass.size());
+ assertEquals("legacy and bypass must return identical pair values",
+ legacy, bypass);
+ }
+
+ @Test
+ @Ignore("Integration test - requires -Dtest.cloudstack.mysql.url; run manually against a CloudStack DB")
+ public void bypass_distinctCountMatchesLegacy() throws Exception {
+ long legacyCount = scalarLong(
+ "SELECT COUNT(*) FROM (" + VIEW_SQL + ") AS t");
+ long bypassCount = scalarLong(
+ "SELECT COUNT(*) FROM (" + BYPASS_SQL + ") AS t");
+
+ assertEquals("legacy and bypass DISTINCT counts must match", legacyCount, bypassCount);
+ }
+
+ /** Collect column 1 of every row into a sorted set (for set equality assertion). */
+ private Set<String> collectColumn1(String query) throws Exception {
+ Set<String> out = new TreeSet<>();
+ try (Statement st = conn.createStatement();
+ ResultSet rs = st.executeQuery(query)) {
+ while (rs.next()) {
+ out.add(rs.getString(1));
+ }
+ }
+ return out;
+ }
+
+ /** Run a query that returns one numeric scalar in column 1. */
+ private long scalarLong(String query) throws Exception {
+ try (Statement st = conn.createStatement();
+ ResultSet rs = st.executeQuery(query)) {
+ if (rs.next()) {
+ return rs.getLong(1);
+ }
+ return 0L;
+ }
+ }
+
+ /**
+ * Run the query {@link #TIMING_ITERATIONS} times, drop the first
+ * {@link #TIMING_WARMUP} as warm-up, return the average wall-clock duration
+ * of the remaining iterations in nanoseconds. Each iteration drains the
+ * ResultSet so we measure execution + transport, not just submission.
+ */
+ private long averageQueryNanos(String query) throws Exception {
+ long total = 0;
+ int counted = 0;
+ for (int i = 0; i < TIMING_ITERATIONS; i++) {
+ long start = System.nanoTime();
+ try (Statement st = conn.createStatement();
+ ResultSet rs = st.executeQuery(query)) {
+ while (rs.next()) {
+ rs.getString(1);
+ }
+ }
+ long elapsed = System.nanoTime() - start;
+ if (i >= TIMING_WARMUP) {
+ total += elapsed;
+ counted++;
+ }
+ }
+ return counted == 0 ? 0 : total / counted;
+ }
+
/**
* Run EXPLAIN and return one map per table alias (or table name) → column→value.
* Uses traditional EXPLAIN format so column names match across MySQL versions.
@@ -259,7 +394,8 @@ public class TemplateJoinDaoImplBypassExplainTest {
out.put(tableKey, row);
}
}
- LOG.info(log.toString());
+ // uncomment if need verbose EXPLAIN output
+// System.out.println(log.toString());
}
return out;
}