changeset 574:06f2dcf9ad3b

Add failure code for thread monitor com channel, client review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-August/002991.html reviewed-by: vanaltj
author Mario Torre <neugens.limasoftware@gmail.com>
date Fri, 31 Aug 2012 10:12:56 +0200
parents 92e6648fd11d
children 4fbe27356f96
files thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/ThreadView.java thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/ThreadCollector.java thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java thread/client-controllers/src/test/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationControllerTest.java thread/client-swing/src/main/java/com/redhat/thermostat/thread/client/swing/impl/SwingThreadView.java thread/collector/src/main/java/com/redhat/thermostat/thread/collector/HarvesterCommand.java thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java
diffstat 9 files changed, 213 insertions(+), 43 deletions(-) [+]
line wrap: on
line diff
--- a/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/ThreadView.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/ThreadView.java	Fri Aug 31 10:12:56 2012 +0200
@@ -62,7 +62,7 @@
         notifier.removeActionListener(listener);
     }
     
-    public abstract void setRecording(boolean recording);
+    public abstract void setRecording(boolean recording, boolean notify);
     
     public abstract void setDaemonThreads(String daemonThreads);
     public abstract void setLiveThreads(String liveThreads);
@@ -70,4 +70,6 @@
     
     public abstract VMThreadCapabilitiesView createVMThreadCapabilitiesView();
     public abstract ThreadTableView createThreadTableView();
+    
+    public abstract void displayWarning(String warning);
 }
--- a/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/ThreadCollector.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/ThreadCollector.java	Fri Aug 31 10:12:56 2012 +0200
@@ -46,9 +46,10 @@
     
     VMThreadCapabilities getVMThreadCapabilities();
     
-    void startHarvester();
-    void stopHarvester();
-
+    boolean startHarvester();
+    boolean stopHarvester();
+    boolean isHarvesterCollecting();
+    
     ThreadSummary getLatestThreadSummary();
     List<ThreadSummary> getThreadSummary(long since);
     List<ThreadSummary> getThreadSummary();
--- a/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java	Fri Aug 31 10:12:56 2012 +0200
@@ -81,26 +81,103 @@
     }
     
     @Override
-    public void startHarvester() {
+    public boolean startHarvester() {
         
         Request harvester = createRequest();
         harvester.setParameter(HarvesterCommand.class.getName(), HarvesterCommand.START.name());
         harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getIdString());
         harvester.setParameter(HarvesterCommand.AGENT_ID.name(), ref.getAgent().getAgentId());
+        
+        final CountDownLatch latch = new CountDownLatch(1);
+        final boolean[] result = new boolean[1];
+        
+        harvester.addListener(new RequestResponseListener() {
+            @Override
+            public void fireComplete(Request request, Response response) {
+                switch (response.getType()) {
+                case OK:
+                    result[0] = true;
+                    break;
+                default:
+                    break;
+                }
+                latch.countDown();
+            }
+        });
+        
+        RequestQueue queue = getRequestQueue();        
+        queue.putRequest(harvester);
 
-        RequestQueue queue = getRequestQueue();
-        queue.putRequest(harvester);
+        try {
+            latch.await();
+        } catch (InterruptedException ignore) {}
+        
+        return result[0];
     }
 
     @Override
-    public void stopHarvester() {
+    public boolean stopHarvester() {
         
         Request harvester = createRequest();
         harvester.setParameter(HarvesterCommand.class.getName(), HarvesterCommand.STOP.name());
         harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getIdString());
 
+        final CountDownLatch latch = new CountDownLatch(1);
+        final boolean[] result = new boolean[1];
+
+        harvester.addListener(new RequestResponseListener() {
+            @Override
+            public void fireComplete(Request request, Response response) {
+                switch (response.getType()) {
+                case OK:
+                    result[0] = true;
+                    break;
+                default:
+                    break;
+                }
+                latch.countDown();
+            }
+        });
+        
         RequestQueue queue = getRequestQueue();
         queue.putRequest(harvester);
+
+        try {
+            latch.await();
+        } catch (InterruptedException ignore) {}
+        return result[0];
+    }
+    
+    @Override
+    public boolean isHarvesterCollecting() {
+        Request harvester = createRequest();
+        harvester.setParameter(HarvesterCommand.class.getName(), HarvesterCommand.IS_COLLECTING.name());
+        harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getIdString());
+
+        final CountDownLatch latch = new CountDownLatch(1);        
+        final boolean[] result = new boolean[1];
+
+        harvester.addListener(new RequestResponseListener() {
+            @Override
+            public void fireComplete(Request request, Response response) {
+                switch (response.getType()) {
+                case OK:
+                    result[0] = true;
+                    break;
+                default:
+                    break;
+                }
+                latch.countDown();
+            }
+        });
+        
+        RequestQueue queue = getRequestQueue();
+        queue.putRequest(harvester);
+
+        try {
+            latch.await();
+        } catch (InterruptedException ignore) {}
+        return result[0];
     }
     
     @Override
