This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 99c34208b499abb700b238095138feb7044accd1 Author: opencode <[email protected]> AuthorDate: Mon Oct 26 21:39:26 2026 +0100 Recognise JDBC 4.2 large executions in jdbc-pool query reporting The query reporting interceptors decide what counts as a query execution by matching the invoked statement method name against the EXECUTE_TYPES list, which only covered the JDBC 3 methods execute, executeQuery, executeUpdate and executeBatch. The JDBC 4.2 methods executeLargeUpdate and executeLargeBatch never matched, so large updates and large batches - a common bulk processing pattern - were never timed, never reported as slow, never counted in the query statistics and their failures were not booked as failed queries, silently removing exactly the work most likely to be slow from the monitoring the interceptors exist to provide. Extend EXECUTE_TYPES with the two large-execution names, and as a side effect of the same recognition, the ResetAbandonedTimer interceptor now also resets the abandoned timer for those executions. Factor the three duplicated SQL extraction blocks in AbstractQueryReport into a shared private helper, which additionally labels executeLargeBatch executions as batches, like executeBatch already was. The executeWithFlags methods named in the original report are deliberately not included: they do not exist in java.sql (the JDBC 4.3 flag methods were deferred by JEP 430 and never added). Add regression tests covering the booking of executeLargeUpdate and executeLargeBatch executions and of a failed executeLargeUpdate, and a changelog entry. --- .../AbstractCreateStatementInterceptor.java | 12 +++- .../jdbc/pool/interceptor/AbstractQueryReport.java | 65 +++++++++---------- .../pool/interceptor/TestInterceptorSemantics.java | 72 ++++++++++++++++++++++ webapps/docs/changelog.xml | 11 ++++ 4 files changed, 127 insertions(+), 33 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java index 99c3e10f7c..9b67e4fc85 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractCreateStatementInterceptor.java @@ -103,11 +103,21 @@ public abstract class AbstractCreateStatementInterceptor extends JdbcInterceptor * {@link java.sql.Statement#executeBatch()} */ protected static final String EXECUTE_BATCH = "executeBatch"; + /** + * {@link java.sql.Statement#executeLargeUpdate} and its overloads, and + * {@link java.sql.PreparedStatement#executeLargeUpdate()} + */ + protected static final String EXECUTE_LARGE_UPDATE = "executeLargeUpdate"; + /** + * {@link java.sql.Statement#executeLargeBatch()} + */ + protected static final String EXECUTE_LARGE_BATCH = "executeLargeBatch"; /** * Array of execute type method names */ - protected static final String[] EXECUTE_TYPES = { EXECUTE, EXECUTE_QUERY, EXECUTE_UPDATE, EXECUTE_BATCH }; + protected static final String[] EXECUTE_TYPES = { EXECUTE, EXECUTE_QUERY, EXECUTE_UPDATE, EXECUTE_BATCH, + EXECUTE_LARGE_UPDATE, EXECUTE_LARGE_BATCH }; /** * the constructors that are used to create statement proxies diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java index 28dad3c3b4..9a167920cc 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/AbstractQueryReport.java @@ -62,30 +62,48 @@ public abstract class AbstractQueryReport extends AbstractCreateStatementInterce protected abstract void prepareCall(String query, long time); /** - * Invoked when a query execution, a call to execute/executeQuery or executeBatch failed. - * @param query the query that was executed and failed - * @param args the arguments to the execution - * @param name the name of the method used to execute {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)} - * @param start the time the query execution started - * @param t the exception that happened - * @return - the SQL that was executed or the string "batch" if it was a batch execution, - * or {@code null} if the invocation carries no SQL string + * Extracts the SQL that a statement execution is reported under: the SQL + * the statement was prepared with, else a {@code String} first argument, + * else {@code "batch"} for batch executions, which carry no SQL of their + * own, else {@code null}. + * + * @param query the SQL the statement was prepared with, or {@code null} + * @param args the arguments of the invoked method + * @param name the name of the invoked method + * + * @return the SQL, {@code "batch"} or {@code null} */ - protected String reportFailedQuery(String query, Object[] args, final String name, long start, Throwable t) { + private String extractQuery(String query, Object[] args, final String name) { //extract the query string String sql = query; if (sql == null && args != null && args.length > 0 && args[0] instanceof String) { sql = (String) args[0]; } //if we do batch execution, then we name the query 'batch' - if (sql==null && compare(EXECUTE_BATCH,name)) { + if (sql == null && (compare(EXECUTE_BATCH, name) || compare(EXECUTE_LARGE_BATCH, name))) { sql = "batch"; } return sql; } /** - * Invoked when a query execution, a call to execute/executeQuery or executeBatch succeeded and was within the timing threshold + * Invoked when a query execution, a call to an execute method as + * recognised by {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)}, failed. + * @param query the query that was executed and failed + * @param args the arguments to the execution + * @param name the name of the method used to execute {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)} + * @param start the time the query execution started + * @param t the exception that happened + * @return - the SQL that was executed or the string "batch" if it was a batch execution, + * or {@code null} if the invocation carries no SQL string + */ + protected String reportFailedQuery(String query, Object[] args, final String name, long start, Throwable t) { + return extractQuery(query, args, name); + } + + /** + * Invoked when a query execution, a call to an execute method as + * recognised by {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)}, succeeded and was within the timing threshold * @param query the query that was executed and failed * @param args the arguments to the execution * @param name the name of the method used to execute {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)} @@ -95,20 +113,12 @@ public abstract class AbstractQueryReport extends AbstractCreateStatementInterce * or {@code null} if the invocation carries no SQL string */ protected String reportQuery(String query, Object[] args, final String name, long start, long delta) { - //extract the query string - String sql = query; - if (sql == null && args != null && args.length > 0 && args[0] instanceof String) { - sql = (String) args[0]; - } - //if we do batch execution, then we name the query 'batch' - if (sql==null && compare(EXECUTE_BATCH,name)) { - sql = "batch"; - } - return sql; + return extractQuery(query, args, name); } /** - * Invoked when a query execution, a call to execute/executeQuery or executeBatch succeeded and was exceeded the timing threshold + * Invoked when a query execution, a call to an execute method as + * recognised by {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)}, succeeded and was exceeded the timing threshold * @param query the query that was executed and failed * @param args the arguments to the execution * @param name the name of the method used to execute {@link AbstractCreateStatementInterceptor#isExecute(Method, boolean)} @@ -118,16 +128,7 @@ public abstract class AbstractQueryReport extends AbstractCreateStatementInterce * or {@code null} if the invocation carries no SQL string */ protected String reportSlowQuery(String query, Object[] args, final String name, long start, long delta) { - //extract the query string - String sql = query; - if (sql == null && args != null && args.length > 0 && args[0] instanceof String) { - sql = (String) args[0]; - } - //if we do batch execution, then we name the query 'batch' - if (sql==null && compare(EXECUTE_BATCH,name)) { - sql = "batch"; - } - return sql; + return extractQuery(query, args, name); } /** diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestInterceptorSemantics.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestInterceptorSemantics.java index 4d60cd33a0..54729b2531 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestInterceptorSemantics.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestInterceptorSemantics.java @@ -153,6 +153,78 @@ class TestInterceptorSemantics extends JdbcPoolBaseTest { return stats.keySet(); } + private static SlowQueryReport.QueryStats statsFor(ConcurrentHashMap<String, ?> stats, + String key) { + SlowQueryReport.QueryStats qs = (SlowQueryReport.QueryStats) stats.get(key); + Assertions.assertNotNull(qs, "no stats entry for " + key + + ", recorded keys: " + statsKeySet(stats)); + return qs; + } + + /* + * The JDBC 4.2 execution methods executeLargeUpdate and executeLargeBatch + * were not recognised as executions, so they were neither timed nor booked + * in the query statistics - a pool under monitoring silently ignored + * exactly the long-running batch work the statistics exist to surface. + */ + @Test + void jdbc42ExecutionsAreBookedInQueryStats() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setName("large_" + System.nanoTime()); + ds.getPoolProperties().setJdbcInterceptors( + "org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport"); + ds.createPool(); + String poolName = ds.getPoolProperties().getPoolName(); + String t = uniqueTable(); + + String largeUpdateSql = "INSERT INTO " + t + " VALUES (1)"; + String batchedSql = "INSERT INTO " + t + " VALUES (2)"; + try (Connection c = ds.getConnection(); Statement st = c.createStatement()) { + c.createStatement().execute("CREATE TABLE " + t + " (v INT)"); + Assertions.assertEquals(1, st.executeLargeUpdate(largeUpdateSql)); + + st.addBatch(batchedSql); + Assertions.assertArrayEquals(new long[] { 1 }, st.executeLargeBatch()); + } + + ConcurrentHashMap<String, ?> stats = SlowQueryReport.getPoolStats(poolName); + Assertions.assertNotNull(stats, "stats must exist once the executions ran"); + Assertions.assertEquals(1, + statsFor(stats, largeUpdateSql).getNrOfInvocations(), + "executeLargeUpdate must be booked under its SQL"); + Assertions.assertEquals(1, statsFor(stats, "batch").getNrOfInvocations(), + "executeLargeBatch must be booked as a batch execution"); + Assertions.assertEquals(2, adminRowCount(t), "both rows must be inserted"); + } + + /* + * A failure of a JDBC 4.2 execution method must be booked against the + * executed SQL as a failed query, just like the JDBC 3 executions are. + */ + @Test + void failedJdbc42ExecutionIsBookedInQueryStats() throws Exception { + DataSource ds = newDataSource(); + ds.getPoolProperties().setName("largefail_" + System.nanoTime()); + ds.getPoolProperties().setJdbcInterceptors( + "org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport"); + ds.createPool(); + String poolName = ds.getPoolProperties().getPoolName(); + + String nonce = "largefail_" + System.nanoTime(); + String sql = "INSERT INTO no_such_table_" + nonce + " VALUES (1)"; + try (Connection c = ds.getConnection(); Statement st = c.createStatement()) { + Assertions.assertThrows(SQLException.class, () -> st.executeLargeUpdate(sql)); + } + + ConcurrentHashMap<String, ?> stats = SlowQueryReport.getPoolStats(poolName); + Assertions.assertNotNull(stats, "stats must exist once a query failed"); + // QueryStats.failure() books the execution through add() before + // counting the failure, so one booked invocation proves the failure + // went through the failed-query reporting path + Assertions.assertEquals(1, statsFor(stats, sql).getNrOfInvocations(), + "a failed executeLargeUpdate must be booked as an execution of " + sql); + } + @Test void queryTimeoutInterceptorCancelsAndPoolStaysUsable() throws Exception { DataSource ds = newDataSource(); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index f88cc91872..25981d1684 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -846,6 +846,17 @@ already in-flight connection when the cancellation comes too late. (remm) </fix> + <fix> + Include the JDBC 4.2 execution methods + <code>executeLargeUpdate()</code> and + <code>executeLargeBatch()</code> in the methods that the query + reporting interceptors recognise as query executions. Slow or + failed large updates and large batches – a common bulk + processing pattern – were never timed, never reported as slow + and never counted in the query statistics, so the monitoring the + interceptors exist to provide silently missed exactly the work + most likely to be slow. (remm) + </fix> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
