This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit cef17d34be1437bd238a107d083444887d6f43a4 Author: opencode <[email protected]> AuthorDate: Mon Oct 26 13:09:54 2026 +0100 Share the slow query JMX notifier across a pool's interceptor instances The SlowQueryReportJmx interceptor is instantiated once per pooled connection. With notifyPool=false, each instance kept its own NotificationBroadcasterSupport while all instances registered their MBean under the same object name derived from the pool name. A JMX client could only subscribe to the MBean the platform server resolved first, so slow and failed query notifications raised by every other interceptor instance were sent to a broadcaster nobody could subscribe to and were silently dropped. Resolve the notification broadcaster by pool name through a static registry so that all interceptor instances sharing a pool name share the bcaster behind the registered MBean, and route every listener registration/removal and notification through that shared instance. When an instance unregisters its MBean, the pool name is released so a surviving same-named instance can take over the MBean. Add a regression test that configures the interceptor twice on one pool with notifyPool=false and asserts that a listener registered through the MBean receives notifications raised by both interceptor instances. --- .../jdbc/pool/interceptor/SlowQueryReportJmx.java | 44 ++++++++-- .../interceptor/TestSlowQueryJmxNotifications.java | 93 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 7 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 ab238cbf0f..f44c209590 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 @@ -83,13 +83,41 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE protected static final ConcurrentHashMap<String,SlowQueryReportJmxMBean> mbeans = new ConcurrentHashMap<>(); + /** + * Notification broadcasters for each connection pool. Interceptor + * instances are created per pooled connection, but JMX clients + * attach their listeners to the single MBean registered per pool, so + * notifications raised by any instance of a pool must reach the + * broadcaster of the pool rather than a broadcaster private to the + * instance. + */ + protected static final ConcurrentHashMap<String,NotificationBroadcasterSupport> notifiers = + new ConcurrentHashMap<>(); + //==============================JMX STUFF======================== /** - * Notification broadcaster support for JMX notifications. + * Notification broadcaster used before the pool name is known; + * once the pool name is known, {@link #getNotifier()} returns the + * broadcaster shared by all interceptor instances of that pool. */ protected volatile NotificationBroadcasterSupport notifier = new NotificationBroadcasterSupport(); + /** + * Returns the notification broadcaster that all interceptor + * instances of the current pool share, so that notifications raised + * on any connection reach the listeners registered with the pool's + * MBean. + * @return the shared broadcaster for this interceptor's pool + */ + protected NotificationBroadcasterSupport getNotifier() { + String name = poolName; + if (name == null) { + return notifier; + } + return notifiers.computeIfAbsent(name, key -> new NotificationBroadcasterSupport()); + } + /** * Adds a notification listener. * @param listener the listener to add @@ -99,7 +127,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE */ @Override public void addNotificationListener(NotificationListener listener, NotificationFilter filter, Object handback) throws IllegalArgumentException { - notifier.addNotificationListener(listener, filter, handback); + getNotifier().addNotificationListener(listener, filter, handback); } @@ -109,7 +137,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE */ @Override public MBeanNotificationInfo[] getNotificationInfo() { - return notifier.getNotificationInfo(); + return getNotifier().getNotificationInfo(); } /** @@ -119,7 +147,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE */ @Override public void removeNotificationListener(NotificationListener listener) throws ListenerNotFoundException { - notifier.removeNotificationListener(listener); + getNotifier().removeNotificationListener(listener); } @@ -132,7 +160,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE */ @Override public void removeNotificationListener(NotificationListener listener, NotificationFilter filter, Object handback) throws ListenerNotFoundException { - notifier.removeNotificationListener(listener, filter, handback); + getNotifier().removeNotificationListener(listener, filter, handback); } @@ -249,7 +277,8 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE this.pool.getJmxPool().notify(type, query); } } else { - if (notifier!=null) { + NotificationBroadcasterSupport target = getNotifier(); + if (target!=null) { Notification notification = new Notification(type, this, @@ -257,7 +286,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE System.currentTimeMillis(), query); - notifier.sendNotification(notification); + target.sendNotification(notification); } } } catch (RuntimeOperationsException e) { @@ -364,6 +393,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE if (mbeans.remove(poolName)!=null) { ObjectName oname = getObjectName(getClass(),poolName); JmxUtil.unregisterJmx(oname); + notifiers.remove(poolName); } } catch (MalformedObjectNameException | RuntimeOperationsException e) { log.warn("Jmx deregistration failed.",e); 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 new file mode 100644 index 0000000000..85af065940 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.pool.interceptor; + +import java.lang.management.ManagementFactory; +import java.sql.Connection; +import java.sql.Statement; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; + +import javax.management.MBeanServer; +import javax.management.Notification; +import javax.management.NotificationListener; +import javax.management.ObjectName; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; +import org.apache.tomcat.jdbc.pool.DataSource; + +/** + * JMX notifications of the {@code SlowQueryReportJmx} interceptor in + * the documented {@code notifyPool=false} mode. Interceptor chains are + * created per pooled connection, while JMX clients attach their + * listeners to the single MBean registered per pool: a slow query + * occurring on any connection must reach those listeners, not only the + * ones occurring on the connection whose interceptor instance happened + * to win the MBean registration. + */ +class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { + + @Test + void notifyPoolFalseDeliversNotificationsOfEveryConnection() throws Exception { + String poolName = "JmxNotify-" + System.nanoTime(); + DataSource ds = newDataSource(); + ds.setName(poolName); + ds.setJdbcInterceptors("org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx" + + "(notifyPool=false,threshold=-1)"); + ds.getPoolProperties().setMaxActive(2); + ds.getPoolProperties().setInitialSize(0); + + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + ObjectName oname = new ObjectName("tomcat.jdbc:type=" + + SlowQueryReportJmx.class.getName() + ",name=" + poolName); + List<Notification> received = new CopyOnWriteArrayList<>(); + NotificationListener listener = (notification, handback) -> received.add(notification); + + try (Connection first = ds.getConnection()) { + assertTrue(server.isRegistered(oname), + "the interceptor MBean must be registered once the pool is in use"); + server.addNotificationListener(oname, listener, null, null); + try (Connection second = ds.getConnection()) { + // two concurrent borrows with no pre-created connections + // means two physical connections, hence two interceptor + // instances - only one of which can own the MBean + assertEquals(2, ds.getNumActive()); + try (Statement st1 = first.createStatement(); + Statement st2 = second.createStatement()) { + st1.execute("SELECT 1"); + st2.execute("SELECT 2"); + } + } + server.removeNotificationListener(oname, listener); + } + + List<String> messages = received.stream().map(Notification::getMessage) + .collect(Collectors.toList()); + assertEquals(2, messages.size(), + "every slow query of every connection must notify, received: " + messages); + assertTrue(messages.stream().anyMatch(m -> m.contains("SELECT 1")), + "the query run on the first connection must be reported, received: " + messages); + assertTrue(messages.stream().anyMatch(m -> m.contains("SELECT 2")), + "the query run on the second physical connection must also reach the listener, received: " + + messages); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