--- a/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java	Fri Aug 31 10:12:56 2012 +0200
@@ -54,6 +54,8 @@
 import com.redhat.thermostat.client.command.RequestQueue;
 import com.redhat.thermostat.common.command.Request;
 import com.redhat.thermostat.common.command.RequestResponseListener;
+import com.redhat.thermostat.common.command.Response;
+import com.redhat.thermostat.common.command.Response.ResponseType;
 import com.redhat.thermostat.common.dao.HostRef;
 import com.redhat.thermostat.common.dao.VmRef;
 import com.redhat.thermostat.thread.client.common.collector.ThreadCollector;
@@ -164,6 +166,26 @@
         when(reference.getIdString()).thenReturn("00101010");
         when(reference.getAgent()).thenReturn(agent);
         
+        final Response response = mock(Response.class);
+        when(response.getType()).thenReturn(ResponseType.OK);
+        
+        final ArgumentCaptor<RequestResponseListener> captor = ArgumentCaptor.forClass(RequestResponseListener.class);
+        doNothing().when(request).addListener(captor.capture());
+        
+        doAnswer(new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) throws Throwable {
+                Request req = (Request) invocation.getArguments()[0];
+                assertSame(request, req);
+                
+                RequestResponseListener listener = captor.getValue();
+                listener.fireComplete(request, response);
+                
+                return null;
+            }
+
+        }).when(requestQueue).putRequest(request);
+        
         ThreadCollector collector = new ThreadMXBeanCollector(threadDao, reference) {
             @Override
             Request createRequest() {
@@ -174,6 +196,7 @@
                 return requestQueue;
             }
         };
+        
         collector.startHarvester();
         
         verify(request).setParameter(HarvesterCommand.class.getName(), HarvesterCommand.START.name());
@@ -193,6 +216,26 @@
         VmRef reference = mock(VmRef.class);
         when(reference.getIdString()).thenReturn("00101010");
         
+        final Response response = mock(Response.class);
+        when(response.getType()).thenReturn(ResponseType.OK);
+        
+        final ArgumentCaptor<RequestResponseListener> captor = ArgumentCaptor.forClass(RequestResponseListener.class);
+        doNothing().when(request).addListener(captor.capture());
+        
+        doAnswer(new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) throws Throwable {
+                Request req = (Request) invocation.getArguments()[0];
+                assertSame(request, req);
+                
+                RequestResponseListener listener = captor.getValue();
+                listener.fireComplete(request, response);
+                
+                return null;
+            }
+
+        }).when(requestQueue).putRequest(request);
+        
         ThreadCollector collector = new ThreadMXBeanCollector(threadDao, reference) {
             @Override
             Request createRequest() {
--- a/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java	Fri Aug 31 10:12:56 2012 +0200
@@ -75,16 +75,12 @@
     private Timer timer;
     private ApplicationCache cache;
     
-    String CACHE_RECORDING_KEY = "thread-live-recording-";
-
     private LivingDaemonThreadDifferenceChart model;
-    
+        
     public ThreadInformationController(VmRef ref, ApplicationService appService,
                                        ThreadCollectorFactory collectorFactory, 
                                        ThreadViewProvider viewFactory)
-    {
-        CACHE_RECORDING_KEY = CACHE_RECORDING_KEY + ref.getStringID();
-        
+    {        
         this.appService = appService;
         cache = appService.getApplicationCache();
 
@@ -115,7 +111,7 @@
                     timer.stop();
                     break;
                 
-                case VISIBLE:                    
+                case VISIBLE:
                     timer.start();
                     break;
 
@@ -125,17 +121,13 @@
             }
         });
         
-        view.setRecording(isRecording());
+        view.setRecording(isRecording(), false);
         view.addThreadActionListener(new ThreadActionListener());
     }
     
     private boolean isRecording() {
-        Boolean isRecording = (Boolean) cache.getAttribute(CACHE_RECORDING_KEY);
-        return (isRecording != null && isRecording); 
-    }
-    
-    private void setRecording(boolean recording) {
-        cache.addAttribute(CACHE_RECORDING_KEY, recording);
+        
+        return collector.isHarvesterCollecting();
     }
     
     @Override
