Remove mocking in InternalNodeProbe spying on StorageServiceMBean

patch by Stefan Miklosovic; reviewed by Andrés de la Peña for CASSANDRA-18152
This commit is contained in:
Stefan Miklosovic 2023-01-16 13:37:21 +01:00
parent 49dfb805e9
commit 36a99bcdcc
4 changed files with 48 additions and 30 deletions

View File

@ -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)

View File

@ -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);
}
}

View File

@ -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

View File

@ -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