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 7dc85d5a858dd94ba4a0d2d946fc7c7bde67c4cf Author: opencode <[email protected]> AuthorDate: Thu Nov 5 15:41:06 2026 +0100 Report a malformed JMX name once per pool SlowQueryReportJmx.registerJmx() retried its publication on every borrow, and a failure that could not form an ObjectName had no name to mark as already reported. A permanently malformed objectName or pool name therefore logged an ERROR with a stack trace on every borrow. Remember the malformed-name report in PoolJmx: the first failure keeps the error log with its exception, later retries stay at debug level, and publication is still retried. Update the malformed-name test to count the error records across two borrows. --- .../jdbc/pool/interceptor/SlowQueryReportJmx.java | 23 ++++--- .../interceptor/TestSlowQueryJmxNotifications.java | 72 ++++++++++++++++------ 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java index d3c5d02eb4..8bfb374fb5 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java @@ -171,6 +171,11 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE * refusal is not reported once per borrow; guarded by this */ private ObjectName failureLoggedUnder; + /** + * whether a failure that could not form an ObjectName has been reported, so that a permanent malformed name + * is not logged with a stack trace on every borrow; guarded by this + */ + private boolean malformedNameReported; /** * whether a publication attempt is in flight, so that the concurrent borrows of one pool make one attempt * rather than one per thread; guarded by this @@ -295,16 +300,19 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE * </p> * * @param oname the ObjectName whose publication was refused, or {@code null} when no ObjectName could be formed - * - there is then no name to mark as reported, and the failure is reported on every borrow of - * such a pool: the condition is a permanent misconfiguration, the stack trace is the signal - * that makes the operator fix it, and a busy pool with a misnamed monitoring MBean is rarer - - * and less costly - than a suppressed error nobody ever acts on + * - there is then no name to mark, so this state remembers that the malformed name was + * reported and later retries stay at debug level * @param x what the MBeanServer answered */ protected synchronized void refused(ObjectName oname, Exception x) { publishing = false; if (oname == null) { - log.error(MessageFactory.get("slowQueryReportJmx.jmxRegistrationFailedNoData"), x); + if (!malformedNameReported) { + malformedNameReported = true; + log.error(MessageFactory.get("slowQueryReportJmx.jmxRegistrationFailedNoData"), x); + } else if (log.isDebugEnabled()) { + log.debug("Jmx registration failed again: " + x); + } return; } if (oname.equals(failureLoggedUnder)) { @@ -914,8 +922,9 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE } catch (MalformedObjectNameException | RuntimeOperationsException x) { // A name that cannot be formed - a malformed objectName property, or // a pool name carrying characters the ObjectName grammar reserves - - // has no ObjectName to record the refusal against, and so is reported - // on every borrow of such a pool (see refused). + // has no ObjectName to record the refusal against; the state + // remembers that it was reported and later retries stay at debug + // level (see refused). state.refused(null, x); } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java index 538ae4bbe2..546d499a6f 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java @@ -32,6 +32,10 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; import java.util.stream.Collectors; import javax.management.InstanceNotFoundException; @@ -1200,11 +1204,9 @@ class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { void aMalformedObjectNameOverrideFailsOnlyTheRegistration() throws Exception { // An override value that is not an ObjectName at all (no domain // separator) fails the name formation on every borrow, before the - // registry slot and its refusal throttle are reachable - the failure - // is reported each time (see the acceptability comment in - // registerJmx). What must stay true while it is: the failure touches - // nothing but the monitoring, never latches the slot, and leaves - // nothing under the name behind. + // registry slot and its refusal throttle are reachable. The first + // failure is reported with its stack trace; later retries stay at + // debug level, and the failure touches nothing but monitoring. String poolName = "JmxMalformedOname-" + System.nanoTime(); DataSource ds = reportingPoolNamed(poolName, "org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx" @@ -1213,22 +1215,54 @@ class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { MBeanServer server = ManagementFactory.getPlatformMBeanServer(); Set<ObjectName> registeredBefore = server.queryNames(new ObjectName("tomcat.jdbc:type=" + SlowQueryReportJmx.class.getName() + ",*"), null); - try (@SuppressWarnings("unused") Connection first = ds.getConnection()) { - // the borrow itself served: a failed name formation cannot fail - // the pool's operation, only its monitoring - Assertions.assertEquals(registeredBefore, server.queryNames(new ObjectName("tomcat.jdbc:type=" - + SlowQueryReportJmx.class.getName() + ",*"), null), - "no registration may appear while the name cannot be formed"); - } - try (@SuppressWarnings("unused") Connection second = ds.getConnection()) { - SlowQueryReportJmx.PoolJmx state = SlowQueryReportJmx.jmxStateFor(ds.getPool()); - Assertions.assertNull(state.publishedUnder(), - "a name that cannot be formed must never leave the pool claiming a publication"); - Assertions.assertNull(state.failureMarker(), - "and a failure that cannot form its ObjectName has no name to mark as" - + " reported, so it is reported on every borrow of such a pool"); + Logger logger = Logger.getLogger(SlowQueryReportJmx.class.getName()); + Level originalLevel = logger.getLevel(); + List<LogRecord> records = new CopyOnWriteArrayList<>(); + Handler handler = new Handler() { + @Override + public void publish(LogRecord record) { + records.add(record); + } + + @Override + public void flush() { + // no buffering + } + + @Override + public void close() { + // nothing to release + } + }; + logger.addHandler(handler); + logger.setLevel(Level.ALL); + try { + try (@SuppressWarnings("unused") Connection first = ds.getConnection()) { + // the borrow itself served: a failed name formation cannot fail + // the pool's operation, only its monitoring + Assertions.assertEquals(registeredBefore, server.queryNames(new ObjectName("tomcat.jdbc:type=" + + SlowQueryReportJmx.class.getName() + ",*"), null), + "no registration may appear while the name cannot be formed"); + } + try (@SuppressWarnings("unused") Connection second = ds.getConnection()) { + SlowQueryReportJmx.PoolJmx state = SlowQueryReportJmx.jmxStateFor(ds.getPool()); + Assertions.assertNull(state.publishedUnder(), + "a name that cannot be formed must never leave the pool claiming a publication"); + Assertions.assertNull(state.failureMarker(), + "a failure that cannot form its ObjectName has no name to mark as reported"); + } + } finally { + logger.removeHandler(handler); + logger.setLevel(originalLevel); } + long noDataErrors = records.stream() + .filter(r -> r.getLevel().intValue() >= Level.SEVERE.intValue() + && r.getMessage() != null && r.getMessage().contains("no JMX data will be exposed")) + .count(); + Assertions.assertEquals(1, noDataErrors, + "a permanently malformed name must be reported once, not on every borrow"); + ConnectionPool pool = ds.getPool(); ds.close(); Assertions.assertNull(SlowQueryReportJmx.jmxStateFor(pool), --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