@@ -174,15 +166,24 @@
 
         @Override
         public void actionPerformed(ActionEvent<ThreadAction> actionEvent) {
+
+            boolean result = false;
+            
             switch (actionEvent.getActionId()) {
             case START_LIVE_RECORDING:
-                collector.startHarvester();
-                setRecording(true);
+                result = collector.startHarvester();
+                if (!result) {
+                    view.displayWarning("Cannot enable Thread recording");
+                    view.setRecording(false, false);
+                }
                 break;
             
             case STOP_LIVE_RECORDING:
-                collector.stopHarvester();
-                setRecording(false);
+                result = collector.stopHarvester();
+                if (!result) {
+                    view.displayWarning("Cannot disable Thread recording");
+                    view.setRecording(true, false);
+                }
                 break;
                 
             default:
--- a/thread/client-controllers/src/test/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationControllerTest.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-controllers/src/test/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationControllerTest.java	Fri Aug 31 10:12:56 2012 +0200
@@ -42,6 +42,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.times;
 
 import org.junit.After;
 import org.junit.Before;
@@ -74,7 +75,7 @@
     private ThreadInformationController controller;
     
     private ApplicationService appService;
-    
+        
     @Before
     public void setUp() {
         ApplicationContextUtil.resetApplicationContext();
@@ -125,13 +126,15 @@
         
         VmRef ref = mock(VmRef.class);
         
-        ThreadCollectorFactory collectorFactory = mock(ThreadCollectorFactory.class); 
+        ThreadCollectorFactory collectorFactory = mock(ThreadCollectorFactory.class);
+        ThreadCollector collector = mock(ThreadCollector.class);
+        when(collectorFactory.getCollector(ref)).thenReturn(collector);
         
         controller = new ThreadInformationController(ref, appService, collectorFactory, viewFactory);
     }
     
     @Test
-    public void liveRecodingKeySet() {
+    public void verifyLiveRecording() {
         
         ActionListener<ThreadView.ThreadAction> threadActionListener;
         ArgumentCaptor<ActionListener> viewArgumentCaptor = ArgumentCaptor.forClass(ActionListener.class);
@@ -139,35 +142,39 @@
         
         VmRef ref = mock(VmRef.class);
         when(ref.getStringID()).thenReturn("42");
+                
+        ThreadCollector collector = mock(ThreadCollector.class);
+        when(collector.isHarvesterCollecting()).thenReturn(false).thenReturn(true);
+        when(collector.startHarvester()).thenReturn(true);
+        when(collector.stopHarvester()).thenReturn(true).thenReturn(false);
+
         ThreadCollectorFactory collectorFactory = mock(ThreadCollectorFactory.class);
-        
-        ThreadCollector collector = mock(ThreadCollector.class);
         when(collectorFactory.getCollector(ref)).thenReturn(collector);
         
         ApplicationCache cache = mock(ApplicationCache.class);
         appService = mock(ApplicationService.class);
         when(appService.getApplicationCache()).thenReturn(cache);
-        when(cache.getAttribute(anyString())).thenReturn(false).thenReturn(true);
         
         controller = new ThreadInformationController(ref, appService, collectorFactory, viewFactory);
-        assertEquals("thread-live-recording-42", controller.CACHE_RECORDING_KEY);
         
-        verify(cache).getAttribute("thread-live-recording-42");
-        verify(view).setRecording(false);
+        verify(collector).isHarvesterCollecting();
+        verify(view, times(1)).setRecording(false, false);
         
         threadActionListener = viewArgumentCaptor.getValue();
         threadActionListener.actionPerformed(new ActionEvent<>(view, ThreadView.ThreadAction.START_LIVE_RECORDING));
         
-        verify(cache).addAttribute("thread-live-recording-42", true);
+        verify(view, times(1)).setRecording(false, false);
         verify(collector).startHarvester();
         
         threadActionListener.actionPerformed(new ActionEvent<>(view, ThreadView.ThreadAction.STOP_LIVE_RECORDING));
-        verify(cache).addAttribute("thread-live-recording-42", false);
+        
         verify(collector).stopHarvester();        
+        verify(view, times(1)).setRecording(false, false);
         
-        // check that the value indeed persist across sessions
-        controller = new ThreadInformationController(ref, appService, collectorFactory, viewFactory);
-        verify(view).setRecording(true);
+        threadActionListener.actionPerformed(new ActionEvent<>(view, ThreadView.ThreadAction.STOP_LIVE_RECORDING));
+        
+        verify(collector, times(2)).stopHarvester();        
+        verify(view, times(1)).setRecording(true, false);
     }
     
     @Test
--- a/thread/client-swing/src/main/java/com/redhat/thermostat/thread/client/swing/impl/SwingThreadView.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/client-swing/src/main/java/com/redhat/thermostat/thread/client/swing/impl/SwingThreadView.java	Fri Aug 31 10:12:56 2012 +0200
@@ -40,6 +40,7 @@
 import java.awt.event.ItemEvent;
 import java.awt.event.ItemListener;
 
+import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 import javax.swing.JTabbedPane;
 import javax.swing.SwingUtilities;
@@ -65,6 +66,8 @@
     
     private static final Translate t = LocaleResources.createLocalizer();
 
+    private boolean skipNotification = false;
+    
     public SwingThreadView() {
         
         panel = new ThreadMainPanel();
@@ -92,6 +95,9 @@
         {
             @Override
             public void itemStateChanged(ItemEvent e) {
+                
+                if (skipNotification) return;
+                
                 ThreadAction action = null;
                 if (e.getStateChange() == ItemEvent.SELECTED) {
                     action = ThreadAction.START_LIVE_RECORDING;
@@ -130,11 +136,13 @@
     }
     
     @Override
-    public void setRecording(final boolean recording) {
+    public void setRecording(final boolean recording, final boolean notify) {
         SwingUtilities.invokeLater(new Runnable() {
             @Override
             public void run() {
+                if (!notify) skipNotification = true;
                 timelinePanel.getRecordButton().setSelected(recording);
+                if (!notify) skipNotification = false;
             }
         });
     }
@@ -149,8 +157,13 @@
         });
     }
     
-    public void setLiveThreads(String liveThreads) {
-        timelinePanel.getLiveThreads().setText(liveThreads);
+    public void setLiveThreads(final String liveThreads) {
+        SwingUtilities.invokeLater(new Runnable() {
+            @Override
+            public void run() {
+                timelinePanel.getLiveThreads().setText(liveThreads);
+            }
+        });
     };
     
     @Override
@@ -179,4 +192,14 @@
     public ThreadTableView createThreadTableView() {
         return threadTable;
     }
+    
+    @Override
+    public void displayWarning(final String warning) {
+        SwingUtilities.invokeLater(new Runnable() {
+            @Override
+            public void run() {
+                JOptionPane.showMessageDialog(panel.getParent(), warning, "", JOptionPane.WARNING_MESSAGE);
+            }
+        });
+    }
 }
--- a/thread/collector/src/main/java/com/redhat/thermostat/thread/collector/HarvesterCommand.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/collector/HarvesterCommand.java	Fri Aug 31 10:12:56 2012 +0200
@@ -5,6 +5,7 @@
     START,
     STOP,
     VM_CAPS,
+    IS_COLLECTING,
     
     AGENT_ID,
     VM_ID;
--- a/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java	Fri Aug 31 10:12:53 2012 +0200
+++ b/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java	Fri Aug 31 10:12:56 2012 +0200
@@ -86,7 +86,14 @@
             result = saveVmCaps(vmId, agentId);
             break;
         }
+        case IS_COLLECTING: {
+            // this is blocking too
+            String vmId = request.getParameter(HarvesterCommand.VM_ID.name());
+            // FIXME: this need to be replaced when we support response parameters
+            return isCollecting(vmId) ? new Response(ResponseType.OK) : new Response(ResponseType.NOK);
+        }        
         default:
+            result = false;
             break;
         }
         
@@ -97,6 +104,14 @@
         }
     }
     
+    private boolean isCollecting(String vmId) {
+        Harvester harvester = connectors.get(vmId);
+        if (harvester == null) {
+            return false;
+        }
+        return harvester.isConnected();
+    }
+    
     private boolean startHarvester(String vmId, String agentId) {
         Harvester harvester = getHarvester(vmId, agentId);
         return harvester.start();