diff --git a/CHANGES.txt b/CHANGES.txt index 19ba7ab37d..a37a3a349a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.2 + * Remove mocking in InternalNodeProbe spying on StorageServiceMBean (CASSANDRA-18152) * Add compaction_properties column to system.compaction_history table and nodetool compactionhistory command (CASSANDRA-18061) * Remove ProtocolVersion entirely from the CollectionSerializer ecosystem (CASSANDRA-18114) * Fix serialization error in new getsstables --show-levels option (CASSANDRA-18140) diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 7dab8745f3..170dade2d5 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -231,6 +231,9 @@ public class StorageService extends NotificationBroadcasterSupport implements IE private final SamplingManager samplingManager = new SamplingManager(); + @VisibleForTesting // this is used for dtests only, see CASSANDRA-18152 + public volatile boolean skipNotificationListeners = false; + @Deprecated public boolean isInShutdownHook() { @@ -7134,4 +7137,27 @@ public class StorageService extends NotificationBroadcasterSupport implements IE { return DatabaseDescriptor.getSkipStreamDiskSpaceCheck(); } + + @Override + public void removeNotificationListener(NotificationListener listener) throws ListenerNotFoundException + { + if (!skipNotificationListeners) + super.removeNotificationListener(listener); + } + + @Override + public void removeNotificationListener(NotificationListener listener, NotificationFilter filter, Object handback) throws ListenerNotFoundException + { + if (!skipNotificationListeners) + super.removeNotificationListener(listener, filter, handback); + } + + @Override + public void addNotificationListener(NotificationListener listener, + NotificationFilter filter, + Object handback) throws java.lang.IllegalArgumentException + { + if (!skipNotificationListeners) + super.addNotificationListener(listener, filter, handback); + } } diff --git a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java index c3cae9ab54..af14035734 100644 --- a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java +++ b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java @@ -932,9 +932,9 @@ public class Instance extends IsolatedExecutor implements IInvokableInstance public NodeToolResult nodetoolResult(boolean withNotifications, String... commandAndArgs) { return sync(() -> { - try (CapturingOutput output = new CapturingOutput()) + try (CapturingOutput output = new CapturingOutput(); + DTestNodeTool nodetool = new DTestNodeTool(withNotifications, output.delegate)) { - DTestNodeTool nodetool = new DTestNodeTool(withNotifications, output.delegate); // install security manager to get informed about the exit-code System.setSecurityManager(new SecurityManager() { @@ -1018,15 +1018,18 @@ public class Instance extends IsolatedExecutor implements IInvokableInstance } } - public static class DTestNodeTool extends NodeTool { + public static class DTestNodeTool extends NodeTool implements AutoCloseable + { private final StorageServiceMBean storageProxy; private final CollectingNotificationListener notifications = new CollectingNotificationListener(); - + private final InternalNodeProbe internalNodeProbe; private Throwable latestError; - public DTestNodeTool(boolean withNotifications, Output output) { + public DTestNodeTool(boolean withNotifications, Output output) + { super(new InternalNodeProbeFactory(withNotifications), output); - storageProxy = new InternalNodeProbe(withNotifications).getStorageService(); + internalNodeProbe = new InternalNodeProbe(withNotifications); + storageProxy = internalNodeProbe.getStorageService(); storageProxy.addNotificationListener(notifications, null, null); } @@ -1072,6 +1075,12 @@ public class Instance extends IsolatedExecutor implements IInvokableInstance super.err(e); latestError = e; } + + @Override + public void close() + { + internalNodeProbe.close(); + } } private static final class CollectingNotificationListener implements NotificationListener diff --git a/test/distributed/org/apache/cassandra/distributed/mock/nodetool/InternalNodeProbe.java b/test/distributed/org/apache/cassandra/distributed/mock/nodetool/InternalNodeProbe.java index 097206a77f..381bb09ec4 100644 --- a/test/distributed/org/apache/cassandra/distributed/mock/nodetool/InternalNodeProbe.java +++ b/test/distributed/org/apache/cassandra/distributed/mock/nodetool/InternalNodeProbe.java @@ -21,7 +21,6 @@ package org.apache.cassandra.distributed.mock.nodetool; import java.lang.management.ManagementFactory; import java.util.Iterator; import java.util.Map; -import javax.management.ListenerNotFoundException; import com.google.common.collect.Multimap; @@ -45,14 +44,13 @@ import org.apache.cassandra.service.CacheServiceMBean; import org.apache.cassandra.service.GCInspector; import org.apache.cassandra.service.StorageProxy; import org.apache.cassandra.service.StorageService; -import org.apache.cassandra.service.StorageServiceMBean; import org.apache.cassandra.streaming.StreamManager; import org.apache.cassandra.tools.NodeProbe; -import org.mockito.Mockito; public class InternalNodeProbe extends NodeProbe { private final boolean withNotifications; + private boolean previousSkipNotificationListeners = false; public InternalNodeProbe(boolean withNotifications) { @@ -66,26 +64,10 @@ public class InternalNodeProbe extends NodeProbe mbeanServerConn = null; jmxc = null; - if (withNotifications) - { - ssProxy = StorageService.instance; - } - else - { - // replace the notification apis with a no-op method - StorageServiceMBean mock = Mockito.spy(StorageService.instance); - Mockito.doNothing().when(mock).addNotificationListener(Mockito.any(), Mockito.any(), Mockito.any()); - try - { - Mockito.doNothing().when(mock).removeNotificationListener(Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doNothing().when(mock).removeNotificationListener(Mockito.any()); - } - catch (ListenerNotFoundException e) - { - throw new AssertionError(e); - } - ssProxy = mock; - } + previousSkipNotificationListeners = StorageService.instance.skipNotificationListeners; + StorageService.instance.skipNotificationListeners = !withNotifications; + + ssProxy = StorageService.instance; msProxy = MessagingService.instance(); streamProxy = StreamManager.instance; compactionProxy = CompactionManager.instance; @@ -105,7 +87,7 @@ public class InternalNodeProbe extends NodeProbe @Override public void close() { - // nothing to close. no-op + StorageService.instance.skipNotificationListeners = previousSkipNotificationListeners; } @Override