# HG changeset patch # User Omair Majid # Date 1364927412 14400 # Node ID dc66dff085a1a3e79fb9a72798f9531fc1417809 # Parent 77607020a4d334011ef8ff8657b140a3b38bc4a8 Make thread backend activate and deactivate correctly Reviewed-by: neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-March/006222.html diff -r 77607020a4d3 -r dc66dff085a1 thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadBackend.java --- 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 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 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); diff -r 77607020a4d3 -r dc66dff085a1 thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java --- 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 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. + *

+ * 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. + *

+ * 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 stopAndRemoveAllHarvesters() { + List result = new ArrayList<>(); Iterator> iter = connectors.entrySet().iterator(); while (iter.hasNext()) { Entry 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() { diff -r 77607020a4d3 -r dc66dff085a1 thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadBackendTest.java --- 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"); } } diff -r 77607020a4d3 -r dc66dff085a1 thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadHarvesterTest.java --- 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 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 pids = harvester.stopAndRemoveAllHarvesters(); + assertEquals(1, pids.size()); + assertEquals(42, (int) pids.get(0)); + + } }