Mercurial > hg > thermostat
changeset 2310:04c4c802a499
Reduce number of queries in HostMonitorAction
Reviewed-by: neugens, jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-May/018899.html
line wrap: on
line diff
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/VmRef.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/VmRef.java Tue May 24 10:39:27 2016 -0400 @@ -36,6 +36,8 @@ package com.redhat.thermostat.storage.core; +import com.redhat.thermostat.storage.model.VmInfo; + public class VmRef implements Ref { private final HostRef hostRef; @@ -50,6 +52,13 @@ this.name = name; } + public VmRef(HostRef hostRef, VmInfo vmInfo) { + this.hostRef = hostRef; + this.id = vmInfo.getVmId(); + this.pid = vmInfo.getVmPid(); + this.name = vmInfo.getMainClass(); + } + @Override public String toString() { return name;
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java Tue May 24 10:39:27 2016 -0400 @@ -82,6 +82,9 @@ /** @return information on all known VMs */ List<VmInfo> getAllVmInfos(); + /** @return information on all known VMs monitored by give agent */ + List<VmInfo> getAllVmInfosForAgent(AgentId agentId); + /** @return {@code null} if no information can be found */ VmInfo getVmInfo(VmId id);
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/DAOImplStatementDescriptorRegistration.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/DAOImplStatementDescriptorRegistration.java Tue May 24 10:39:27 2016 -0400 @@ -69,7 +69,7 @@ daoDescs.add(NetworkInterfaceInfoDAOImpl.QUERY_NETWORK_INFO); daoDescs.add(NetworkInterfaceInfoDAOImpl.DESC_REPLACE_NETWORK_INFO); daoDescs.add(NetworkInterfaceInfoDAOImpl.AGGREGATE_COUNT_ALL_NETWORK_INTERFACES); - daoDescs.add(VmInfoDAOImpl.QUERY_ALL_VMS_FOR_HOST); + daoDescs.add(VmInfoDAOImpl.QUERY_ALL_VMS_FOR_AGENT); daoDescs.add(VmInfoDAOImpl.QUERY_ALL_VMS); daoDescs.add(VmInfoDAOImpl.QUERY_VM_INFO); daoDescs.add(VmInfoDAOImpl.AGGREGATE_COUNT_ALL_VMS);
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java Tue May 24 10:39:27 2016 -0400 @@ -69,7 +69,7 @@ + vmInfoCategory.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" + Key.VM_ID.getName() + "' = ?s LIMIT 1"; - static final String QUERY_ALL_VMS_FOR_HOST = "QUERY " + static final String QUERY_ALL_VMS_FOR_AGENT = "QUERY " + vmInfoCategory.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s"; @@ -167,7 +167,7 @@ public Collection<VmRef> getVMs(HostRef host) { AgentId agentId = new AgentId(host.getAgentId()); - Collection<VmInfo> vmInfos = getAllVmInfosForHost(agentId); + Collection<VmInfo> vmInfos = getAllVmInfosForAgent(agentId); if (vmInfos.equals(Collections.emptyList())) { return Collections.emptyList(); } @@ -198,7 +198,7 @@ @Override public Set<VmId> getVmIds(AgentId agentId) { Set<VmId> vmIds = new HashSet<>(); - Collection<VmInfo> vmInfos = getAllVmInfosForHost(agentId); + Collection<VmInfo> vmInfos = getAllVmInfosForAgent(agentId); for (VmInfo vmInfo : vmInfos) { vmIds.add(new VmId(vmInfo.getVmId())); } @@ -206,9 +206,10 @@ return vmIds; } - private Collection<VmInfo> getAllVmInfosForHost(final AgentId agentId) { + @Override + public List<VmInfo> getAllVmInfosForAgent(final AgentId agentId) { return executeQuery( - new AbstractDaoQuery<VmInfo>(storage, vmInfoCategory, QUERY_ALL_VMS_FOR_HOST) { + new AbstractDaoQuery<VmInfo>(storage, vmInfoCategory, QUERY_ALL_VMS_FOR_AGENT) { @Override public PreparedStatement<VmInfo> customize(PreparedStatement<VmInfo> preparedStatement) { preparedStatement.setString(0, agentId.get());
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/monitor/internal/HostMonitorAction.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/monitor/internal/HostMonitorAction.java Tue May 24 10:39:27 2016 -0400 @@ -38,8 +38,10 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.List; import com.redhat.thermostat.common.ActionNotifier; +import com.redhat.thermostat.storage.core.AgentId; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.VmInfoDAO; @@ -72,12 +74,12 @@ @Override protected Collection<VmRef> getNewReferences() { - Collection<VmRef> vms = vmsDao.getVMs(host); + List<VmInfo> vms = vmsDao.getAllVmInfosForAgent(new AgentId(host.getAgentId())); Collection<VmRef> livingVMS = new ArrayList<>(); - for (VmRef vm : vms) { - VmInfo vmInfo = vmsDao.getVmInfo(vm); + for (VmInfo vmInfo : vms) { if (vmInfo.isAlive()) { - livingVMS.add(vm); + VmRef vmRef = new VmRef(host, vmInfo); + livingVMS.add(vmRef); } } return livingVMS;
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java Tue May 24 10:39:27 2016 -0400 @@ -39,7 +39,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -49,6 +48,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; @@ -68,7 +68,6 @@ import com.redhat.thermostat.storage.core.Storage; import com.redhat.thermostat.storage.core.VmId; import com.redhat.thermostat.storage.core.VmRef; -import com.redhat.thermostat.storage.dao.DAOException; import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; import com.redhat.thermostat.storage.model.VmInfo; @@ -120,7 +119,7 @@ String expectedVmInfo = "QUERY vm-info WHERE 'agentId' = ?s AND 'vmId' = ?s LIMIT 1"; assertEquals(expectedVmInfo, VmInfoDAOImpl.QUERY_VM_INFO); String expectedVmInfoAll = "QUERY vm-info WHERE 'agentId' = ?s"; - assertEquals(expectedVmInfoAll, VmInfoDAOImpl.QUERY_ALL_VMS_FOR_HOST); + assertEquals(expectedVmInfoAll, VmInfoDAOImpl.QUERY_ALL_VMS_FOR_AGENT); String expectedAllVms = "QUERY vm-info"; assertEquals(expectedAllVms, VmInfoDAOImpl.QUERY_ALL_VMS); String aggregateAllVms = "QUERY-COUNT vm-info"; @@ -272,7 +271,7 @@ } @Test - public void testgetVmIdsWithSingleVM() throws DescriptorParsingException, StatementExecutionException { + public void testGetVmIdsWithSingleVM() throws DescriptorParsingException, StatementExecutionException { Storage storage = setupStorageForSingleVM(); VmInfoDAO dao = new VmInfoDAOImpl(storage); AgentId agentId = new AgentId("123"); @@ -324,6 +323,28 @@ assertVmIdCollection(vmIds, new VmId("vmId1"), new VmId("vmId2")); } + + @Test + public void testGetAllVmInfos() throws DescriptorParsingException, StatementExecutionException { + Storage storage = setupStorageForMultiVM(); + VmInfoDAO dao = new VmInfoDAOImpl(storage); + + List<VmInfo> vmInfos = dao.getAllVmInfos(); + + assertTrue(vmInfos.size() == 2); + } + + @Test + public void testGetAllVmInfosForAgent() throws DescriptorParsingException, StatementExecutionException { + Storage storage = setupStorageForMultiVM(); + VmInfoDAO dao = new VmInfoDAOImpl(storage); + AgentId agentId = new AgentId("456"); + + List<VmInfo> vmInfos = dao.getAllVmInfosForAgent(agentId); + + assertTrue(vmInfos.size() == 2); + } + private Storage setupStorageForMultiVM() throws DescriptorParsingException, StatementExecutionException { VmInfo vm1 = new VmInfo(); vm1.setVmId("vmId1");
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/monitor/internal/HostMonitorActionTest.java Tue May 24 09:17:54 2016 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/monitor/internal/HostMonitorActionTest.java Tue May 24 10:39:27 2016 -0400 @@ -45,7 +45,9 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.List; +import com.redhat.thermostat.storage.core.AgentId; import org.junit.Before; import org.junit.Test; @@ -75,65 +77,62 @@ HostRef host = new HostRef("01", "01"); - Collection<VmRef> currentVMs = new ArrayList<>(); - VmRef a = new VmRef(host, "0", 0, "a"); - VmRef b = new VmRef(host, "1", 1, "b"); - VmRef c = new VmRef(host, "2", 2, "c"); - VmRef d = new VmRef(host, "3", 3, "d"); - VmRef e = new VmRef(host, "4", 3, "e"); + List<VmInfo> currentVMs = new ArrayList<>(); VmInfo info_a = mock(VmInfo.class); when(info_a.isAlive()).thenReturn(true); + when(info_a.getVmId()).thenReturn("0"); + when(info_a.getVmPid()).thenReturn(0); + when(info_a.getVmName()).thenReturn("0"); VmInfo info_b = mock(VmInfo.class); when(info_b.isAlive()).thenReturn(true); + when(info_b.getVmId()).thenReturn("1"); + when(info_b.getVmPid()).thenReturn(1); + when(info_b.getVmName()).thenReturn("1"); VmInfo info_c = mock(VmInfo.class); when(info_c.isAlive()).thenReturn(true); + when(info_c.getVmId()).thenReturn("2"); + when(info_c.getVmPid()).thenReturn(2); + when(info_c.getVmName()).thenReturn("2"); VmInfo info_d = mock(VmInfo.class); when(info_d.isAlive()).thenReturn(true); + when(info_d.getVmId()).thenReturn("3"); + when(info_d.getVmPid()).thenReturn(3); + when(info_d.getVmName()).thenReturn("3"); VmInfo info_e = mock(VmInfo.class); when(info_e.isAlive()).thenReturn(false); - - when(vmsDAO.getVmInfo(a)).thenReturn(info_a); - when(vmsDAO.getVmInfo(b)).thenReturn(info_b); - when(vmsDAO.getVmInfo(c)).thenReturn(info_c); - when(vmsDAO.getVmInfo(d)).thenReturn(info_d); - when(vmsDAO.getVmInfo(e)).thenReturn(info_e); + when(info_e.getVmId()).thenReturn("4"); + when(info_e.getVmPid()).thenReturn(4); + when(info_e.getVmName()).thenReturn("4"); - currentVMs.add(a); - currentVMs.add(b); - currentVMs.add(c); - currentVMs.add(d); - currentVMs.add(e); + currentVMs.add(info_a); + currentVMs.add(info_b); + currentVMs.add(info_c); + currentVMs.add(info_d); + currentVMs.add(info_e); - when(vmsDAO.getVMs(host)).thenReturn(currentVMs); + when(vmsDAO.getAllVmInfosForAgent(any(AgentId.class))).thenReturn(currentVMs); // the first result is to be notified of all those vms HostMonitorAction action = new HostMonitorAction(notifier, vmsDAO, host); action.run(); - verify(notifier).fireAction(Action.VM_ADDED, a); - verify(notifier).fireAction(Action.VM_ADDED, b); - verify(notifier).fireAction(Action.VM_ADDED, c); - verify(notifier).fireAction(Action.VM_ADDED, d); - - verify(notifier,times(0)).fireAction(Action.VM_ADDED, e); - verify(notifier, times(0)).fireAction(Action.VM_REMOVED, eq(any(VmRef.class))); + verify(notifier, times(4)).fireAction(eq(Action.VM_ADDED), any(VmRef.class)); + verify(notifier, times(0)).fireAction(eq(Action.VM_REMOVED), any(VmRef.class)); // now remove one vm from each side - currentVMs.remove(b); - currentVMs.remove(c); + currentVMs.remove(info_b); + currentVMs.remove(info_c); action.run(); - verify(notifier).fireAction(Action.VM_REMOVED, b); - verify(notifier).fireAction(Action.VM_REMOVED, c); - - verify(notifier, times(0)).fireAction(Action.VM_ADDED, eq(any(VmRef.class))); + verify(notifier, times(4)).fireAction(eq(Action.VM_ADDED), any(VmRef.class)); + verify(notifier, times(2)).fireAction(eq(Action.VM_REMOVED), any(VmRef.class)); when(info_a.isAlive()).thenReturn(false); @@ -142,11 +141,8 @@ action.run(); - verify(notifier,times(1)).fireAction(Action.VM_ADDED, e); - verify(notifier,times(1)).fireAction(Action.VM_REMOVED, a); - - verify(notifier, times(0)).fireAction(Action.VM_ADDED, eq(any(VmRef.class))); - verify(notifier, times(0)).fireAction(Action.VM_REMOVED, eq(any(VmRef.class))); + verify(notifier, times(5)).fireAction(eq(Action.VM_ADDED), any(VmRef.class)); + verify(notifier, times(3)).fireAction(eq(Action.VM_REMOVED), any(VmRef.class)); } }