This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 328687afd0989c67ef8fff414a4cfbb6178a2351 Author: opencode <[email protected]> AuthorDate: Tue Nov 3 14:41:50 2026 +0100 Put the pooled-connection JMX contract in line with what the pool publishes The per-connection JMX resource published three timestamps whose contract on the management interface did not match what the implementation hands out, and the attribute documentation told a monitoring consumer almost nothing about what it was reading. LastConnected carries a -1 sentinel whenever the pooled connection has no physical session - the field starts there and every disconnect resets it, which every reconnect passes through while the MBean stays registered - and MaxAgeExpired reads the same field, reporting true for that whole window, which is self-consistent for the pool's force-reconnect reading but not for a consumer computing session ages from the raw value. The sentinel and the disconnected-session reading (qualified to the case where a max age is configured, as without one nothing can expire) are now part of the documented contract on interface and implementation, and reconnectWith() takes the timestamp out of service with the session fields it hides and hands it back when the attempt is refused, so the sentinel covers the credential-reconnect window like every other no-session state. LastValidated was stamped from a timestamp sampled before the validator, isValid() or the validation query ran, so the published completion time was early by the whole validation duration; the three success points now stamp completion. Its construction-time seeding, which makes a never-validated connection read as one validated at creation, is kept - seeding a sentinel instead would force a real validation onto the first borrow of a just-connected session - and is now documented. Timestamp published the epoch between the MBean registration inside connect() and the pool's first setTimestamp(), which runs the init SQL and the connect-time validation; the field is seeded at construction, itself a pool action, so a registered attribute never carries an unsanctioned value, and the interface documents its basis, initial value and reset sources, including the ResetAbandonedTimer interceptor that moves it on plain application calls. clearWarnings is the one write a JMX client can perform on the resource. It now reports a driver refusal through a declared SQLException, which JMX surfaces as an MBeanException, where it previously swallowed the failure and left the client unable to tell a completed clear from a refused one; the one internal caller, the return path, catches it so a return still never fails over a refused warnings clear. The delegation surface as a whole is stated as best-effort snapshot access against a possibly-busy session, which is the semantics the per-connection lock could actually deliver - the borrower never holds that lock while using the session, and it is a no-op unless configured - and the interface documents that a remote clear reaches an in-use session's unread warnings. The remaining documentation gaps close as well: ConnectionVersion is explained as the pool purge generation at the connection's creation, seeded from Long.MIN_VALUE so a freshly started pool legitimately reports a large negative number, and isClosed documents the no-session state. Tested by the jdbc-pool testsuite (822 tests, 0 failures); the javadoc build stays warning-free. --- .../apache/tomcat/jdbc/pool/ConnectionPool.java | 12 ++++- .../apache/tomcat/jdbc/pool/PooledConnection.java | 57 ++++++++++++-------- .../tomcat/jdbc/pool/PooledConnectionMBean.java | 60 +++++++++++++++++----- 3 files changed, 93 insertions(+), 36 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index e1b8e39487..41dcd9cd51 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -1886,7 +1886,17 @@ public class ConnectionPool { if (busy.remove(con)) { try { if (!shouldClose(con, PooledConnection.VALIDATE_RETURN) && reconnectIfExpired(con)) { - con.clearWarnings(); + try { + con.clearWarnings(); + } catch (SQLException x) { + // The application is done with the session and + // the return must not fail over a warning chain + // the driver refuses to drop; this is the one + // caller for which a refused clear carries no + // outcome anyone can act on, unlike the JMX + // operation, which reports it to its client + log.warn(MessageFactory.get("pooledConnection.unableToClearWarnings"), x); + } con.setStackTrace(null); con.setTimestamp(System.currentTimeMillis()); // The application is done with the connection: diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java index 0d106907e0..46ea99ddb2 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java @@ -108,9 +108,11 @@ public class PooledConnection implements PooledConnectionMBean { */ private final AtomicReference<Object> abandonTrace = new AtomicReference<>(); /** - * Timestamp the connection was last 'touched' by the pool + * Timestamp the connection was last 'touched' by the pool. Seeded at construction - itself a pool action - so a + * connection that registers its MBean inside a successful connect() never publishes the epoch-0 initial value + * before the pool's first setTimestamp() lands. */ - private volatile long timestamp; + private volatile long timestamp = System.currentTimeMillis(); /** * Lock for this connection only */ @@ -124,7 +126,9 @@ public class PooledConnection implements PooledConnectionMBean { */ private volatile long lastConnected = -1; /** - * timestamp to keep track of validation intervals + * Timestamp of the last completed successful validation, used to keep track of validation intervals. Seeded with + * the time this pooled connection was created, so on a pool that runs no validation it never moves and cannot be + * told apart from a real validation at that instant. */ private volatile long lastValidated = System.currentTimeMillis(); /** @@ -597,7 +601,9 @@ public class PooledConnection implements PooledConnectionMBean { /** * Returns true if the connection has been connected more than {@link PoolConfiguration#getMaxAge()} milliseconds. - * false otherwise. + * false otherwise. When a max age is configured, a connection that currently has no physical session - see + * {@link #getLastConnected()} - also returns true, as a session that is not connected is never young enough to + * keep; without a configured max age this method always returns false. * * @return Returns true if the connection has been connected more than {@link PoolConfiguration#getMaxAge()} * milliseconds. false otherwise. @@ -664,6 +670,7 @@ public class PooledConnection implements PooledConnectionMBean { java.sql.Connection served = connection; javax.sql.XAConnection servedXa = xaConnection; javax.sql.PooledConnection servedCpds = cpdsConnection; + long servedLastConnected = lastConnected; HashMap<Object, Object> sessionAttributes = getAttributes(); boolean hadUser = sessionAttributes.containsKey(PROP_USER); Object servedUser = sessionAttributes.get(PROP_USER); @@ -687,10 +694,14 @@ public class PooledConnection implements PooledConnectionMBean { // Set the session being replaced out of reach of the connect below, // which closes any session it finds before it opens the new one. This // one has to stay open until the attempt that replaces it is known to - // have succeeded. + // have succeeded. The connected timestamp goes with it: while the + // fields read null, the published LastConnected must read -1 like in + // every other no-session state, as the per-connection MBean stays + // registered across this window. connection = null; xaConnection = null; cpdsConnection = null; + lastConnected = -1; try { connect(); } catch (SQLException | RuntimeException x) { @@ -704,6 +715,7 @@ public class PooledConnection implements PooledConnectionMBean { connection = served; xaConnection = servedXa; cpdsConnection = servedCpds; + lastConnected = servedLastConnected; if (hadUser) { sessionAttributes.put(PROP_USER, servedUser); } else { @@ -947,7 +959,7 @@ public class PooledConnection implements PooledConnectionMBean { return false; } if (valid) { - this.lastValidated = now; + this.lastValidated = System.currentTimeMillis(); return true; } else { if (getPoolProperties().getLogValidationErrors()) { @@ -986,7 +998,7 @@ public class PooledConnection implements PooledConnectionMBean { } try { if (connection.isValid(validationQueryTimeout)) { - this.lastValidated = now; + this.lastValidated = System.currentTimeMillis(); // Terminate - by rollback, never by commit - any // transaction the isValid() call opened, or that a // previous borrower left open: a read-only health check @@ -1041,7 +1053,7 @@ public class PooledConnection implements PooledConnectionMBean { stmt.execute(query); stmt.close(); - this.lastValidated = now; + this.lastValidated = System.currentTimeMillis(); if (executingInitSql) { // Terminate the transaction the init SQL executed into by // committing it: this is the pool's own initialization of a @@ -1370,9 +1382,11 @@ public class PooledConnection implements PooledConnectionMBean { } /** - * Returns the timestamp of the last successful validation query execution. + * Returns the timestamp at which the last successful validation completed. Until a validation has completed for + * the first time, this is the time this pooled connection was created - which on a pool with no validation + * configured is the value the attribute keeps. * - * @return the timestamp of the last successful validation query execution as defined by + * @return the timestamp at which the last successful validation completed as defined by * {@link System#currentTimeMillis()} */ @Override @@ -1460,10 +1474,12 @@ public class PooledConnection implements PooledConnectionMBean { /** - * Returns the timestamp of when the connection was last connected to the database. ie, a successful call to + * Returns the timestamp of when the connection was last connected to the database, ie, a successful call to * {@link java.sql.Driver#connect(String, java.util.Properties)}. * - * @return the timestamp when this connection was created as defined by {@link System#currentTimeMillis()} + * @return the timestamp of the last successful connection as defined by {@link System#currentTimeMillis()}, or -1 + * when this pooled connection currently has no physical session - never connected, disconnected, or + * mid-reconnect */ @Override public long getLastConnected() { @@ -1586,21 +1602,20 @@ public class PooledConnection implements PooledConnectionMBean { } /** - * Clears the warnings on the physical connection. A no-op while this pooled connection has no physical connection, - * as there is nothing to clear warnings on. A driver failure to clear the warnings is logged and swallowed rather - * than propagated; the operation is JMX-exposed and therefore declares no exception. + * Clears the warnings on the physical connection. A no-op while this pooled connection has no physical + * connection, as there is nothing to clear warnings on. Like the delegating getters below, this operation is + * best effort against the live session: it does not take the per-connection lock, and the session it reaches + * may be in use by a borrower or by the pool's own work. + * + * @throws SQLException if the driver refuses to clear the warnings */ @Override - public void clearWarnings() { + public void clearWarnings() throws SQLException { java.sql.Connection c = connection; if (c == null) { return; } - try { - c.clearWarnings(); - } catch (SQLException e) { - log.warn(MessageFactory.get("pooledConnection.unableToClearWarnings"), e); - } + c.clearWarnings(); } /** diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnectionMBean.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnectionMBean.java index 1d6aeccef3..78df8ad1d6 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnectionMBean.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnectionMBean.java @@ -19,14 +19,19 @@ package org.apache.tomcat.jdbc.pool; import java.sql.SQLException; /** - * JMX MBean interface for monitoring pooled connections. + * JMX MBean interface for monitoring and managing pooled connections. */ public interface PooledConnectionMBean { // PooledConnection /** - * Returns the connection version. + * Returns the connection version: the pool purge generation in force + * when this pooled connection was created. The pool counter is seeded + * from {@link Long#MIN_VALUE}, so a connection created against a freshly + * started pool reports a large negative number; it is not a connection + * age and not a version of the driver. Connections older than the pool's + * current generation are retired by the pool as they are used or swept. * - * @return the connection version + * @return the pool purge generation at connection creation */ long getConnectionVersion(); @@ -38,7 +43,11 @@ public interface PooledConnectionMBean { boolean isInitialized(); /** - * Returns true if the connection has exceeded its maximum age. + * Returns true if the connection has exceeded its maximum age. When a + * maximum age is configured, a connection that currently has no physical + * session - see {@link #getLastConnected()} - also returns true, as a + * session that is not connected is never young enough to keep; without a + * configured maximum age this attribute always reports false. * * @return true if max age expired */ @@ -52,9 +61,16 @@ public interface PooledConnectionMBean { boolean isSuspect(); /** - * Returns the timestamp of the last pool action. + * Returns the timestamp of the last pool action on this connection. + * Timestamps are set when the pool hands the connection out or takes it + * back, and the value drives the abandoned-connection check - it is also + * reset by the ResetAbandonedTimer interceptor on application calls, + * which additionally clears the suspect flag, so a reset need not be a + * pool action. The value starts at the time this pooled connection was + * created. * - * @return the timestamp + * @return the timestamp of the last pool action as defined by + * {@link System#currentTimeMillis()} */ long getTimestamp(); @@ -66,16 +82,25 @@ public interface PooledConnectionMBean { boolean isDiscarded(); /** - * Returns the timestamp of the last successful validation. + * Returns the timestamp at which the last successful validation + * completed. Until a validation has completed for the first time, this is + * the time this pooled connection was created - which on a pool with no + * validation configured is the value the attribute keeps, so a + * never-validated connection cannot be told apart from one validated at + * its creation instant. * - * @return the last validated timestamp + * @return the last validated timestamp as defined by + * {@link System#currentTimeMillis()} */ long getLastValidated(); /** - * Returns the timestamp of the last successful connection. + * Returns the timestamp of the last successful connection, or -1 when + * this pooled connection currently has no physical session - never + * connected, disconnected, or mid-reconnect. * - * @return the last connected timestamp + * @return the last connected timestamp as defined by + * {@link System#currentTimeMillis()}, or -1 */ long getLastConnected(); @@ -88,16 +113,23 @@ public interface PooledConnectionMBean { // java.sql.Connection /** - * Clears any warnings reported on this connection. + * Clears any warnings reported on this connection. The operation reaches + * a live session whether or not it is currently borrowed, so warnings an + * application has not yet read are cleared as well. It is a no-op while + * the pooled connection has no physical session. + * + * @throws SQLException if the driver refuses to clear the warnings, so a + * JMX client can tell a refused clear from a completed one */ - void clearWarnings(); + void clearWarnings() throws SQLException; /** - * Returns true if the connection has been closed. + * Returns true if the connection has been closed, including the case + * where the pooled connection currently has no physical session. * * @return true if closed * - * @throws SQLException if a database access error occurs + * @throws SQLException if the physical connection reports a failure */ boolean isClosed() throws SQLException; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
