Mercurial > hg > release > thermostat-0.7

changeset 1052:dc66dff085a1

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Make thread backend activate and deactivate correctly Reviewed-by: neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-March/006222.html
author Omair Majid <omajid@redhat.com>
date Tue, 02 Apr 2013 14:30:12 -0400
parents 77607020a4d3
children b288a8a06f03
files thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadBackend.java thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadBackendTest.java thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadHarvesterTest.java
diffstat 4 files changed, 120 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadBackend.java	Fri Mar 22 19:57:11 2013 +0100
+++ b/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadBackend.java	Tue Apr 02 14:30:12 2013 -0400
@@ -36,6 +36,9 @@
 
 package com.redhat.thermostat.thread.harvester;
 
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
 import java.util.logging.Logger;
 
 import com.redhat.thermostat.agent.VmStatusListener;
@@ -54,6 +57,7 @@
 
     private boolean active = false;
     private VmStatusListenerRegistrar vmListener;
+    private final List<Integer> pidsToHarvestOnEnable = new ArrayList<>();
 
     public ThreadBackend(Version version, VmStatusListenerRegistrar registrar, ReceiverRegistry registry, ThreadHarvester harvester) {
         super("VM Thread Backend", "Gathers thread information about a JVM", "Red Hat, Inc", version.getVersionNumber());
@@ -73,9 +77,17 @@
         if (active) {
             return true;
         }
+
+        // bring back all harvesters that were active
+        Iterator<Integer> iter = pidsToHarvestOnEnable.iterator();
+        while (iter.hasNext()) {
+            harvester.startHarvester(String.valueOf(iter.next()));
+            iter.remove();
+        }
+
         vmListener.register(this);
         registry.registerReceiver(harvester);
-        // FIXME enable harvester
+
         active = true;
         return true;
     }
@@ -87,7 +99,10 @@
         }
         vmListener.unregister(this);
         registry.unregisterReceivers();
-        // FIXME disable harvester
+
+        // stop all currently active harvesters
+        pidsToHarvestOnEnable.addAll(harvester.stopAndRemoveAllHarvesters());
+
         active = false;
         return true;
     }
@@ -108,7 +123,6 @@
             break;
         case VM_STOPPED:
             harvester.stopHarvester(vmId);
-            harvester.addThreadHarvestingStatus(vmId);
             break;
         default:
             logger.warning("Unexpected VM state: " + newStatus);
--- a/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java	Fri Mar 22 19:57:11 2013 +0100
+++ b/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java	Tue Apr 02 14:30:12 2013 -0400
@@ -36,8 +36,10 @@
 
 package com.redhat.thermostat.thread.harvester;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.concurrent.ScheduledExecutorService;
@@ -71,11 +73,22 @@
         this.clock = clock;
     }
 
+    /**
+     * Set the new implementation of thread DAO to be used as stroage
+     * @param dao
+     */
     public void setThreadDao(ThreadDao dao) {
-        // a new ThreadDao has appeared, stop everything using the old implementation
-        removeAllHarvesters();
+        // stop everything using the old implementation
+        List<Integer> saved = new ArrayList<>();
+        if (this.dao != null) {
+            saved.addAll(stopAndRemoveAllHarvesters());
+        }
+
         this.dao = dao;
-        // TODO maybe bring back the old stuff using the new ThreadDao?
+        // re-enable all existing harvesters
+        for (Integer pid : saved) {
+            startHarvester(String.valueOf(pid));
+        }
     }
     
     @Override
@@ -109,7 +122,12 @@
             return new Response(ResponseType.ERROR);
         }
     }
-    
+
+    /**
+     * Attaches and starts a harvester to the given PID.
+     * <p>
+     * Saves current harvesting status to storage.
+     */
     public boolean startHarvester(String vmId) {
         Harvester harvester = getHarvester(vmId);
         boolean result = harvester.start();
@@ -123,16 +141,23 @@
         Harvester harvester = getHarvester(vmId);
         return harvester.saveVmCaps();
     }
-    
+
+    /**
+     * Stops and detaches a harvester from the given PID.
+     * <p>
+     * Saves current harvesting status to storage.
+     */
     public boolean stopHarvester(String vmId) {
         Harvester harvester = connectors.get(vmId);
+        boolean result = true;
         if (harvester != null) {
-            return harvester.stop();
+            result = harvester.stop();
         }
         updateHarvestingStatus(Integer.valueOf(vmId), false);
         return true;
     }
 
+    /** Save current status to storage */
     public void addThreadHarvestingStatus(String pid) {
         updateHarvestingStatus(Integer.valueOf(pid), connectors.containsKey(pid));
     }
@@ -144,23 +169,36 @@
         status.setHarvesting(harvesting);
         dao.saveHarvestingStatus(status);
     }
-    Harvester getHarvester(String vmId) {
+
+    private Harvester getHarvester(String vmId) {
         Harvester harvester = connectors.get(vmId);
         if (harvester == null) {
-            harvester = new Harvester(dao, executor, vmId);
+            harvester = createHarvester(vmId);
             connectors.put(vmId, harvester);
         }
         
         return harvester;
     }
 
-    private void removeAllHarvesters() {
+    Harvester createHarvester(String vmId) {
+        return new Harvester(dao, executor, vmId);
+    }
+
+    /**
+     * Returns a list of PIDs which the harvester stopped harvesting
+     */
+    public List<Integer> stopAndRemoveAllHarvesters() {
+        List<Integer> result = new ArrayList<>();
         Iterator<Entry<String, Harvester>> iter = connectors.entrySet().iterator();
         while (iter.hasNext()) {
             Entry<String, Harvester> entry = iter.next();
+            int pid = Integer.valueOf(entry.getKey());
             entry.getValue().stop();
+            updateHarvestingStatus(pid, false);
             iter.remove();
+            result.add(pid);
         }
+        return result;
     }
 
     private boolean allRequirementsAvailable() {
--- a/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadBackendTest.java	Fri Mar 22 19:57:11 2013 +0100
+++ b/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadBackendTest.java	Tue Apr 02 14:30:12 2013 -0400
@@ -43,6 +43,8 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.Arrays;
+
 import org.junit.Before;
 import org.junit.Test;
 
@@ -88,6 +90,19 @@
 
         verify(vmListenerRegistrar).unregister(backend);
         verify(receiverRegistry).unregisterReceivers();
+        verify(threadHarvester).stopAndRemoveAllHarvesters();
+    }
+
+    @Test
+    public void testActivateAfterDeactivate() {
+        when(threadHarvester.stopAndRemoveAllHarvesters()).thenReturn(Arrays.asList(1));
+
+        assertTrue(backend.activate());
+        assertTrue(backend.deactivate());
+        assertTrue(backend.activate());
+        assertTrue(backend.isActive());
+
+        verify(threadHarvester).startHarvester("1");
     }
 
     @Test
@@ -103,6 +118,5 @@
         backend.vmStatusChanged(Status.VM_STOPPED, 10);
 
         verify(threadHarvester).stopHarvester("10");
-        verify(threadHarvester).addThreadHarvestingStatus("10");
     }
 }
--- a/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadHarvesterTest.java	Fri Mar 22 19:57:11 2013 +0100
+++ b/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadHarvesterTest.java	Tue Apr 02 14:30:12 2013 -0400
@@ -64,8 +64,9 @@
         ThreadDao dao = mock(ThreadDao.class);
         Request request = mock(Request.class);
         
-        final boolean[] getHarvesterCalled = new boolean[1];
+        final boolean[] createHarvesterCalled = new boolean[1];
         final Harvester harverster = mock(Harvester.class);
+        when(harverster.start()).thenReturn(true);
         
         ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
         
@@ -76,9 +77,9 @@
         
         ThreadHarvester threadHarvester = new ThreadHarvester(executor) {
             @Override
-            Harvester getHarvester(String vmId) {
+            Harvester createHarvester(String vmId) {
                 
-                getHarvesterCalled[0] = true;
+                createHarvesterCalled[0] = true;
                 assertEquals("42", vmId);
                 
                 return harverster;
@@ -93,7 +94,7 @@
         assertEquals(HarvesterCommand.class.getName(), values.get(0));
         assertEquals(HarvesterCommand.VM_ID.name(), values.get(1));
         
-        assertTrue(getHarvesterCalled[0]);
+        assertTrue(createHarvesterCalled[0]);
         
         verify(harverster).start();
     }
@@ -132,14 +133,14 @@
         ScheduledExecutorService executor = mock(ScheduledExecutorService.class);
         ThreadDao dao = mock(ThreadDao.class);
         
-        final boolean[] getHarvesterCalled = new boolean[1];
+        final boolean[] createHarvesterCalled = new boolean[1];
         final Harvester harverster = mock(Harvester.class);
         
         ThreadHarvester threadHarvester = new ThreadHarvester(executor) {
             @Override
-            Harvester getHarvester(String vmId) {
+            Harvester createHarvester(String vmId) {
                 
-                getHarvesterCalled[0] = true;
+                createHarvesterCalled[0] = true;
                 assertEquals("42", vmId);
                 
                 return harverster;
@@ -148,7 +149,7 @@
         threadHarvester.setThreadDao(dao);
         threadHarvester.saveVmCaps("42");
         
-        assertTrue(getHarvesterCalled[0]);
+        assertTrue(createHarvesterCalled[0]);
         
         verify(harverster).saveVmCaps();
     }    
@@ -183,5 +184,37 @@
         assertEquals(false, status.isHarvesting());
         assertEquals(1, status.getTimeStamp());
     }
+
+    @Test
+    public void testStopAndRemoveAll() {
+        ScheduledExecutorService executor = mock(ScheduledExecutorService.class);
+        Clock clock = mock(Clock.class);
+        when(clock.getRealTimeMillis()).thenReturn(1l);
+        ThreadDao dao = mock(ThreadDao.class);
+
+        final boolean[] createHarvesterCalled = new boolean[1];
+        final Harvester javaHarvester = mock(Harvester.class);
+        when(javaHarvester.start()).thenReturn(true);
+        when(javaHarvester.stop()).thenReturn(true);
+
+        ThreadHarvester harvester = new ThreadHarvester(executor, clock) {
+            @Override
+            Harvester createHarvester(String vmId) {
+
+                createHarvesterCalled[0] = true;
+                assertEquals("42", vmId);
+
+                return javaHarvester;
+            }
+        };
+        harvester.setThreadDao(dao);
+
+        assertTrue(harvester.startHarvester("42"));
+
+        List<Integer> pids = harvester.stopAndRemoveAllHarvesters();
+        assertEquals(1, pids.size());
+        assertEquals(42, (int) pids.get(0));
+
+    }
 }