# HG changeset patch # User Elliott Baron # Date 1374186829 14400 # Node ID f7961cf1e44073f4610115b4313b050ce0d4f596 # Parent 53d3cab7702569a8c705718fc475aeaa2971594c Convert VM IDs to UUIDs This commit changes VM IDs used in VmRef and VmInfo to a randomly generated UUID string. The VM PIDs are still stored, but are not used to identify a specific VM. The motivation behind this change is that PIDs can be reused by the OS, thus there is a chance we could have a PID that refers to two different VMs (one alive, one dead). There are then security implications if we return the wrong VM, which the client user may not have access to. The VM's PID is still required when dealing with external interfaces like /proc and JMX, but the data collected is placed in storage using the VM's UUID. Additionally, I have cleaned up the API in VmRef. I have removed getIdString from VmRef since we already have a getStringID that does the same thing. I've also changed VmRef.getAgent to getHostRef to more accurately reflect what is returned. Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-July/007242.html diff -r 53d3cab77025 -r f7961cf1e440 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/MongoProcessRunner.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/MongoProcessRunner.java Thu Jul 18 16:31:24 2013 -0400 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/MongoProcessRunner.java Thu Jul 18 18:33:49 2013 -0400 @@ -78,7 +78,7 @@ private DBStartupConfiguration configuration; private boolean isQuiet; - private String pid; + private Integer pid; public MongoProcessRunner(DBStartupConfiguration configuration, boolean quiet) { this.configuration = configuration; @@ -90,13 +90,19 @@ Charset charset = Charset.defaultCharset(); if (pidfile.exists()) { try (BufferedReader reader = Files.newBufferedReader(pidfile.toPath(), charset)) { - pid = reader.readLine(); - if (pid.isEmpty()) { + String line = reader.readLine(); + if (line.isEmpty()) { pid = null; } + else { + pid = Integer.parseInt(line); + } } catch (IOException ex) { logger.log(Level.WARNING, "Exception while reading pid file", ex); pid = null; + } catch (NumberFormatException e) { + logger.log(Level.WARNING, "Mongo PID file does not contain a valid PID", e); + pid = null; } } else { pid = null; @@ -135,9 +141,9 @@ ApplicationException, InvalidConfigurationException { if (isStorageRunning()) { - LocalizedString message = translator.localize(LocaleResources.STORAGE_ALREADY_RUNNING_WITH_PID, pid); + LocalizedString message = translator.localize(LocaleResources.STORAGE_ALREADY_RUNNING_WITH_PID, String.valueOf(pid)); display(message); - throw new StorageAlreadyRunningException(Integer.valueOf(pid), message.getContents()); + throw new StorageAlreadyRunningException(pid, message.getContents()); } String dbVersion = getDBVersion(); @@ -167,7 +173,7 @@ if (status == 0) { display(translator.localize(LocaleResources.SERVER_LISTENING_ON, configuration.getDBConnectionString())); display(translator.localize(LocaleResources.LOG_FILE_AT, configuration.getLogFile().toString())); - display(translator.localize(LocaleResources.PID_IS, pid)); + display(translator.localize(LocaleResources.PID_IS, String.valueOf(pid))); } else { @@ -187,7 +193,7 @@ throw new StorageNotRunningException(message.getContents()); } List commands = new ArrayList<>(Arrays.asList(MONGO_SHUTDOWN_ARGS)); - commands.add(pid); + commands.add(String.valueOf(pid)); LoggedExternalProcess process = new LoggedExternalProcess(commands); int status = process.runAndReturnResult(); diff -r 53d3cab77025 -r f7961cf1e440 agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListener.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -64,9 +64,9 @@ VM_STOPPED, } - // TODO what other information other than pid may be useful for plugins? + // TODO what other information other than vmId and pid may be useful for plugins? - public void vmStatusChanged(Status newStatus, int pid); + public void vmStatusChanged(Status newStatus, String vmId, int pid); } diff -r 53d3cab77025 -r f7961cf1e440 agent/core/src/main/java/com/redhat/thermostat/backend/VmListenerBackend.java --- a/agent/core/src/main/java/com/redhat/thermostat/backend/VmListenerBackend.java Thu Jul 18 16:31:24 2013 -0400 +++ b/agent/core/src/main/java/com/redhat/thermostat/backend/VmListenerBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -117,13 +117,13 @@ return started; } - public void vmStatusChanged(Status newStatus, int pid) { + public void vmStatusChanged(Status newStatus, String vmId, int pid) { switch (newStatus) { case VM_STARTED: /* fall-through */ case VM_ACTIVE: if (getObserveNewJvm()) { - VmUpdateListener listener = createVmListener(pid); + VmUpdateListener listener = createVmListener(vmId, pid); monitor.handleNewVm(listener, pid); } else { logger.log(Level.FINE, "skipping new vm " + pid); @@ -142,10 +142,11 @@ * specified by the pid. This method is called when a new * JVM is started or for JVMs already active when this Backend * was activated. + * @param vmId unique identifier of the JVM * @param pid the process ID of the JVM * @return a new listener for the VM specified by pid */ - protected abstract VmUpdateListener createVmListener(int pid); + protected abstract VmUpdateListener createVmListener(String vmId, int pid); /* * For testing purposes only. diff -r 53d3cab77025 -r f7961cf1e440 agent/core/src/main/java/com/redhat/thermostat/utils/management/internal/MXBeanConnectionPoolImpl.java --- a/agent/core/src/main/java/com/redhat/thermostat/utils/management/internal/MXBeanConnectionPoolImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/agent/core/src/main/java/com/redhat/thermostat/utils/management/internal/MXBeanConnectionPoolImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -91,7 +91,7 @@ static class ConnectorCreator { public MXBeanConnector create(int pid) { - return new MXBeanConnector(String.valueOf(pid)); + return new MXBeanConnector(pid); } } } diff -r 53d3cab77025 -r f7961cf1e440 agent/core/src/main/java/com/redhat/thermostat/utils/management/internal/MXBeanConnector.java --- a/agent/core/src/main/java/com/redhat/thermostat/utils/management/internal/MXBeanConnector.java Thu Jul 18 16:31:24 2013 -0400 +++ b/agent/core/src/main/java/com/redhat/thermostat/utils/management/internal/MXBeanConnector.java Thu Jul 18 18:33:49 2013 -0400 @@ -58,21 +58,21 @@ private boolean attached; - private String reference; + private int pid; - public MXBeanConnector(String reference) { - this.reference = reference; + public MXBeanConnector(int pid) { + this.pid = pid; } public MXBeanConnector(VmRef reference) { - this.reference = reference.getStringID(); + this.pid = reference.getPid(); } public synchronized void attach() throws Exception { if (attached) throw new IOException("Already attached"); - vm = VirtualMachine.attach(reference); + vm = VirtualMachine.attach(String.valueOf(pid)); attached = true; Properties props = vm.getAgentProperties(); diff -r 53d3cab77025 -r f7961cf1e440 agent/core/src/test/java/com/redhat/thermostat/backend/VmListenerBackendTest.java --- a/agent/core/src/test/java/com/redhat/thermostat/backend/VmListenerBackendTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/agent/core/src/test/java/com/redhat/thermostat/backend/VmListenerBackendTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -55,6 +55,8 @@ import com.redhat.thermostat.backend.internal.VmMonitor; public class VmListenerBackendTest { + private static final String VM_ID = "vmId"; + private static final int VM_PID = 1; private VmListenerBackend backend; private VmStatusListenerRegistrar registrar; @@ -114,46 +116,39 @@ @Test public void testNewVM() { - final int VM_PID = 1; - // Should be no response if not observing new jvm. backend.setObserveNewJvm(false); - backend.vmStatusChanged(Status.VM_STARTED, VM_PID); + backend.vmStatusChanged(Status.VM_STARTED, VM_ID, VM_PID); verify(monitor, times(0)).handleNewVm(same(listener), same(VM_PID)); backend.setObserveNewJvm(true); - backend.vmStatusChanged(Status.VM_STARTED, VM_PID); + backend.vmStatusChanged(Status.VM_STARTED, VM_ID, VM_PID); verify(monitor).handleNewVm(listener, VM_PID); } @Test public void testAlreadyRunningVM() { - final int VM_PID = 1; - backend.setObserveNewJvm(true); - backend.vmStatusChanged(Status.VM_ACTIVE, VM_PID); + backend.vmStatusChanged(Status.VM_ACTIVE, VM_ID, VM_PID); verify(monitor).handleNewVm(listener, VM_PID); } @Test public void testStoppedVM() throws MonitorException, URISyntaxException { - final int VM_PID = 1; - backend.setObserveNewJvm(true); - backend.vmStatusChanged(Status.VM_STARTED, VM_PID); - backend.vmStatusChanged(Status.VM_STOPPED, VM_PID); + backend.vmStatusChanged(Status.VM_STARTED, VM_ID, VM_PID); + backend.vmStatusChanged(Status.VM_STOPPED, VM_ID, VM_PID); verify(monitor).handleStoppedVm(VM_PID); } @Test public void testDeactivateUnregistersListener() throws URISyntaxException, MonitorException { - final int VM_PID = 1; backend.activate(); backend.setObserveNewJvm(true); - backend.vmStatusChanged(Status.VM_STARTED, VM_PID); + backend.vmStatusChanged(Status.VM_STARTED, VM_ID, VM_PID); backend.deactivate(); verify(monitor).removeVmListeners(); } @@ -171,7 +166,7 @@ } @Override - protected VmUpdateListener createVmListener(int pid) { + protected VmUpdateListener createVmListener(String vmId, int pid) { return listener; } diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/main/java/com/redhat/thermostat/client/cli/HostVMArguments.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/HostVMArguments.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/HostVMArguments.java Thu Jul 18 18:33:49 2013 -0400 @@ -67,22 +67,12 @@ } else { host = new HostRef(hostId, "dummy"); } - try { - int parsedVmId = parseVmId(vmId); - vm = new VmRef(host, parsedVmId, "dummy"); - } catch (CommandException ce) { - if (vmRequired) { - throw ce; - } + if (vmId == null && vmRequired) { + throw new CommandException(tr.localize(LocaleResources.VMID_REQUIRED_MESSAGE)); + } else if (vmId == null) { vm = null; - } - } - - private int parseVmId(String vmId) throws CommandException { - try { - return Integer.parseInt(vmId); - } catch (NumberFormatException ex) { - throw new CommandException(tr.localize(LocaleResources.INVALID_VMID_MESSAGE, vmId), ex); + } else { + vm = new VmRef(host, vmId, -1, "dummy"); } } diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java Thu Jul 18 18:33:49 2013 -0400 @@ -60,6 +60,7 @@ COMMAND_SHELL_IO_EXCEPTION, + VM_INFO_VM_ID, VM_INFO_PROCESS_ID, VM_INFO_START_TIME, VM_INFO_STOP_TIME, @@ -74,6 +75,7 @@ COLUMN_HEADER_HOST_ID, COLUMN_HEADER_HOST, COLUMN_HEADER_VM_ID, + COLUMN_HEADER_VM_PID, COLUMN_HEADER_VM_NAME, COLUMN_HEADER_VM_STATUS, COLUMN_HEADER_TIME, @@ -83,7 +85,7 @@ VM_STATUS_DEAD, HOSTID_REQUIRED_MESSAGE, - INVALID_VMID_MESSAGE, + VMID_REQUIRED_MESSAGE, ; static final String RESOURCE_BUNDLE = "com.redhat.thermostat.client.cli.strings"; diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java Thu Jul 18 18:33:49 2013 -0400 @@ -110,6 +110,7 @@ VmInfo vmInfo = vmsDAO.getVmInfo(vm); TableRenderer table = new TableRenderer(2); + table.printLine(translator.localize(LocaleResources.VM_INFO_VM_ID).getContents(), vmInfo.getVmId()); table.printLine(translator.localize(LocaleResources.VM_INFO_PROCESS_ID).getContents(), String.valueOf(vmInfo.getVmPid())); table.printLine(translator.localize(LocaleResources.VM_INFO_START_TIME).getContents(), new Date(vmInfo.getStartTimeStamp()).toString()); if (vmInfo.isAlive()) { diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMListFormatter.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMListFormatter.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMListFormatter.java Thu Jul 18 18:33:49 2013 -0400 @@ -50,6 +50,7 @@ private static final String HOST_ID = translator.localize(LocaleResources.COLUMN_HEADER_HOST_ID).getContents(); private static final String HOST = translator.localize(LocaleResources.COLUMN_HEADER_HOST).getContents(); private static final String VM_ID = translator.localize(LocaleResources.COLUMN_HEADER_VM_ID).getContents(); + private static final String VM_PID = translator.localize(LocaleResources.COLUMN_HEADER_VM_PID).getContents(); private static final String VM_NAME = translator.localize(LocaleResources.COLUMN_HEADER_VM_NAME).getContents(); private static final String VM_STATUS = translator.localize(LocaleResources.COLUMN_HEADER_VM_STATUS).getContents(); @@ -59,7 +60,7 @@ private TableRenderer tableRenderer; VMListFormatter() { - tableRenderer = new TableRenderer(5); + tableRenderer = new TableRenderer(6); printHeader(); } @@ -72,19 +73,20 @@ } private void printHeader() { - printLine(HOST_ID, HOST, VM_ID, VM_STATUS, VM_NAME); + printLine(HOST_ID, HOST, VM_ID, VM_PID, VM_STATUS, VM_NAME); } private void printVM(VmRef vm, VmInfo info) { - printLine(vm.getAgent().getAgentId(), - vm.getAgent().getHostName(), - vm.getId().toString(), + printLine(vm.getHostRef().getAgentId(), + vm.getHostRef().getHostName(), + vm.getVmId(), + vm.getPid().toString(), info.isAlive() ? STATUS_ALIVE : STATUS_DEAD, vm.getName()); } - private void printLine(String hostId, String host, String vmId, String status, String vmName) { - tableRenderer.printLine(hostId, host, vmId, status, vmName); + private void printLine(String hostId, String host, String vmId, String pid, String status, String vmName) { + tableRenderer.printLine(hostId, host, vmId, pid, status, vmName); } } diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties --- a/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties Thu Jul 18 18:33:49 2013 -0400 @@ -18,6 +18,7 @@ COMMAND_SHELL_IO_EXCEPTION = IOException caught during Thermostat shell session. +VM_INFO_VM_ID = VM ID: VM_INFO_PROCESS_ID = Process ID: VM_INFO_START_TIME = Start time: VM_INFO_STOP_TIME = Stop time: @@ -32,6 +33,7 @@ COLUMN_HEADER_HOST_ID = HOST_ID COLUMN_HEADER_HOST = HOST COLUMN_HEADER_VM_ID = VM_ID +COLUMN_HEADER_VM_PID = VM_PID COLUMN_HEADER_VM_NAME = VM_NAME COLUMN_HEADER_VM_STATUS = STATUS COLUMN_HEADER_TIME = TIME @@ -41,4 +43,4 @@ VM_STATUS_DEAD = EXITED HOSTID_REQUIRED_MESSAGE = a hostId is required -INVALID_VMID_MESSAGE = Invalid VM ID: {0} +VMID_REQUIRED_MESSAGE = a vmId is required diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -100,8 +100,9 @@ context.registerService(VmInfoDAO.class, vmsDAO, null); HostRef host1 = new HostRef("123", "h1"); - VmRef vm1 = new VmRef(host1, 1, "n"); - VmInfo vm1Info = new VmInfo(1, 0, 1, "", "", "", "", "", "", "", "", null, null, null, -1, null); + String vmId = "vmId"; + VmRef vm1 = new VmRef(host1, vmId, 1, "n"); + VmInfo vm1Info = new VmInfo(vmId, 1, 0, 1, "", "", "", "", "", "", "", "", null, null, null, -1, null); when(hostsDAO.getHosts()).thenReturn(Arrays.asList(host1)); when(vmsDAO.getVMs(host1)).thenReturn(Arrays.asList(vm1)); when(vmsDAO.getVmInfo(eq(vm1))).thenReturn(vm1Info); @@ -113,8 +114,8 @@ cmd.run(ctx); String output = cmdCtxFactory.getOutput(); - assertEquals("HOST_ID HOST VM_ID STATUS VM_NAME\n" + - "123 h1 1 EXITED n\n", output); + assertEquals("HOST_ID HOST VM_ID VM_PID STATUS VM_NAME\n" + + "123 h1 vmId 1 EXITED n\n", output); } @Test @@ -126,11 +127,11 @@ HostRef host2 = new HostRef("456", "longhostname"); when(hostsDAO.getHosts()).thenReturn(Arrays.asList(host1, host2)); - VmRef vm1 = new VmRef(host1, 1, "n"); - VmRef vm2 = new VmRef(host1, 2, "n1"); - VmRef vm3 = new VmRef(host2, 123456, "longvmname"); + VmRef vm1 = new VmRef(host1, "vm1", 1, "n"); + VmRef vm2 = new VmRef(host1, "vm2", 2, "n1"); + VmRef vm3 = new VmRef(host2, "vm3", 123456, "longvmname"); - VmInfo vmInfo = new VmInfo(1, 0, 1, "", "", "", "", "", "", "", "", null, null, null, -1, null); + VmInfo vmInfo = new VmInfo("vm1", 1, 0, 1, "", "", "", "", "", "", "", "", null, null, null, -1, null); when(vmsDAO.getVMs(host1)).thenReturn(Arrays.asList(vm1, vm2)); when(vmsDAO.getVMs(host2)).thenReturn(Arrays.asList(vm3)); @@ -144,10 +145,10 @@ cmd.run(ctx); String output = cmdCtxFactory.getOutput(); - assertEquals("HOST_ID HOST VM_ID STATUS VM_NAME\n" + - "123 h1 1 EXITED n\n" + - "123 h1 2 EXITED n1\n" + - "456 longhostname 123456 EXITED longvmname\n", output); + assertEquals("HOST_ID HOST VM_ID VM_PID STATUS VM_NAME\n" + + "123 h1 vm1 1 EXITED n\n" + + "123 h1 vm2 2 EXITED n1\n" + + "456 longhostname vm3 123456 EXITED longvmname\n", output); } @Test diff -r 53d3cab77025 -r f7961cf1e440 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -67,6 +67,7 @@ public class VMInfoCommandTest { private static final Translate translator = LocaleResources.createLocalizer(); + private static final String VM_ID = "eafd6db9-9487-41fc-a571-d6a4bfcfbd2f"; private static TimeZone defaultTimezone; @BeforeClass @@ -101,14 +102,14 @@ private void setupDAOs() { HostRef host = new HostRef("123", "dummy"); - vm = new VmRef(host, 234, "dummy"); + vm = new VmRef(host, VM_ID, -1, "dummy"); Calendar start = Calendar.getInstance(); start.set(2012, 5, 7, 15, 32, 0); Calendar end = Calendar.getInstance(); end.set(2013, 10, 1, 1, 22, 0); - VmInfo vmInfo = new VmInfo(234, start.getTimeInMillis(), end.getTimeInMillis(), "vmVersion", "javaHome", "mainClass", "commandLine", "vmName", "vmInfo", "vmVersion", "vmArguments", new HashMap(), new HashMap(), new String[0], 2000, "myUser"); + VmInfo vmInfo = new VmInfo(VM_ID, 234, start.getTimeInMillis(), end.getTimeInMillis(), "vmVersion", "javaHome", "mainClass", "commandLine", "vmName", "vmInfo", "vmVersion", "vmArguments", new HashMap(), new HashMap(), new String[0], 2000, "myUser"); when(vmsDAO.getVmInfo(vm)).thenReturn(vmInfo); - when(vmsDAO.getVmInfo(new VmRef(host, 9876, "dummy"))).thenThrow(new DAOException("Unknown VM ID: 9876")); + when(vmsDAO.getVmInfo(new VmRef(host, "noVm", -1, "dummy"))).thenThrow(new DAOException("Unknown VM ID: noVm")); when(vmsDAO.getVMs(host)).thenReturn(Arrays.asList(vm)); } @@ -118,10 +119,11 @@ context.registerService(VmInfoDAO.class, vmsDAO, null); cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); - args.addArgument("vmId", "234"); + args.addArgument("vmId", VM_ID); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); - String expected = "Process ID: 234\n" + + String expected = "VM ID: " + VM_ID + "\n" + + "Process ID: 234\n" + "Start time: Thu Jun 07 15:32:00 UTC 2012\n" + "Stop time: Fri Nov 01 01:22:00 UTC 2013\n" + "User ID: 2000(myUser)\n" + @@ -142,10 +144,11 @@ context.registerService(VmInfoDAO.class, vmsDAO, null); cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); - args.addArgument("vmId", "234"); + args.addArgument("vmId", VM_ID); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); - String expected = "Process ID: 234\n" + + String expected = "VM ID: " + VM_ID + "\n" + + "Process ID: 234\n" + "Start time: Thu Jun 07 15:32:00 UTC 2012\n" + "Stop time: Fri Nov 01 01:22:00 UTC 2013\n" + "User ID: \n" + @@ -166,10 +169,11 @@ context.registerService(VmInfoDAO.class, vmsDAO, null); cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); - args.addArgument("vmId", "234"); + args.addArgument("vmId", VM_ID); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); - String expected = "Process ID: 234\n" + + String expected = "VM ID: " + VM_ID + "\n" + + "Process ID: 234\n" + "Start time: Thu Jun 07 15:32:00 UTC 2012\n" + "Stop time: Fri Nov 01 01:22:00 UTC 2013\n" + "User ID: 2000\n" + @@ -203,7 +207,8 @@ SimpleArguments args = new SimpleArguments(); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); - String expected = "Process ID: 234\n" + + String expected = "VM ID: " + VM_ID + "\n" + + "Process ID: 234\n" + "Start time: Thu Jun 07 15:32:00 UTC 2012\n" + "Stop time: Fri Nov 01 01:22:00 UTC 2013\n" + "User ID: 2000(myUser)\n" + @@ -220,29 +225,14 @@ context.registerService(VmInfoDAO.class, vmsDAO, null); cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); - args.addArgument("vmId", "9876"); + args.addArgument("vmId", "noVm"); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); - String expected = "Unknown VM ID: 9876\n"; + String expected = "Unknown VM ID: noVm\n"; assertEquals("", cmdCtxFactory.getOutput()); assertEquals(expected, cmdCtxFactory.getError()); } - @Test - public void testVmInfoNonNumericalVMID() throws CommandException { - context.registerService(VmInfoDAO.class, vmsDAO, null); - cmd = new VMInfoCommand(context); - SimpleArguments args = new SimpleArguments(); - args.addArgument("vmId", "fluff"); - args.addArgument("hostId", "123"); - try { - cmd.run(cmdCtxFactory.createContext(args)); - } catch (CommandException ex) { - String expected = "Invalid VM ID: fluff"; - assertEquals(expected, ex.getMessage()); - } - } - @Bug(id="1046", summary="CLI vm-info display wrong stop time for living vms", url="http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1046") @@ -252,14 +242,16 @@ cmd = new VMInfoCommand(context); Calendar start = Calendar.getInstance(); start.set(2012, 5, 7, 15, 32, 0); - VmInfo vmInfo = new VmInfo(234, start.getTimeInMillis(), Long.MIN_VALUE, "vmVersion", "javaHome", "mainClass", "commandLine", "vmName", "vmInfo", "vmVersion", "vmArguments", new HashMap(), new HashMap(), new String[0], 2000, "myUser"); + final String vmId = "61a255db-1c27-43d6-aaee-28bb4788b8db"; + VmInfo vmInfo = new VmInfo(vmId, 234, start.getTimeInMillis(), Long.MIN_VALUE, "vmVersion", "javaHome", "mainClass", "commandLine", "vmName", "vmInfo", "vmVersion", "vmArguments", new HashMap(), new HashMap(), new String[0], 2000, "myUser"); when(vmsDAO.getVmInfo(vm)).thenReturn(vmInfo); SimpleArguments args = new SimpleArguments(); - args.addArgument("vmId", "234"); + args.addArgument("vmId", VM_ID); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); - String expected = "Process ID: 234\n" + + String expected = "VM ID: " + vmId + "\n" + + "Process ID: 234\n" + "Start time: Thu Jun 07 15:32:00 UTC 2012\n" + "Stop time: \n" + "User ID: 2000(myUser)\n" + diff -r 53d3cab77025 -r f7961cf1e440 client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java --- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindow.java Thu Jul 18 18:33:49 2013 -0400 @@ -661,7 +661,7 @@ } else if (value instanceof VmRef) { VmRef vmRef = (VmRef) value; String vmName = vmRef.getName(); - String vmId = vmRef.getIdString(); + String vmId = vmRef.getVmId(); return translator.localize(LocaleResources.VM_TOOLTIP, vmName, vmId).getContents(); } else { return null; diff -r 53d3cab77025 -r f7961cf1e440 client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java --- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -568,7 +568,7 @@ vmInfoControllerProvider.getVmInfoController(vmRef); view.setSubView(vmInformation.getView()); view.setStatusBarPrimaryStatus(t.localize(LocaleResources.VM_PRIMARY_STATUS, - vmRef.getName(), vmRef.getStringID(), vmRef.getAgent().getHostName())); + vmRef.getName(), vmRef.getVmId(), vmRef.getHostRef().getHostName())); } else { throw new IllegalArgumentException("unknown type of ref"); } diff -r 53d3cab77025 -r f7961cf1e440 client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java --- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -79,9 +79,7 @@ import com.redhat.thermostat.client.ui.HostContextAction; import com.redhat.thermostat.client.ui.MenuAction; import com.redhat.thermostat.client.ui.MenuRegistry; -import com.redhat.thermostat.client.ui.SummaryController; import com.redhat.thermostat.client.ui.VMContextAction; -import com.redhat.thermostat.client.ui.VmInformationController; import com.redhat.thermostat.common.ActionEvent; import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.common.ApplicationService; @@ -401,8 +399,8 @@ Collection expectedVMs = new ArrayList<>(); HostRef host = new HostRef("123", "fluffhost1"); - expectedVMs.add(new VmRef(host, 123, "vm1")); - expectedVMs.add(new VmRef(host, 456, "vm2")); + expectedVMs.add(new VmRef(host, "321", 123, "vm1")); + expectedVMs.add(new VmRef(host, "654", 456, "vm2")); when(mockVmsDAO.getVMs(any(HostRef.class))).thenReturn(expectedVMs); @@ -447,10 +445,10 @@ VmRef vmRef = mock(VmRef.class); when(vmRef.getName()).thenReturn("testvm"); - when(vmRef.getIdString()).thenReturn("testvmid"); + when(vmRef.getVmId()).thenReturn("testvmid"); HostRef ref = mock(HostRef.class); when(ref.getAgentId()).thenReturn("agentId"); - when(vmRef.getAgent()).thenReturn(ref); + when(vmRef.getHostRef()).thenReturn(ref); when(view.getSelectedHostOrVm()).thenReturn(vmRef); @@ -485,14 +483,14 @@ when(view.getSelectedHostOrVm()).thenReturn(vmRef1).thenReturn(vmRef1).thenReturn(vmRef2).thenReturn(vmRef1); when(vmRef1.getName()).thenReturn("testvm"); - when(vmRef1.getIdString()).thenReturn("testvmid"); + when(vmRef1.getVmId()).thenReturn("testvmid"); HostRef ref = mock(HostRef.class); when(ref.getAgentId()).thenReturn("agentId"); - when(vmRef1.getAgent()).thenReturn(ref); + when(vmRef1.getHostRef()).thenReturn(ref); when(vmRef2.getName()).thenReturn("testvm"); - when(vmRef2.getIdString()).thenReturn("testvmid"); - when(vmRef2.getAgent()).thenReturn(ref); + when(vmRef2.getVmId()).thenReturn("testvmid"); + when(vmRef2.getHostRef()).thenReturn(ref); VmInformationView vmInfoView2 = mock(VmInformationView.class); @@ -568,7 +566,7 @@ @Test public void verityVMActionsAreShown() { - VmInfo vmInfo = new VmInfo(0, 1, 2, null, null, null, null, null, null, null, null, null, null, null, -1, null); + VmInfo vmInfo = new VmInfo("123", 0, 1, 2, null, null, null, null, null, null, null, null, null, null, null, -1, null); when(mockVmsDAO.getVmInfo(isA(VmRef.class))).thenReturn(vmInfo); VmRef ref = mock(VmRef.class); diff -r 53d3cab77025 -r f7961cf1e440 eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/HostCpuViewPartTest.java --- a/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/HostCpuViewPartTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/HostCpuViewPartTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -116,7 +116,7 @@ view.createPartControl(parent); HostRef hostRef = new HostRef("TEST", "Test"); - VmRef vmRef = new VmRef(hostRef, 0, "Test"); + VmRef vmRef = new VmRef(hostRef, "vmId", 0, "Test"); IStructuredSelection selection = mockSelection(vmRef); view.selectionChanged(hostVMView, selection); diff -r 53d3cab77025 -r f7961cf1e440 eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmClassStatViewPartTest.java --- a/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmClassStatViewPartTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmClassStatViewPartTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -65,7 +65,7 @@ view.createPartControl(parent); HostRef hostRef = new HostRef("TEST", "Test"); - VmRef vmRef = new VmRef(hostRef, 0, "Test"); + VmRef vmRef = new VmRef(hostRef, "vmId", 0, "Test"); IStructuredSelection selection = mockSelection(vmRef); view.selectionChanged(hostVMView, selection); diff -r 53d3cab77025 -r f7961cf1e440 eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmCpuViewPartTest.java --- a/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmCpuViewPartTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmCpuViewPartTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -77,7 +77,7 @@ @Test public void testSelectionHostRefAfterVmRef() throws Exception { HostRef hostRef = new HostRef("TEST", "Test"); - VmRef vmRef = new VmRef(hostRef, 0, "Test"); + VmRef vmRef = new VmRef(hostRef, "vmId", 0, "Test"); mockSelection(vmRef); view.createPartControl(parent); @@ -93,7 +93,7 @@ view.createPartControl(parent); HostRef hostRef = new HostRef("TEST", "Test"); - VmRef vmRef = new VmRef(hostRef, 0, "Test"); + VmRef vmRef = new VmRef(hostRef, "vmId", 0, "Test"); IStructuredSelection selection = mockSelection(vmRef); view.selectionChanged(hostVMView, selection); diff -r 53d3cab77025 -r f7961cf1e440 eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmGcViewPartTest.java --- a/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmGcViewPartTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/eclipse/com.redhat.thermostat.eclipse.test/src/com/redhat/thermostat/eclipse/test/views/VmGcViewPartTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -65,7 +65,7 @@ view.createPartControl(parent); HostRef hostRef = new HostRef("TEST", "Test"); - VmRef vmRef = new VmRef(hostRef, 0, "Test"); + VmRef vmRef = new VmRef(hostRef, "vmId", 0, "Test"); IStructuredSelection selection = mockSelection(vmRef); view.selectionChanged(hostVMView, selection); diff -r 53d3cab77025 -r f7961cf1e440 eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/views/HostRefViewPart.java --- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/views/HostRefViewPart.java Thu Jul 18 16:31:24 2013 -0400 +++ b/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/views/HostRefViewPart.java Thu Jul 18 18:33:49 2013 -0400 @@ -55,7 +55,7 @@ ref = (HostRef) selection; } else if (selection instanceof VmRef) { - ref = ((VmRef) selection).getAgent(); + ref = ((VmRef) selection).getHostRef(); } return ref; } diff -r 53d3cab77025 -r f7961cf1e440 integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java --- a/integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -174,6 +174,9 @@ "/etc/thermostat-users.properties"; private static final String THERMOSTAT_ROLES_FILE = getThermostatHome() + "/etc/thermostat-roles.properties"; + private static final String VM_ID1 = "vmId1"; + private static final String VM_ID2 = "vmId2"; + private static final String VM_ID3 = "vmId3"; private ExpressionFactory factory = new ExpressionFactory(); @@ -381,7 +384,7 @@ pojo.setAgentId("fluff"); pojo.setLoadedClasses(12345); pojo.setTimeStamp(42); - pojo.setVmId(987); + pojo.setVmId(VM_ID1); add.setPojo(pojo); add.apply(); @@ -391,16 +394,16 @@ pojo.setAgentId("fluff"); pojo.setLoadedClasses(67890); pojo.setTimeStamp(42); - pojo.setVmId(654); + pojo.setVmId(VM_ID2); add.setPojo(pojo); add.apply(); add = webStorage.createAdd(VmClassStatDAO.vmClassStatsCategory); pojo = new VmClassStat(); pojo.setAgentId("fluff"); - pojo.setLoadedClasses(12345); + pojo.setLoadedClasses(34567); pojo.setTimeStamp(42); - pojo.setVmId(321); + pojo.setVmId(VM_ID3); add.setPojo(pojo); add.apply(); @@ -739,9 +742,8 @@ storage.registerCategory(VmCpuStatDAO.vmCpuStatCategory); long timeStamp = 5; - int vmId = 10; double cpuLoad = 0.15; - VmCpuStat pojo = new VmCpuStat(timeStamp, vmId, cpuLoad); + VmCpuStat pojo = new VmCpuStat(timeStamp, VM_ID1, cpuLoad); // Note: agentId not set on pojo Add add = storage.createAdd(VmCpuStatDAO.vmCpuStatCategory); add.setPojo(pojo); @@ -754,7 +756,7 @@ assertFalse(cursor.hasNext()); assertEquals(timeStamp, pojo.getTimeStamp()); - assertEquals(vmId, pojo.getVmId()); + assertEquals(VM_ID1, pojo.getVmId()); assertEquals(cpuLoad, pojo.getCpuLoad(), EQUALS_DELTA); assertEquals(uuid.toString(), pojo.getAgentId()); diff -r 53d3cab77025 -r f7961cf1e440 killvm/agent/src/main/java/com/redhat/thermostat/killvm/agent/internal/KillVmReceiver.java --- a/killvm/agent/src/main/java/com/redhat/thermostat/killvm/agent/internal/KillVmReceiver.java Thu Jul 18 16:31:24 2013 -0400 +++ b/killvm/agent/src/main/java/com/redhat/thermostat/killvm/agent/internal/KillVmReceiver.java Thu Jul 18 18:33:49 2013 -0400 @@ -36,6 +36,7 @@ package com.redhat.thermostat.killvm.agent.internal; +import java.util.logging.Level; import java.util.logging.Logger; import com.redhat.thermostat.agent.command.RequestReceiver; @@ -62,10 +63,16 @@ log.severe("Unix service null!"); return new Response(ResponseType.ERROR); } - String vmId = request.getParameter("vm-id"); - unixService.sendSignal(vmId, UNIXSignal.TERM); - log.fine("Killed VM with ID " + vmId); - return new Response(ResponseType.OK); + String strPid = request.getParameter("vm-pid"); + try { + Integer pid = Integer.parseInt(strPid); + unixService.sendSignal(pid, UNIXSignal.TERM); + log.fine("Killed VM with PID " + pid); + return new Response(ResponseType.OK); + } catch (NumberFormatException e) { + log.log(Level.WARNING, "Invalid PID argument", e); + return new Response(ResponseType.ERROR); + } } } diff -r 53d3cab77025 -r f7961cf1e440 killvm/agent/src/test/java/com/redhat/thermostat/killvm/agent/internal/KillVmReceiverTest.java --- a/killvm/agent/src/test/java/com/redhat/thermostat/killvm/agent/internal/KillVmReceiverTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/killvm/agent/src/test/java/com/redhat/thermostat/killvm/agent/internal/KillVmReceiverTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -39,6 +39,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.lang.reflect.Constructor; import java.lang.reflect.Method; @@ -58,12 +59,32 @@ UNIXProcessHandler proc = mock(UNIXProcessHandler.class); KillVmReceiver receiver = new KillVmReceiver(proc); Request req = mock(Request.class); + when(req.getParameter("vm-pid")).thenReturn("12345"); Response response = receiver.receive(req); assertEquals(ResponseType.OK, response.getType()); } + + @Test + public void receiverReturnsErrorNoPid() { + UNIXProcessHandler proc = mock(UNIXProcessHandler.class); + KillVmReceiver receiver = new KillVmReceiver(proc); + Request req = mock(Request.class); + Response response = receiver.receive(req); + assertEquals(ResponseType.ERROR, response.getType()); + } + + @Test + public void receiverReturnsErrorBadPid() { + UNIXProcessHandler proc = mock(UNIXProcessHandler.class); + KillVmReceiver receiver = new KillVmReceiver(proc); + Request req = mock(Request.class); + when(req.getParameter("vm-pid")).thenReturn("hi"); + Response response = receiver.receive(req); + assertEquals(ResponseType.ERROR, response.getType()); + } @Test - public void receiverReturnsError() { + public void receiverReturnsErrorNoProcessHandler() { KillVmReceiver receiver = new KillVmReceiver(null); Request req = mock(Request.class); Response response = receiver.receive(req); diff -r 53d3cab77025 -r f7961cf1e440 killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/KillVMAction.java --- a/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/KillVMAction.java Thu Jul 18 16:31:24 2013 -0400 +++ b/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/KillVMAction.java Thu Jul 18 18:33:49 2013 -0400 @@ -88,13 +88,13 @@ @Override public void execute(VmRef reference) { - String address = agentDao.getAgentInformation(reference.getAgent()).getConfigListenAddress(); + String address = agentDao.getAgentInformation(reference.getHostRef()).getConfigListenAddress(); String [] host = address.split(":"); InetSocketAddress target = new InetSocketAddress(host[0], Integer.parseInt(host[1])); Request murderer = getKillRequest(target); murderer.setParameter(Request.ACTION, CMD_CHANNEL_ACTION_NAME); - murderer.setParameter("vm-id", reference.getIdString()); + murderer.setParameter("vm-pid", String.valueOf(reference.getPid())); murderer.setReceiver(RECEIVER); murderer.addListener(listener); diff -r 53d3cab77025 -r f7961cf1e440 killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/SwingVMKilledListener.java --- a/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/SwingVMKilledListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/SwingVMKilledListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -63,15 +63,15 @@ public void fireComplete(Request request, Response response) { switch (response.getType()) { case ERROR: - String vmId = request.getParameter("vm-id"); + String pid = request.getParameter("vm-pid"); logger.log(Level.SEVERE, "Kill request error for VM ID " - + vmId); - showErrorMessage(t.localize(LocaleResources.KILL_ACTION_ERROR_RESPONSE_MSG, vmId).getContents()); + + pid); + showErrorMessage(t.localize(LocaleResources.KILL_ACTION_ERROR_RESPONSE_MSG, pid).getContents()); break; case OK: logger.log(Level.INFO, - "VM with id " + request.getParameter("vm-id") + "VM with id " + request.getParameter("vm-pid") + " killed on host " + request.getTarget().toString()); break; diff -r 53d3cab77025 -r f7961cf1e440 killvm/client-swing/src/test/java/com/redhat/thermostat/killvm/client/internal/KillVMActionTest.java --- a/killvm/client-swing/src/test/java/com/redhat/thermostat/killvm/client/internal/KillVMActionTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/killvm/client-swing/src/test/java/com/redhat/thermostat/killvm/client/internal/KillVMActionTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -91,7 +91,7 @@ public void canQueueKillRequest() { VmRef ref = mock(VmRef.class); HostRef hostref = mock(HostRef.class); - when(ref.getAgent()).thenReturn(hostref); + when(ref.getHostRef()).thenReturn(hostref); String agentAddress = "127.0.0.1:8888"; AgentInformation agentInfo = mock(AgentInformation.class); @@ -113,7 +113,7 @@ } }; action.execute(ref); - verify(req).setParameter(eq("vm-id"), any(String.class)); + verify(req).setParameter(eq("vm-pid"), any(String.class)); verify(req).setParameter(eq(Request.ACTION), any(String.class)); verify(req).addListener(agentResponseListener); ArgumentCaptor receiverCaptor = ArgumentCaptor diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/main/java/com/redhat/thermostat/storage/core/Key.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/Key.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/Key.java Thu Jul 18 18:33:49 2013 -0400 @@ -45,7 +45,7 @@ // Keys used by most Categories. public static final Key TIMESTAMP = new Key<>("timeStamp", false); public static final Key AGENT_ID = new Key<>("agentId", true); - public static final Key VM_ID = new Key<>("vmId", true); + public static final Key VM_ID = new Key<>("vmId", true); public static final Key ID = new Key<>("_id", false); private String name; diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/main/java/com/redhat/thermostat/storage/core/VmLatestPojoListGetter.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/VmLatestPojoListGetter.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/VmLatestPojoListGetter.java Thu Jul 18 18:33:49 2013 -0400 @@ -58,7 +58,7 @@ this.cat = cat; this.queryLatest = "QUERY " + cat.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i AND '" + + Key.VM_ID.getName() + "' = ?s AND '" + Key.TIMESTAMP.getName() + "' > ?l SORT '" + Key.TIMESTAMP.getName() + "' DSC"; } @@ -94,8 +94,8 @@ PreparedStatement stmt = null; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, vmRef.getAgent().getAgentId()); - stmt.setInt(1, vmRef.getId()); + stmt.setString(0, vmRef.getHostRef().getAgentId()); + stmt.setString(1, vmRef.getVmId()); stmt.setLong(2, since); } catch (DescriptorParsingException e) { // should not happen, but if it *does* happen, at least log it diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/main/java/com/redhat/thermostat/storage/core/VmRef.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/VmRef.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/VmRef.java Thu Jul 18 18:33:49 2013 -0400 @@ -39,14 +39,14 @@ public class VmRef implements Ref { private final HostRef hostRef; - private final Integer uid; - private final String uidString; + private final String id; + private final Integer pid; private final String name; - public VmRef(HostRef hostRef, Integer id, String name) { + public VmRef(HostRef hostRef, String id, Integer pid, String name) { this.hostRef = hostRef; - this.uid = id; - this.uidString = id.toString(); + this.id = id; + this.pid = pid; this.name = name; } @@ -55,19 +55,16 @@ return name; } - public HostRef getAgent() { + public HostRef getHostRef() { return hostRef; } - - public Integer getId() { - return uid; + + public String getVmId() { + return id; } - - /** - * Equivalent to {@link #getStringID()}. - */ - public String getIdString() { - return uidString; + + public Integer getPid() { + return pid; } public String getName() { @@ -86,7 +83,9 @@ return false; } VmRef other = (VmRef) obj; - if (equals(this.hostRef, other.hostRef) && equals(this.uid, other.uid) && equals(this.name, other.name)) { + if (equals(this.hostRef, other.hostRef) + && equals(this.id, other.id) + && equals(this.pid, other.pid) && equals(this.name, other.name)) { return true; } return false; @@ -98,12 +97,12 @@ @Override public int hashCode() { - return uid.hashCode(); + return id.hashCode(); } @Override public String getStringID() { - return getIdString(); + return id; } } diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/dao/VmInfoDAO.java Thu Jul 18 18:33:49 2013 -0400 @@ -82,6 +82,6 @@ public void putVmInfo(VmInfo info); - public void putVmStoppedTime(int vmId, long since); + public void putVmStoppedTime(String vmId, long since); } diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -67,7 +67,7 @@ static final String QUERY_VM_INFO = "QUERY " + vmInfoCategory.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i LIMIT 1"; + + Key.VM_ID.getName() + "' = ?s LIMIT 1"; static final String QUERY_ALL_VMS = "QUERY " + vmInfoCategory.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s"; @@ -88,8 +88,8 @@ Cursor cursor; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, ref.getAgent().getAgentId()); - stmt.setInt(1, ref.getId()); + stmt.setString(0, ref.getHostRef().getAgentId()); + stmt.setString(1, ref.getVmId()); cursor = stmt.executeQuery(); } catch (DescriptorParsingException e) { // should not happen, but if it *does* happen, at least log it @@ -107,7 +107,7 @@ } else { // FIXME this is inconsistent with null returned elsewhere - throw new DAOException("Unknown VM: host:" + ref.getAgent().getAgentId() + ";vm:" + ref.getId()); + throw new DAOException("Unknown VM: host:" + ref.getHostRef().getAgentId() + ";vm:" + ref.getVmId()); } return result; } @@ -145,10 +145,11 @@ } private VmRef buildVmRefFromChunk(VmInfo vmInfo, HostRef host) { - Integer id = vmInfo.getVmId(); + String id = vmInfo.getVmId(); + Integer pid = vmInfo.getVmPid(); // TODO can we do better than the main class? String mainClass = vmInfo.getMainClass(); - VmRef ref = new VmRef(host, id, mainClass); + VmRef ref = new VmRef(host, id, pid, mainClass); return ref; } @@ -165,7 +166,7 @@ } @Override - public void putVmStoppedTime(int vmId, long timestamp) { + public void putVmStoppedTime(String vmId, long timestamp) { Update update = storage.createUpdate(vmInfoCategory); Expression expr = factory.equalTo(Key.VM_ID, vmId); update.where(expr); diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/main/java/com/redhat/thermostat/storage/model/VmInfo.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/model/VmInfo.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/VmInfo.java Thu Jul 18 18:33:49 2013 -0400 @@ -84,6 +84,7 @@ } + private String vmId; private int vmPid = 0; private long startTime = System.currentTimeMillis(); private long stopTime = Long.MIN_VALUE; @@ -105,12 +106,13 @@ /* use defaults */ } - public VmInfo(int vmPid, long startTime, long stopTime, + public VmInfo(String vmId, int vmPid, long startTime, long stopTime, String javaVersion, String javaHome, String mainClass, String commandLine, String vmName, String vmInfo, String vmVersion, String vmArguments, Map properties, Map environment, String[] loadedNativeLibraries, long uid, String username) { + this.vmId = vmId; this.vmPid = vmPid; this.startTime = startTime; this.stopTime = stopTime; @@ -130,13 +132,13 @@ } @Persist - public int getVmId() { - return vmPid; + public String getVmId() { + return vmId; } @Persist - public void setVmId(int vmId) { - this.vmPid = vmId; + public void setVmId(String vmId) { + this.vmId = vmId; } @Persist diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/test/java/com/redhat/thermostat/storage/core/DefaultHostsVMsLoaderTest.java --- a/storage/core/src/test/java/com/redhat/thermostat/storage/core/DefaultHostsVMsLoaderTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/core/DefaultHostsVMsLoaderTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -95,8 +95,8 @@ public void canGetVms() { HostRef hostR = mock(HostRef.class); Collection expectedVms = new ArrayList<>(); - expectedVms.add(new VmRef(hostR, 1, "test1")); - expectedVms.add(new VmRef(hostR, 2, "test2")); + expectedVms.add(new VmRef(hostR, "321", 1, "test1")); + expectedVms.add(new VmRef(hostR, "654", 2, "test2")); loader = new DefaultHostsVMsLoader(mockHostsDAO, mockVmsDAO, false /* irrelevant */); when(mockVmsDAO.getVMs(hostR)).thenReturn(expectedVms); diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/test/java/com/redhat/thermostat/storage/core/VmLatestPojoListGetterTest.java --- a/storage/core/src/test/java/com/redhat/thermostat/storage/core/VmLatestPojoListGetterTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/core/VmLatestPojoListGetterTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -55,6 +55,7 @@ public class VmLatestPojoListGetterTest { private static final String AGENT_ID = "agentid"; private static final String HOSTNAME = "host.example.com"; + private static final String VM_ID = "vmId"; private static final int VM_PID = 123; private static final String MAIN_CLASS = "Foo.class"; private static final String CATEGORY_NAME = "vmcategory"; @@ -77,7 +78,7 @@ @Before public void setUp() { hostRef = new HostRef(AGENT_ID, HOSTNAME); - vmRef = new VmRef(hostRef, VM_PID, MAIN_CLASS); + vmRef = new VmRef(hostRef, VM_ID, VM_PID, MAIN_CLASS); result1 = mock(TestPojo.class); when(result1.getTimeStamp()).thenReturn(t1); when(result1.getData()).thenReturn(lc1); @@ -95,7 +96,7 @@ VmLatestPojoListGetter getter = new VmLatestPojoListGetter<>(storage, cat); String actualDesc = getter.getQueryLatestDesc(); String expected = "QUERY vmcategory WHERE 'agentId' = ?s AND " + - "'vmId' = ?i AND 'timeStamp' > ?l SORT 'timeStamp' DSC"; + "'vmId' = ?s AND 'timeStamp' > ?l SORT 'timeStamp' DSC"; assertEquals(expected, actualDesc); } @@ -112,7 +113,7 @@ assertNotNull(query); verify(storage).prepareStatement(anyDescriptor()); verify(query).setString(0, AGENT_ID); - verify(query).setInt(1, VM_PID); + verify(query).setString(1, VM_ID); verify(query).setLong(2, 123l); verifyNoMoreInteractions(query); } @@ -138,7 +139,7 @@ assertNotNull(query); verify(storage, times(2)).prepareStatement(anyDescriptor()); verify(query).setString(0, AGENT_ID); - verify(query).setInt(1, VM_PID); + verify(query).setString(1, VM_ID); verify(query).setLong(2, Long.MIN_VALUE); verifyNoMoreInteractions(query); } @@ -162,7 +163,7 @@ verify(storage).prepareStatement(anyDescriptor()); verify(query).setString(0, AGENT_ID); - verify(query).setInt(1, VM_PID); + verify(query).setString(1, VM_ID); verify(query).setLong(2, t2); verify(query).executeQuery(); verifyNoMoreInteractions(query); diff -r 53d3cab77025 -r f7961cf1e440 storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java --- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/VmInfoDAOTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -72,7 +72,8 @@ public class VmInfoDAOTest { - private int vmId; + private String vmId; + private int vmPid; private long startTime; private long stopTime; private String jVersion; @@ -91,7 +92,8 @@ @Before public void setUp() { - vmId = 1; + vmId = "vmId"; + vmPid = 1; startTime = 2; stopTime = 3; jVersion = "java 1.0"; @@ -111,7 +113,7 @@ @Test public void preparedQueryDescriptorsAreSane() { - String expectedVmInfo = "QUERY vm-info WHERE 'agentId' = ?s AND 'vmId' = ?i LIMIT 1"; + 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); @@ -148,7 +150,7 @@ @SuppressWarnings("unchecked") PreparedStatement stmt = (PreparedStatement) mock(PreparedStatement.class); when(storage.prepareStatement(anyDescriptor())).thenReturn(stmt); - VmInfo expected = new VmInfo(vmId, startTime, stopTime, jVersion, jHome, mainClass, commandLine, vmName, vmInfo, vmVersion, vmArgs, props, env, libs, uid, username); + VmInfo expected = new VmInfo(vmId, vmPid, startTime, stopTime, jVersion, jHome, mainClass, commandLine, vmName, vmInfo, vmVersion, vmArgs, props, env, libs, uid, username); @SuppressWarnings("unchecked") Cursor cursor = (Cursor) mock(Cursor.class); when(cursor.hasNext()).thenReturn(true).thenReturn(false); @@ -159,8 +161,8 @@ when(hostRef.getAgentId()).thenReturn("system"); VmRef vmRef = mock(VmRef.class); - when(vmRef.getAgent()).thenReturn(hostRef); - when(vmRef.getId()).thenReturn(321); + when(vmRef.getHostRef()).thenReturn(hostRef); + when(vmRef.getVmId()).thenReturn("vmId"); VmInfoDAO dao = new VmInfoDAOImpl(storage); VmInfo info = dao.getVmInfo(vmRef); @@ -168,7 +170,7 @@ verify(storage).prepareStatement(anyDescriptor()); verify(stmt).setString(0, "system"); - verify(stmt).setInt(1, 321); + verify(stmt).setString(1, "vmId"); verify(stmt).executeQuery(); } @@ -191,20 +193,20 @@ when(hostRef.getAgentId()).thenReturn("system"); VmRef vmRef = mock(VmRef.class); - when(vmRef.getAgent()).thenReturn(hostRef); - when(vmRef.getId()).thenReturn(321); + when(vmRef.getVmId()).thenReturn("noVm"); + when(vmRef.getHostRef()).thenReturn(hostRef); VmInfoDAO dao = new VmInfoDAOImpl(storage); try { dao.getVmInfo(vmRef); fail(); } catch (DAOException ex) { - assertEquals("Unknown VM: host:system;vm:321", ex.getMessage()); + assertEquals("Unknown VM: host:system;vm:noVm", ex.getMessage()); } verify(storage).prepareStatement(anyDescriptor()); verify(stmt).setString(0, "system"); - verify(stmt).setInt(1, 321); + verify(stmt).setString(1, "noVm"); verify(stmt).executeQuery(); } @@ -216,12 +218,13 @@ Collection vms = dao.getVMs(host); - assertCollection(vms, new VmRef(host, 123, "mainClass1")); + assertCollection(vms, new VmRef(host, "vmId", 123, "mainClass1")); } private Storage setupStorageForSingleVM() throws DescriptorParsingException, StatementExecutionException { VmInfo vm1 = new VmInfo(); + vm1.setVmId("vmId"); vm1.setVmPid(123); vm1.setMainClass("mainClass1"); @@ -247,15 +250,17 @@ Collection vms = dao.getVMs(host); - assertCollection(vms, new VmRef(host, 123, "mainClass1"), new VmRef(host, 456, "mainClass2")); + assertCollection(vms, new VmRef(host, "vmId1", 123, "mainClass1"), new VmRef(host, "vmId2", 456, "mainClass2")); } private Storage setupStorageForMultiVM() throws DescriptorParsingException, StatementExecutionException { VmInfo vm1 = new VmInfo(); + vm1.setVmId("vmId1"); vm1.setVmPid(123); vm1.setMainClass("mainClass1"); VmInfo vm2 = new VmInfo(); + vm2.setVmId("vmId2"); vm2.setVmPid(456); vm2.setMainClass("mainClass2"); @@ -295,7 +300,7 @@ Replace replace = mock(Replace.class); when(storage.createReplace(any(Category.class))).thenReturn(replace); - VmInfo info = new VmInfo(vmId, startTime, stopTime, jVersion, jHome, + VmInfo info = new VmInfo(vmId, vmPid, startTime, stopTime, jVersion, jHome, mainClass, commandLine, vmName, vmInfo, vmVersion, vmArgs, props, env, libs, uid, username); VmInfoDAO dao = new VmInfoDAOImpl(storage); @@ -317,7 +322,7 @@ verify(storage).createUpdate(VmInfoDAO.vmInfoCategory); ExpressionFactory factory = new ExpressionFactory(); - verify(mockUpdate).where(factory.equalTo(Key.VM_ID, 1)); + verify(mockUpdate).where(factory.equalTo(Key.VM_ID, "vmId")); verify(mockUpdate).set(VmInfoDAO.stopTimeKey, 3L); verify(mockUpdate).apply(); verifyNoMoreInteractions(mockUpdate); diff -r 53d3cab77025 -r f7961cf1e440 system-backend/src/main/java/com/redhat/thermostat/backend/system/JvmStatHostListener.java --- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/JvmStatHostListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/JvmStatHostListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -40,6 +40,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.logging.Level; import java.util.logging.Logger; @@ -53,6 +54,7 @@ import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.backend.system.ProcessUserInfoBuilder.ProcessUserInfo; +import com.redhat.thermostat.common.Pair; import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.VmInfo; @@ -64,7 +66,7 @@ private final VmInfoDAO vmInfoDAO; - private Map monitoredVms = new HashMap<>(); + private Map> monitoredVms = new HashMap<>(); private VmStatusChangeNotifier notifier; @@ -109,36 +111,37 @@ } } - private void sendNewVM(Integer vmId, MonitoredHost host) + private void sendNewVM(Integer vmPid, MonitoredHost host) throws MonitorException, URISyntaxException { MonitoredVm vm = host.getMonitoredVm(host.getHostIdentifier().resolve( - new VmIdentifier(vmId.toString()))); + new VmIdentifier(vmPid.toString()))); if (vm != null) { JvmStatDataExtractor extractor = new JvmStatDataExtractor(vm); + String vmId = UUID.randomUUID().toString(); try { long startTime = System.currentTimeMillis(); long stopTime = Long.MIN_VALUE; - recordVmInfo(vmId, startTime, stopTime, extractor); + recordVmInfo(vmId, vmPid, startTime, stopTime, extractor); logger.finer("Sent VM_STARTED messsage"); } catch (MonitorException me) { logger.log(Level.WARNING, "error getting vm info for " + vmId, me); } - notifier.notifyVmStatusChange(Status.VM_STARTED, vmId); + notifier.notifyVmStatusChange(Status.VM_STARTED, vmId, vmPid); - monitoredVms.put(vmId, vm); + monitoredVms.put(vmPid, new Pair<>(vmId, vm)); } } - void recordVmInfo(Integer vmId, long startTime, long stopTime, + void recordVmInfo(String vmId, Integer vmPid, long startTime, long stopTime, JvmStatDataExtractor extractor) throws MonitorException { Map properties = new HashMap(); ProcDataSource dataSource = new ProcDataSource(); - Map environment = new ProcessEnvironmentBuilder(dataSource).build(vmId); + Map environment = new ProcessEnvironmentBuilder(dataSource).build(vmPid); // TODO actually figure out the loaded libraries. String[] loadedNativeLibraries = new String[0]; - ProcessUserInfo userInfo = userInfoBuilder.build(vmId); - VmInfo info = new VmInfo(vmId, startTime, stopTime, + ProcessUserInfo userInfo = userInfoBuilder.build(vmPid); + VmInfo info = new VmInfo(vmId, vmPid, startTime, stopTime, extractor.getJavaVersion(), extractor.getJavaHome(), extractor.getMainClass(), extractor.getCommandLine(), extractor.getVmName(), extractor.getVmInfo(), extractor.getVmVersion(), extractor.getVmArguments(), @@ -146,17 +149,19 @@ vmInfoDAO.putVmInfo(info); } - private void sendStoppedVM(Integer vmId, MonitoredHost host) throws URISyntaxException, MonitorException { + private void sendStoppedVM(Integer vmPid, MonitoredHost host) throws URISyntaxException, MonitorException { - VmIdentifier resolvedVmID = host.getHostIdentifier().resolve(new VmIdentifier(vmId.toString())); + VmIdentifier resolvedVmID = host.getHostIdentifier().resolve(new VmIdentifier(vmPid.toString())); if (resolvedVmID != null) { - long stopTime = System.currentTimeMillis(); + Pair vmData = monitoredVms.remove(vmPid); + + String vmId = vmData.getFirst(); + notifier.notifyVmStatusChange(Status.VM_STOPPED, vmId, vmPid); - notifier.notifyVmStatusChange(Status.VM_STOPPED, vmId); - + long stopTime = System.currentTimeMillis(); vmInfoDAO.putVmStoppedTime(vmId, stopTime); - MonitoredVm vm = monitoredVms.remove(vmId); + MonitoredVm vm = vmData.getSecond(); vm.detach(); } } @@ -164,7 +169,7 @@ /* * For testing purposes only. */ - Map getMonitoredVms() { + Map> getMonitoredVms() { return monitoredVms; } diff -r 53d3cab77025 -r f7961cf1e440 system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java --- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java Thu Jul 18 16:31:24 2013 -0400 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java Thu Jul 18 18:33:49 2013 -0400 @@ -62,13 +62,13 @@ public class VmStatusChangeNotifier { private final Object listenerLock = new Object(); - private final Set activePids; + private final Map activePids; private final Map> listeners = new HashMap<>(); private final ServiceTracker tracker; public VmStatusChangeNotifier(BundleContext bundleContext) { - this.activePids = new TreeSet<>(); + this.activePids = new HashMap<>(); tracker = new ServiceTracker(bundleContext, VmStatusListener.class, null) { @Override @@ -77,8 +77,9 @@ synchronized (listenerLock) { Set notifiedAbout = new TreeSet<>(); - for (Integer pid : activePids) { - listener.vmStatusChanged(Status.VM_ACTIVE, pid); + for (Entry entry : activePids.entrySet()) { + Integer pid = entry.getKey(); + listener.vmStatusChanged(Status.VM_ACTIVE, entry.getValue(), pid); notifiedAbout.add(pid); } @@ -111,21 +112,22 @@ * * @param newStatus either {@link VmStatusListener.Status#VM_STARTED} or * {@link VmStatusListener.Status#VM_STOPPED} - * @param pid + * @param vmId unique identifier for the VM + * @param pid process ID for the VM */ - public void notifyVmStatusChange(VmStatusListener.Status newStatus, int pid) { + public void notifyVmStatusChange(VmStatusListener.Status newStatus, String vmId, int pid) { if (newStatus == Status.VM_ACTIVE) { throw new IllegalArgumentException("Dont pass in " + Status.VM_ACTIVE + ", that will be handled automatically"); } synchronized (listenerLock) { for (Entry> entry : listeners.entrySet()) { - entry.getKey().vmStatusChanged(newStatus, pid); + entry.getKey().vmStatusChanged(newStatus, vmId, pid); entry.getValue().add(pid); } if (newStatus == Status.VM_STARTED) { - activePids.add(pid); + activePids.put(pid, vmId); } else { activePids.remove(pid); } diff -r 53d3cab77025 -r f7961cf1e440 system-backend/src/test/java/com/redhat/thermostat/backend/system/JvmStatHostListenerTest.java --- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/JvmStatHostListenerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/JvmStatHostListenerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -38,8 +38,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; @@ -50,7 +52,9 @@ import java.net.URISyntaxException; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.UUID; import org.junit.Before; import org.junit.Test; @@ -66,8 +70,8 @@ import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.backend.system.ProcessUserInfoBuilder.ProcessUserInfo; +import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.VmInfo; -import com.redhat.thermostat.storage.dao.VmInfoDAO; public class JvmStatHostListenerTest { @@ -137,10 +141,15 @@ assertTrue(hostListener.getMonitoredVms().containsKey(1)); assertTrue(hostListener.getMonitoredVms().containsKey(2)); - assertEquals(monitoredVm1, hostListener.getMonitoredVms().get(1)); - assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2)); - - verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), (isA(Integer.class))); + assertEquals(monitoredVm1, hostListener.getMonitoredVms().get(1).getSecond()); + assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2).getSecond()); + + // Check valid UUIDs + UUID uuid1 = UUID.fromString(hostListener.getMonitoredVms().get(1).getFirst()); + UUID uuid2 = UUID.fromString(hostListener.getMonitoredVms().get(2).getFirst()); + assertFalse(uuid1.equals(uuid2)); + + verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), anyString(), (isA(Integer.class))); } @Test @@ -160,10 +169,49 @@ // Ensure only 1 removed assertFalse(hostListener.getMonitoredVms().containsKey(1)); assertTrue(hostListener.getMonitoredVms().containsKey(2)); - assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2)); + assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2).getSecond()); + + verify(notifier).notifyVmStatusChange(eq(Status.VM_STOPPED), anyString(), (isA(Integer.class))); - verify(notifier).notifyVmStatusChange(eq(Status.VM_STOPPED), (isA(Integer.class))); - + } + + @Test + public void testReusedPid() { + final Set started = new HashSet<>(); + started.add(1); + + // Start VM + VmStatusChangeEvent event = mock(VmStatusChangeEvent.class); + when(event.getMonitoredHost()).thenReturn(host); + when(event.getStarted()).thenReturn(started); + when(event.getTerminated()).thenReturn(Collections.emptySet()); + hostListener.vmStatusChanged(event); + + ArgumentCaptor vmIdCaptor = ArgumentCaptor.forClass(String.class); + + // Stop VM + event = mock(VmStatusChangeEvent.class); + when(event.getMonitoredHost()).thenReturn(host); + when(event.getStarted()).thenReturn(Collections.emptySet()); + when(event.getTerminated()).thenReturn(started); + hostListener.vmStatusChanged(event); + + // Start new VM + event = mock(VmStatusChangeEvent.class); + when(event.getMonitoredHost()).thenReturn(host); + when(event.getStarted()).thenReturn(started); + when(event.getTerminated()).thenReturn(Collections.emptySet()); + hostListener.vmStatusChanged(event); + + verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), vmIdCaptor.capture(), eq(1)); + List vmIds = vmIdCaptor.getAllValues(); + + assertEquals(2, vmIds.size()); + String vmId1 = vmIds.get(0); + String vmId2 = vmIds.get(1); + assertNotNull(vmId1); + assertNotNull(vmId2); + assertFalse(vmId1.equals(vmId2)); } private void startVMs() throws InterruptedException, MonitorException { @@ -178,20 +226,21 @@ when(event.getTerminated()).thenReturn(Collections.emptySet()); hostListener.vmStatusChanged(event); - verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), isA(Integer.class)); + verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), anyString(), isA(Integer.class)); } @Test public void testRecordVmInfo() throws MonitorException { - final int INFO_ID = 1; + final String INFO_ID = "vmId"; + final int INFO_PID = 1; final long INFO_STARTTIME = Long.MIN_VALUE; final long INFO_STOPTIME = Long.MAX_VALUE; - hostListener.recordVmInfo(INFO_ID, INFO_STARTTIME, INFO_STOPTIME, extractor); + hostListener.recordVmInfo(INFO_ID, INFO_PID, INFO_STARTTIME, INFO_STOPTIME, extractor); ArgumentCaptor captor = ArgumentCaptor.forClass(VmInfo.class); verify(vmInfoDAO).putVmInfo(captor.capture()); VmInfo info = captor.getValue(); - assertEquals(INFO_ID, info.getVmId()); + assertEquals(INFO_PID, info.getVmPid()); assertEquals(INFO_STARTTIME, info.getStartTimeStamp()); assertEquals(INFO_STOPTIME, info.getStopTimeStamp()); assertEquals(INFO_CMDLINE, info.getJavaCommandLine()); diff -r 53d3cab77025 -r f7961cf1e440 system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java --- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -49,19 +49,21 @@ @Test public void verifyWorksWithoutAnyListeners() { - final int VM_ID = 2; + final String VM_ID = "vmId"; + final int VM_PID = 2; StubBundleContext bundleContext = new StubBundleContext(); VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); notifier.start(); - notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID); + notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID, VM_PID); - notifier.notifyVmStatusChange(Status.VM_STOPPED, VM_ID); + notifier.notifyVmStatusChange(Status.VM_STOPPED, VM_ID, VM_PID); } @Test public void verifyAllListenersAreNotified() { - final int VM_ID = 2; + final String VM_ID = "vmId"; + final int VM_PID = 2; StubBundleContext bundleContext = new StubBundleContext(); VmStatusListener listener = mock(VmStatusListener.class); @@ -69,28 +71,29 @@ VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); notifier.start(); - notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID); + notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID, VM_PID); - verify(listener).vmStatusChanged(Status.VM_STARTED, VM_ID); + verify(listener).vmStatusChanged(Status.VM_STARTED, VM_ID, VM_PID); - notifier.notifyVmStatusChange(Status.VM_STOPPED, VM_ID); + notifier.notifyVmStatusChange(Status.VM_STOPPED, VM_ID, VM_PID); - verify(listener).vmStatusChanged(Status.VM_STOPPED, VM_ID); + verify(listener).vmStatusChanged(Status.VM_STOPPED, VM_ID, VM_PID); } @Test public void verifyListenersAddedAfterVmStartRecieveVmActiveEvent() { - final int VM_ID = 2; + final String VM_ID = "vmId"; + final int VM_PID = 2; StubBundleContext bundleContext = new StubBundleContext(); VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); notifier.start(); - notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID); + notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID, VM_PID); VmStatusListener listener = mock(VmStatusListener.class); bundleContext.registerService(VmStatusListener.class, listener, null); - verify(listener).vmStatusChanged(Status.VM_ACTIVE, VM_ID); + verify(listener).vmStatusChanged(Status.VM_ACTIVE, VM_ID, VM_PID); } } diff -r 53d3cab77025 -r f7961cf1e440 thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java --- a/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java Thu Jul 18 18:33:49 2013 -0400 @@ -87,7 +87,7 @@ } Request createRequest() { - HostRef targetHostRef = ref.getAgent(); + HostRef targetHostRef = ref.getHostRef(); String address = agentDao.getAgentInformation(targetHostRef).getConfigListenAddress(); String [] host = address.split(":"); @@ -106,7 +106,8 @@ Request harvester = createRequest(); harvester.setParameter(Request.ACTION, CMD_CHANNEL_ACTION_NAME); harvester.setParameter(HarvesterCommand.class.getName(), HarvesterCommand.START.name()); - harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getIdString()); + harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getVmId()); + harvester.setParameter(HarvesterCommand.VM_PID.name(), String.valueOf(ref.getPid())); return postAndWait(harvester); @@ -118,7 +119,7 @@ Request harvester = createRequest(); harvester.setParameter(Request.ACTION, CMD_CHANNEL_ACTION_NAME); harvester.setParameter(HarvesterCommand.class.getName(), HarvesterCommand.STOP.name()); - harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getIdString()); + harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getVmId()); boolean result = postAndWait(harvester); return result; @@ -180,7 +181,8 @@ Request harvester = createRequest(); harvester.setParameter(Request.ACTION, CMD_CHANNEL_ACTION_NAME); harvester.setParameter(HarvesterCommand.class.getName(), HarvesterCommand.FIND_DEADLOCKS.name()); - harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getIdString()); + harvester.setParameter(HarvesterCommand.VM_ID.name(), ref.getVmId()); + harvester.setParameter(HarvesterCommand.VM_PID.name(), String.valueOf(ref.getPid())); postAndWait(harvester); } diff -r 53d3cab77025 -r f7961cf1e440 thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java --- a/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -81,7 +81,7 @@ agentDao = mock(AgentInfoDAO.class); threadDao = mock(ThreadDao.class); reference = mock(VmRef.class); - when(reference.getIdString()).thenReturn("00101010"); + when(reference.getVmId()).thenReturn("00101010"); final Response response = mock(Response.class); when(response.getType()).thenReturn(ResponseType.OK); diff -r 53d3cab77025 -r f7961cf1e440 thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java --- a/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java Thu Jul 18 18:33:49 2013 -0400 @@ -76,7 +76,7 @@ { this.appService = appService; view = viewFactory.createView(); - view.setApplicationService(appService, ref.getIdString() + "-" + ref.getAgent().getAgentId()); + view.setApplicationService(appService, ref.getVmId() + "-" + ref.getHostRef().getAgentId()); collector = collectorFactory.getCollector(ref); diff -r 53d3cab77025 -r f7961cf1e440 thread/client-controllers/src/test/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationControllerTest.java --- a/thread/client-controllers/src/test/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/client-controllers/src/test/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -145,7 +145,7 @@ VmRef ref = mock(VmRef.class); HostRef agent = mock(HostRef.class); - when(ref.getAgent()).thenReturn(agent); + when(ref.getHostRef()).thenReturn(agent); when(agent.getAgentId()).thenReturn("0xcafe"); ThreadCollectorFactory collectorFactory = mock(ThreadCollectorFactory.class); @@ -175,9 +175,9 @@ doNothing().when(view).addThreadActionListener(viewArgumentCaptor.capture()); VmRef ref = mock(VmRef.class); - when(ref.getStringID()).thenReturn("42"); + when(ref.getVmId()).thenReturn("42"); HostRef agent = mock(HostRef.class); - when(ref.getAgent()).thenReturn(agent); + when(ref.getHostRef()).thenReturn(agent); when(agent.getAgentId()).thenReturn("0xcafe"); ThreadCollector collector = mock(ThreadCollector.class); diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/collector/HarvesterCommand.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/collector/HarvesterCommand.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/collector/HarvesterCommand.java Thu Jul 18 18:33:49 2013 -0400 @@ -42,7 +42,8 @@ STOP, FIND_DEADLOCKS, - VM_ID; + VM_ID, + VM_PID; public static final String RECEIVER = "com.redhat.thermostat.thread.harvester.ThreadHarvester"; diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/dao/impl/ThreadDaoImpl.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/dao/impl/ThreadDaoImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/dao/impl/ThreadDaoImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -69,33 +69,33 @@ static final String QUERY_THREAD_CAPS = "QUERY " + THREAD_CAPABILITIES.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i LIMIT 1"; + + Key.VM_ID.getName() + "' = ?s LIMIT 1"; static final String QUERY_LATEST_SUMMARY = "QUERY " + THREAD_SUMMARY.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i SORT '" + + Key.VM_ID.getName() + "' = ?s SORT '" + Key.TIMESTAMP.getName() + "' DSC LIMIT 1"; static final String QUERY_SUMMARY_SINCE = "QUERY " + THREAD_SUMMARY.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i AND '" + + Key.VM_ID.getName() + "' = ?s AND '" + Key.TIMESTAMP.getName() + "' > ?l SORT '" + Key.TIMESTAMP.getName() + "' DSC"; static final String QUERY_LATEST_HARVESTING_STATUS = "QUERY " + THREAD_HARVESTING_STATUS.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i SORT '" + + Key.VM_ID.getName() + "' = ?s SORT '" + Key.TIMESTAMP.getName() + "' DSC LIMIT 1"; static final String QUERY_THREAD_INFO = "QUERY " + THREAD_INFO.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i AND '" + + Key.VM_ID.getName() + "' = ?s AND '" + Key.TIMESTAMP.getName() + "' > ?l SORT '" + Key.TIMESTAMP.getName() + "' DSC"; static final String QUERY_LATEST_DEADLOCK_INFO = "QUERY " + DEADLOCK_INFO.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i SORT '" + + Key.VM_ID.getName() + "' = ?s SORT '" + Key.TIMESTAMP.getName() + "' DSC LIMIT 1"; private Storage storage; @@ -299,8 +299,8 @@ PreparedStatement stmt = null; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, ref.getAgent().getAgentId()); - stmt.setInt(1, ref.getId()); + stmt.setString(0, ref.getHostRef().getAgentId()); + stmt.setString(1, ref.getVmId()); if (since != null) { stmt.setLong(2, since); } diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadHarvestingStatus.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadHarvestingStatus.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadHarvestingStatus.java Thu Jul 18 18:33:49 2013 -0400 @@ -43,17 +43,17 @@ @Entity public class ThreadHarvestingStatus extends BasePojo { - private int vmId; + private String vmId; private long timeStamp; private boolean collecting; @Persist - public int getVmId() { + public String getVmId() { return vmId; } @Persist - public void setVmId(int newVmId) { + public void setVmId(String newVmId) { this.vmId = newVmId; } diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadInfoData.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadInfoData.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadInfoData.java Thu Jul 18 18:33:49 2013 -0400 @@ -48,7 +48,7 @@ @Entity public class ThreadInfoData extends BasePojo implements TimeStampedPojo { - private int vmId; + private String vmId; private StackTraceElement[] stackTrace; private long threadID; private State threadState; @@ -83,12 +83,12 @@ } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } @Persist - public int getVmId() { + public String getVmId() { return vmId; } diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadSummary.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadSummary.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/model/ThreadSummary.java Thu Jul 18 18:33:49 2013 -0400 @@ -45,19 +45,19 @@ @Entity public class ThreadSummary extends BasePojo implements TimeStampedPojo { - private int vmId; + private String vmId; private long currentLiveThreads; private long daemonThreads; private long timestamp; @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } @Persist - public int getVmId() { + public String getVmId() { return vmId; } diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/model/VMThreadCapabilities.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/model/VMThreadCapabilities.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/model/VMThreadCapabilities.java Thu Jul 18 18:33:49 2013 -0400 @@ -46,15 +46,15 @@ private String[] features; - private int vmId; + private String vmId; @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } @Persist - public int getVmId() { + public String getVmId() { return vmId; } diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/main/java/com/redhat/thermostat/thread/model/VmDeadLockData.java --- a/thread/collector/src/main/java/com/redhat/thermostat/thread/model/VmDeadLockData.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/main/java/com/redhat/thermostat/thread/model/VmDeadLockData.java Thu Jul 18 18:33:49 2013 -0400 @@ -48,7 +48,7 @@ public static final String NO_DEADLOCK = "no-deadlocks"; private long timeStamp; - private int vmId; + private String vmId; private String description; @Persist @@ -63,12 +63,12 @@ } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } @Persist - public int getVmId() { + public String getVmId() { return vmId; } diff -r 53d3cab77025 -r f7961cf1e440 thread/collector/src/test/java/com/redhat/thermostat/thread/dao/impl/ThreadDaoImplTest.java --- a/thread/collector/src/test/java/com/redhat/thermostat/thread/dao/impl/ThreadDaoImplTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/collector/src/test/java/com/redhat/thermostat/thread/dao/impl/ThreadDaoImplTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -71,17 +71,17 @@ @Test public void preparedQueryDescriptorsAreSane() { - String expectedQueryThreadCaps = "QUERY vm-thread-capabilities WHERE 'agentId' = ?s AND 'vmId' = ?i LIMIT 1"; + String expectedQueryThreadCaps = "QUERY vm-thread-capabilities WHERE 'agentId' = ?s AND 'vmId' = ?s LIMIT 1"; assertEquals(expectedQueryThreadCaps, ThreadDaoImpl.QUERY_THREAD_CAPS); - String expectedQueryLatestSummary = "QUERY vm-thread-summary WHERE 'agentId' = ?s AND 'vmId' = ?i SORT 'timeStamp' DSC LIMIT 1"; + String expectedQueryLatestSummary = "QUERY vm-thread-summary WHERE 'agentId' = ?s AND 'vmId' = ?s SORT 'timeStamp' DSC LIMIT 1"; assertEquals(expectedQueryLatestSummary, ThreadDaoImpl.QUERY_LATEST_SUMMARY); - String expectedQuerySummarySince = "QUERY vm-thread-summary WHERE 'agentId' = ?s AND 'vmId' = ?i AND 'timeStamp' > ?l SORT 'timeStamp' DSC"; + String expectedQuerySummarySince = "QUERY vm-thread-summary WHERE 'agentId' = ?s AND 'vmId' = ?s AND 'timeStamp' > ?l SORT 'timeStamp' DSC"; assertEquals(expectedQuerySummarySince, ThreadDaoImpl.QUERY_SUMMARY_SINCE); - String expectedQueryLatestHarvestingStatus = "QUERY vm-thread-harvesting WHERE 'agentId' = ?s AND 'vmId' = ?i SORT 'timeStamp' DSC LIMIT 1"; + String expectedQueryLatestHarvestingStatus = "QUERY vm-thread-harvesting WHERE 'agentId' = ?s AND 'vmId' = ?s SORT 'timeStamp' DSC LIMIT 1"; assertEquals(expectedQueryLatestHarvestingStatus, ThreadDaoImpl.QUERY_LATEST_HARVESTING_STATUS); - String expectedQueryThreadInfo = "QUERY vm-thread-info WHERE 'agentId' = ?s AND 'vmId' = ?i AND 'timeStamp' > ?l SORT 'timeStamp' DSC"; + String expectedQueryThreadInfo = "QUERY vm-thread-info WHERE 'agentId' = ?s AND 'vmId' = ?s AND 'timeStamp' > ?l SORT 'timeStamp' DSC"; assertEquals(expectedQueryThreadInfo, ThreadDaoImpl.QUERY_THREAD_INFO); - String expectedQueryThreadLatestDeadlockInfo = "QUERY vm-deadlock-data WHERE 'agentId' = ?s AND 'vmId' = ?i SORT 'timeStamp' DSC LIMIT 1"; + String expectedQueryThreadLatestDeadlockInfo = "QUERY vm-deadlock-data WHERE 'agentId' = ?s AND 'vmId' = ?s SORT 'timeStamp' DSC LIMIT 1"; assertEquals(expectedQueryThreadLatestDeadlockInfo, ThreadDaoImpl.QUERY_LATEST_DEADLOCK_INFO); } @@ -105,12 +105,12 @@ Storage storage = mock(Storage.class); when(storage.prepareStatement(anyDescriptor(VMThreadCapabilities.class))).thenReturn(stmt); VmRef ref = mock(VmRef.class); - when(ref.getId()).thenReturn(42); + when(ref.getVmId()).thenReturn("VM42"); HostRef agent = mock(HostRef.class); when(agent.getAgentId()).thenReturn("0xcafe"); - when(ref.getAgent()).thenReturn(agent); + when(ref.getHostRef()).thenReturn(agent); VMThreadCapabilities expected = new VMThreadCapabilities(); expected.setSupportedFeaturesList(new String[] { ThreadDao.CPU_TIME, ThreadDao.THREAD_ALLOCATED_MEMORY }); @@ -125,7 +125,7 @@ verify(storage).prepareStatement(anyDescriptor(VMThreadCapabilities.class)); verify(stmt).setString(0, "0xcafe"); - verify(stmt).setInt(1, 42); + verify(stmt).setString(1, "VM42"); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -146,12 +146,12 @@ Storage storage = mock(Storage.class); when(storage.prepareStatement(anyDescriptor(VMThreadCapabilities.class))).thenReturn(stmt); VmRef ref = mock(VmRef.class); - when(ref.getId()).thenReturn(42); + when(ref.getVmId()).thenReturn("VM42"); HostRef agent = mock(HostRef.class); when(agent.getAgentId()).thenReturn("0xcafe"); - when(ref.getAgent()).thenReturn(agent); + when(ref.getHostRef()).thenReturn(agent); VMThreadCapabilities expected = new VMThreadCapabilities(); expected.setSupportedFeaturesList(new String[] { ThreadDao.CPU_TIME, ThreadDao.THREAD_ALLOCATED_MEMORY }); @@ -166,7 +166,7 @@ verify(storage).prepareStatement(anyDescriptor(VMThreadCapabilities.class)); verify(stmt).setString(0, "0xcafe"); - verify(stmt).setInt(1, 42); + verify(stmt).setString(1, "VM42"); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -183,7 +183,7 @@ when(caps.supportContentionMonitor()).thenReturn(true); when(caps.supportCPUTime()).thenReturn(true); when(caps.supportThreadAllocatedMemory()).thenReturn(true); - when(caps.getVmId()).thenReturn(42); + when(caps.getVmId()).thenReturn("VM42"); ThreadDaoImpl dao = new ThreadDaoImpl(storage); dao.saveCapabilities(caps); @@ -195,12 +195,11 @@ @Test public void testLoadLatestDeadLockStatus() throws DescriptorParsingException, StatementExecutionException { VmRef vm = mock(VmRef.class); - when(vm.getId()).thenReturn(42); - when(vm.getIdString()).thenReturn("42"); + when(vm.getVmId()).thenReturn("VM42"); HostRef agent = mock(HostRef.class); when(agent.getAgentId()).thenReturn("0xcafe"); - when(vm.getAgent()).thenReturn(agent); + when(vm.getHostRef()).thenReturn(agent); Storage storage = mock(Storage.class); @SuppressWarnings("unchecked") @@ -221,7 +220,7 @@ verify(storage).prepareStatement(anyDescriptor(VmDeadLockData.class)); verify(stmt).setString(0, "0xcafe"); - verify(stmt).setInt(1, 42); + verify(stmt).setString(1, "VM42"); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); } @@ -244,12 +243,11 @@ @Test public void testGetLatestHarvestingStatus() throws DescriptorParsingException, StatementExecutionException { VmRef vm = mock(VmRef.class); - when(vm.getId()).thenReturn(42); - when(vm.getIdString()).thenReturn("42"); + when(vm.getVmId()).thenReturn("VM42"); HostRef agent = mock(HostRef.class); when(agent.getAgentId()).thenReturn("0xcafe"); - when(vm.getAgent()).thenReturn(agent); + when(vm.getHostRef()).thenReturn(agent); Storage storage = mock(Storage.class); @SuppressWarnings("unchecked") @@ -268,7 +266,7 @@ verify(storage).prepareStatement(anyDescriptor(ThreadHarvestingStatus.class)); verify(stmt).setString(0, "0xcafe"); - verify(stmt).setInt(1, 42); + verify(stmt).setString(1, "VM42"); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); diff -r 53d3cab77025 -r f7961cf1e440 thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/Harvester.java --- a/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/Harvester.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/Harvester.java Thu Jul 18 18:33:49 2013 -0400 @@ -74,16 +74,17 @@ private MXBeanConnection connection; private ThreadMXBean collectorBean; private ThreadDao threadDao; - private int vmId; - + private String vmId; + private int pid; - Harvester(ThreadDao threadDao, ScheduledExecutorService threadPool, String vmId, MXBeanConnectionPool connectionPool) { - this(threadDao, threadPool, new SystemClock(), vmId, connectionPool); + Harvester(ThreadDao threadDao, ScheduledExecutorService threadPool, String vmId, int pid, MXBeanConnectionPool connectionPool) { + this(threadDao, threadPool, new SystemClock(), vmId, pid, connectionPool); } - Harvester(ThreadDao threadDao, ScheduledExecutorService threadPool, Clock clock, String vmId, MXBeanConnectionPool connectionPool) { + Harvester(ThreadDao threadDao, ScheduledExecutorService threadPool, Clock clock, String vmId, int pid, MXBeanConnectionPool connectionPool) { this.threadDao = threadDao; - this.vmId = Integer.valueOf(vmId); + this.vmId = vmId; + this.pid = pid; this.threadPool = threadPool; this.connectionPool = connectionPool; this.clock = clock; @@ -105,7 +106,7 @@ private boolean connect() { try { - connection = connectionPool.acquire(vmId); + connection = connectionPool.acquire(pid); } catch (Exception ex) { logger.log(Level.SEVERE, "can't connect", ex); return false; @@ -128,6 +129,10 @@ return disconnect(); } + + int getPid() { + return pid; + } private boolean disconnect() { if (collectorBean != null) { @@ -137,7 +142,7 @@ isConnected = false; try { - connectionPool.release(vmId, connection); + connectionPool.release(pid, connection); } catch (Exception ex) { logger.log(Level.SEVERE, "can't disconnect", ex); return false; @@ -238,7 +243,7 @@ boolean success = false; try { - MXBeanConnection connection = connectionPool.acquire(vmId); + MXBeanConnection connection = connectionPool.acquire(pid); try { ThreadMXBean bean = getDataCollectorBean(connection); @@ -269,7 +274,7 @@ logger.log(Level.SEVERE, "can't get MXBeanConnection connection", ex); success = false; } - connectionPool.release(vmId, connection); + connectionPool.release(pid, connection); } catch (Exception e) { logger.log(Level.SEVERE, "can't get MXBeanConnection connection", e); success = false; diff -r 53d3cab77025 -r f7961cf1e440 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 Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -45,6 +45,7 @@ import com.redhat.thermostat.agent.VmStatusListenerRegistrar; import com.redhat.thermostat.agent.command.ReceiverRegistry; import com.redhat.thermostat.backend.BaseBackend; +import com.redhat.thermostat.common.Pair; import com.redhat.thermostat.common.Version; import com.redhat.thermostat.common.utils.LoggingUtils; @@ -57,7 +58,7 @@ private boolean active = false; private VmStatusListenerRegistrar vmListener; - private final List pidsToHarvestOnEnable = new ArrayList<>(); + private final List> vmsToHarvestOnEnable = 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()); @@ -79,9 +80,10 @@ } // bring back all harvesters that were active - Iterator iter = pidsToHarvestOnEnable.iterator(); + Iterator> iter = vmsToHarvestOnEnable.iterator(); while (iter.hasNext()) { - harvester.startHarvester(String.valueOf(iter.next())); + Pair saved = iter.next(); + harvester.startHarvester(saved.getFirst(), saved.getSecond()); iter.remove(); } @@ -101,7 +103,7 @@ registry.unregisterReceivers(); // stop all currently active harvesters - pidsToHarvestOnEnable.addAll(harvester.stopAndRemoveAllHarvesters()); + vmsToHarvestOnEnable.addAll(harvester.stopAndRemoveAllHarvesters()); active = false; return true; @@ -113,12 +115,11 @@ } @Override - public void vmStatusChanged(Status newStatus, int pid) { - String vmId = String.valueOf(pid); + public void vmStatusChanged(Status newStatus, String vmId, int pid) { switch (newStatus) { case VM_STARTED: case VM_ACTIVE: /* this is blocking */ - harvester.saveVmCaps(vmId); + harvester.saveVmCaps(vmId, pid); harvester.addThreadHarvestingStatus(vmId); break; case VM_STOPPED: diff -r 53d3cab77025 -r f7961cf1e440 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 Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/harvester/src/main/java/com/redhat/thermostat/thread/harvester/ThreadHarvester.java Thu Jul 18 18:33:49 2013 -0400 @@ -43,14 +43,17 @@ import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ScheduledExecutorService; +import java.util.logging.Level; +import java.util.logging.Logger; import com.redhat.thermostat.agent.command.RequestReceiver; import com.redhat.thermostat.common.Clock; +import com.redhat.thermostat.common.Pair; import com.redhat.thermostat.common.SystemClock; import com.redhat.thermostat.common.command.Request; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; - +import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.thread.collector.HarvesterCommand; import com.redhat.thermostat.thread.dao.ThreadDao; import com.redhat.thermostat.thread.model.ThreadHarvestingStatus; @@ -58,6 +61,8 @@ public class ThreadHarvester implements RequestReceiver { + private static final Logger logger = LoggingUtils.getLogger(ThreadHarvester.class); + private ScheduledExecutorService executor; Map connectors; @@ -75,22 +80,22 @@ this.clock = clock; this.connectionPool = connectionPool; } - + /** - * Set the new implementation of thread DAO to be used as stroage + * Set the new implementation of thread DAO to be used as storage * @param dao */ public void setThreadDao(ThreadDao dao) { // stop everything using the old implementation - List saved = new ArrayList<>(); + List> allSaved = new ArrayList<>(); if (this.dao != null) { - saved.addAll(stopAndRemoveAllHarvesters()); + allSaved.addAll(stopAndRemoveAllHarvesters()); } this.dao = dao; // re-enable all existing harvesters - for (Integer pid : saved) { - startHarvester(String.valueOf(pid)); + for (Pair saved : allSaved) { + startHarvester(saved.getFirst(), saved.getSecond()); } } @@ -106,7 +111,13 @@ switch (HarvesterCommand.valueOf(command)) { case START: { String vmId = request.getParameter(HarvesterCommand.VM_ID.name()); - result = startHarvester(vmId); + String strPid = request.getParameter(HarvesterCommand.VM_PID.name()); + try { + Integer pid = Integer.parseInt(strPid); + result = startHarvester(vmId, pid); + } catch (NumberFormatException e) { + logger.log(Level.WARNING, "Invalid pid: " + strPid, e); + } break; } case STOP: { @@ -116,7 +127,13 @@ } case FIND_DEADLOCKS: String vmId = request.getParameter(HarvesterCommand.VM_ID.name()); - result = findAndSaveDeadLockInformation(vmId); + String strPid = request.getParameter(HarvesterCommand.VM_PID.name()); + try { + Integer pid = Integer.parseInt(strPid); + result = findAndSaveDeadLockInformation(vmId, pid); + } catch (NumberFormatException e) { + logger.log(Level.WARNING, "Invalid pid: " + strPid, e); + } break; default: result = false; @@ -135,17 +152,17 @@ *

* Saves current harvesting status to storage. */ - public boolean startHarvester(String vmId) { - Harvester harvester = getHarvester(vmId); + public boolean startHarvester(String vmId, int pid) { + Harvester harvester = getHarvester(vmId, pid); boolean result = harvester.start(); if (result) { - updateHarvestingStatus(Integer.valueOf(vmId), result); + updateHarvestingStatus(vmId, result); } return result; } - boolean saveVmCaps(String vmId) { - Harvester harvester = getHarvester(vmId); + boolean saveVmCaps(String vmId, int pid) { + Harvester harvester = getHarvester(vmId, pid); return harvester.saveVmCaps(); } @@ -160,21 +177,21 @@ if (harvester != null) { result = harvester.stop(); } - updateHarvestingStatus(Integer.valueOf(vmId), false); - return true; + updateHarvestingStatus(vmId, false); + return result; } /** Save current status to storage */ - public void addThreadHarvestingStatus(String pid) { + public void addThreadHarvestingStatus(String vmId) { boolean harvesting = false; - Harvester harvester = connectors.get(pid); + Harvester harvester = connectors.get(vmId); if (harvester != null) { harvesting = harvester.isConnected(); } - updateHarvestingStatus(Integer.valueOf(pid), harvesting); + updateHarvestingStatus(vmId, harvesting); } - private void updateHarvestingStatus(int vmId, boolean harvesting) { + private void updateHarvestingStatus(String vmId, boolean harvesting) { ThreadHarvestingStatus status = new ThreadHarvestingStatus(); status.setTimeStamp(clock.getRealTimeMillis()); status.setVmId(vmId); @@ -182,33 +199,35 @@ dao.saveHarvestingStatus(status); } - private Harvester getHarvester(String vmId) { + private Harvester getHarvester(String vmId, int pid) { Harvester harvester = connectors.get(vmId); if (harvester == null) { - harvester = createHarvester(vmId); + harvester = createHarvester(vmId, pid); connectors.put(vmId, harvester); } return harvester; } - Harvester createHarvester(String vmId) { - return new Harvester(dao, executor, vmId, connectionPool); + Harvester createHarvester(String vmId, int pid) { + return new Harvester(dao, executor, vmId, pid, connectionPool); } /** - * Returns a list of PIDs which the harvester stopped harvesting + * Returns a list of VM ID, PID pairs which can be used to resume + * the harvesters at a later time. */ - public List stopAndRemoveAllHarvesters() { - List result = new ArrayList<>(); + 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); + String vmId = entry.getKey(); + Harvester harvester = entry.getValue(); + harvester.stop(); + updateHarvestingStatus(vmId, false); iter.remove(); - result.add(pid); + result.add(new Pair<>(vmId, harvester.getPid())); } return result; } @@ -217,10 +236,11 @@ return dao != null; } - public boolean findAndSaveDeadLockInformation(String vmId) { - Harvester harvester = getHarvester(vmId); + public boolean findAndSaveDeadLockInformation(String vmId, int pid) { + Harvester harvester = getHarvester(vmId, pid); boolean result = harvester.saveDeadLockData(); return result; } + } diff -r 53d3cab77025 -r f7961cf1e440 thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/HarvesterTest.java --- a/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/HarvesterTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/HarvesterTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -86,7 +86,7 @@ when(executor.scheduleAtFixedRate(arg0.capture(), arg1.capture(), arg2.capture(), arg3.capture())).thenReturn(null); - Harvester harvester = new Harvester(dao, executor, "42", pool) { + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool) { @Override synchronized void harvestData() { harvestDataCalled[0] = true; @@ -130,7 +130,7 @@ when(executor.scheduleAtFixedRate(arg0.capture(), arg1.capture(), arg2.capture(), arg3.capture())).thenReturn(null); - Harvester harvester = new Harvester(dao, executor, "42", pool) { + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool) { @Override synchronized void harvestData() { harvestDataCalled[0] = true; @@ -170,7 +170,7 @@ when(executor.scheduleAtFixedRate(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class))).thenReturn(future); - Harvester harvester = new Harvester(dao, executor, "42", pool); + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool); harvester.start(); @@ -204,7 +204,7 @@ when(executor.scheduleAtFixedRate(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class))).thenReturn(future); - Harvester harvester = new Harvester(dao, executor, "42", pool); + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool); harvester.start(); @@ -232,7 +232,7 @@ MXBeanConnectionPool pool = mock(MXBeanConnectionPool.class); - Harvester harvester = new Harvester(dao, executor, "42", pool); + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool); verify(executor, times(0)).scheduleAtFixedRate(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class)); @@ -286,7 +286,7 @@ final boolean [] getDataCollectorBeanCalled = new boolean[1]; - Harvester harvester = new Harvester(dao, executor, "42", pool) { + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool) { @Override ThreadMXBean getDataCollectorBean(MXBeanConnection connection) throws MalformedObjectNameException { @@ -307,10 +307,10 @@ verify(dao, times(2)).saveThreadInfo(any(ThreadInfoData.class)); assertEquals(42, summaryCapture.getValue().getCurrentLiveThreads()); - assertEquals(42, summaryCapture.getValue().getVmId()); + assertEquals("vmId", summaryCapture.getValue().getVmId()); assertEquals(42, summaryCapture.getValue().getCurrentLiveThreads()); - assertEquals(42, summaryCapture.getValue().getVmId()); + assertEquals("vmId", summaryCapture.getValue().getVmId()); List threadInfos = threadInfoCapture.getAllValues(); assertEquals(2, threadInfos.size()); @@ -345,7 +345,7 @@ final boolean [] getDataCollectorBeanCalled = new boolean[1]; - Harvester harvester = new Harvester(dao, executor, "42", pool) { + Harvester harvester = new Harvester(dao, executor, "vmId", 42, pool) { @Override ThreadMXBean getDataCollectorBean(MXBeanConnection connection) throws MalformedObjectNameException { @@ -358,7 +358,7 @@ assertTrue(getDataCollectorBeanCalled[0]); verify(dao, times(1)).saveCapabilities(any(VMThreadCapabilities.class)); - assertEquals(42, capsCapture.getValue().getVmId()); + assertEquals("vmId", capsCapture.getValue().getVmId()); String[] features = capsCapture.getValue().getSupportedFeaturesList(); assertEquals(2, features.length); @@ -387,7 +387,7 @@ final boolean[] getDataCollectorBeanCalled = new boolean[1]; - Harvester harvester = new Harvester(dao, executor, clock, "42", pool) { + Harvester harvester = new Harvester(dao, executor, clock, "vmId", 42, pool) { @Override ThreadMXBean getDataCollectorBean(MXBeanConnection connection) throws MalformedObjectNameException { @@ -403,7 +403,7 @@ VmDeadLockData data = deadLockCapture.getValue(); - assertEquals(42, data.getVmId()); + assertEquals("vmId", data.getVmId()); assertEquals(101010l, data.getTimeStamp()); assertEquals("", data.getDeadLockDescription()); } diff -r 53d3cab77025 -r f7961cf1e440 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 Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadBackendTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -38,7 +38,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -48,9 +47,10 @@ import org.junit.Before; import org.junit.Test; +import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.agent.VmStatusListenerRegistrar; -import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.agent.command.ReceiverRegistry; +import com.redhat.thermostat.common.Pair; import com.redhat.thermostat.common.Version; public class ThreadBackendTest { @@ -95,28 +95,28 @@ @Test public void testActivateAfterDeactivate() { - when(threadHarvester.stopAndRemoveAllHarvesters()).thenReturn(Arrays.asList(1)); + when(threadHarvester.stopAndRemoveAllHarvesters()).thenReturn(Arrays.asList(new Pair<>("vmId", 10))); assertTrue(backend.activate()); assertTrue(backend.deactivate()); assertTrue(backend.activate()); assertTrue(backend.isActive()); - verify(threadHarvester).startHarvester("1"); + verify(threadHarvester).startHarvester("vmId", 10); } @Test public void testVmStarts() { - backend.vmStatusChanged(Status.VM_STARTED, 10); + backend.vmStatusChanged(Status.VM_STARTED, "vmId", 10); - verify(threadHarvester).saveVmCaps("10"); - verify(threadHarvester).addThreadHarvestingStatus("10"); + verify(threadHarvester).saveVmCaps("vmId", 10); + verify(threadHarvester).addThreadHarvestingStatus("vmId"); } @Test public void testVmStops() { - backend.vmStatusChanged(Status.VM_STOPPED, 10); + backend.vmStatusChanged(Status.VM_STOPPED, "vmId", 10); - verify(threadHarvester).stopHarvester("10"); + verify(threadHarvester).stopHarvester("vmId"); } } diff -r 53d3cab77025 -r f7961cf1e440 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 Thu Jul 18 16:31:24 2013 -0400 +++ b/thread/harvester/src/test/java/com/redhat/thermostat/thread/harvester/ThreadHarvesterTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -50,6 +50,7 @@ import org.mockito.ArgumentCaptor; import com.redhat.thermostat.common.Clock; +import com.redhat.thermostat.common.Pair; import com.redhat.thermostat.common.command.Request; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; @@ -82,15 +83,17 @@ when(request.getParameter(captor.capture())). thenReturn(HarvesterCommand.START.name()). + thenReturn("vmId"). thenReturn("42"). thenReturn("0xcafe"); ThreadHarvester threadHarvester = new ThreadHarvester(executor, pool) { @Override - Harvester createHarvester(String vmId) { + Harvester createHarvester(String vmId, int pid) { createHarvesterCalled[0] = true; - assertEquals("42", vmId); + assertEquals("vmId", vmId); + assertEquals(42, pid); return harverster; } @@ -99,10 +102,11 @@ threadHarvester.receive(request); List values = captor.getAllValues(); - assertEquals(2, values.size()); + assertEquals(3, values.size()); assertEquals(HarvesterCommand.class.getName(), values.get(0)); assertEquals(HarvesterCommand.VM_ID.name(), values.get(1)); + assertEquals(HarvesterCommand.VM_PID.name(), values.get(2)); assertTrue(createHarvesterCalled[0]); @@ -120,10 +124,10 @@ when(request.getParameter(captor.capture())). thenReturn(HarvesterCommand.STOP.name()). - thenReturn("42"); + thenReturn("vmId"); ThreadHarvester threadHarvester = new ThreadHarvester(executor, pool) { - { connectors.put("42", harverster); } + { connectors.put("vmId", harverster); } }; threadHarvester.setThreadDao(dao); threadHarvester.receive(request); @@ -150,15 +154,17 @@ when(request.getParameter(captor.capture())). thenReturn(HarvesterCommand.FIND_DEADLOCKS.name()). + thenReturn("vmId"). thenReturn("42"). thenReturn("0xcafe"); ThreadHarvester threadHarvester = new ThreadHarvester(executor, pool) { @Override - Harvester createHarvester(String vmId) { + Harvester createHarvester(String vmId, int pid) { createHarvesterCalled[0] = true; - assertEquals("42", vmId); + assertEquals("vmId", vmId); + assertEquals(42, pid); return harverster; } @@ -167,10 +173,11 @@ threadHarvester.receive(request); List values = captor.getAllValues(); - assertEquals(2, values.size()); + assertEquals(3, values.size()); assertEquals(HarvesterCommand.class.getName(), values.get(0)); assertEquals(HarvesterCommand.VM_ID.name(), values.get(1)); + assertEquals(HarvesterCommand.VM_PID.name(), values.get(2)); assertTrue(createHarvesterCalled[0]); @@ -186,16 +193,17 @@ ThreadHarvester threadHarvester = new ThreadHarvester(executor, pool) { @Override - Harvester createHarvester(String vmId) { + Harvester createHarvester(String vmId, int pid) { createHarvesterCalled[0] = true; - assertEquals("42", vmId); + assertEquals("vmId", vmId); + assertEquals(42, pid); return harverster; } }; threadHarvester.setThreadDao(dao); - threadHarvester.saveVmCaps("42"); + threadHarvester.saveVmCaps("vmId", 42); assertTrue(createHarvesterCalled[0]); @@ -219,13 +227,13 @@ ThreadHarvester harvester = new ThreadHarvester(executor, clock, pool); harvester.setThreadDao(dao); - harvester.addThreadHarvestingStatus("10"); + harvester.addThreadHarvestingStatus("vmId"); ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(ThreadHarvestingStatus.class); verify(dao).saveHarvestingStatus(statusCaptor.capture()); ThreadHarvestingStatus status = statusCaptor.getValue(); - assertEquals(10, status.getVmId()); + assertEquals("vmId", status.getVmId()); assertEquals(false, status.isHarvesting()); assertEquals(1, status.getTimeStamp()); } @@ -239,14 +247,14 @@ ThreadHarvester harvester = new ThreadHarvester(executor, clock, pool); harvester.setThreadDao(dao); - harvester.saveVmCaps("10"); - harvester.addThreadHarvestingStatus("10"); + harvester.saveVmCaps("vmId", 10); + harvester.addThreadHarvestingStatus("vmId"); ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(ThreadHarvestingStatus.class); verify(dao).saveHarvestingStatus(statusCaptor.capture()); ThreadHarvestingStatus status = statusCaptor.getValue(); - assertEquals(10, status.getVmId()); + assertEquals("vmId", status.getVmId()); assertEquals(false, status.isHarvesting()); assertEquals(1, status.getTimeStamp()); } @@ -257,29 +265,34 @@ 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); + when(javaHarvester.getPid()).thenReturn(42); ThreadHarvester harvester = new ThreadHarvester(executor, clock, pool) { @Override - Harvester createHarvester(String vmId) { - - createHarvesterCalled[0] = true; - assertEquals("42", vmId); + Harvester createHarvester(String vmId, int pid) { + assertEquals("vmId", vmId); + assertEquals(42, pid); return javaHarvester; } }; + + harvester.setThreadDao(dao); + harvester.startHarvester("vmId", 42); + + // Reset DAO harvester.setThreadDao(dao); - assertTrue(harvester.startHarvester("42")); + assertTrue(harvester.startHarvester("vmId", 42)); - List pids = harvester.stopAndRemoveAllHarvesters(); - assertEquals(1, pids.size()); - assertEquals(42, (int) pids.get(0)); - + List> allSaved = harvester.stopAndRemoveAllHarvesters(); + assertEquals(1, allSaved.size()); + Pair saved = allSaved.get(0); + assertEquals("vmId", saved.getFirst()); + assertEquals(42, saved.getSecond().intValue()); } } diff -r 53d3cab77025 -r f7961cf1e440 unix-process-handler/src/main/java/com/redhat/thermostat/service/process/UNIXProcessHandler.java --- a/unix-process-handler/src/main/java/com/redhat/thermostat/service/process/UNIXProcessHandler.java Thu Jul 18 16:31:24 2013 -0400 +++ b/unix-process-handler/src/main/java/com/redhat/thermostat/service/process/UNIXProcessHandler.java Thu Jul 18 18:33:49 2013 -0400 @@ -47,11 +47,11 @@ * Sends the given {@link UNIXSignal} to the process indicated by * {@code pid}. */ - public void sendSignal(String pid, UNIXSignal signal); + public void sendSignal(Integer pid, UNIXSignal signal); /** * Gets the process name given its {@code pid}. */ - public String getProcessName(String pid); + public String getProcessName(Integer pid); } diff -r 53d3cab77025 -r f7961cf1e440 unix-process-handler/src/main/java/com/redhat/thermostat/service/process/UnixProcessUtilities.java --- a/unix-process-handler/src/main/java/com/redhat/thermostat/service/process/UnixProcessUtilities.java Thu Jul 18 16:31:24 2013 -0400 +++ b/unix-process-handler/src/main/java/com/redhat/thermostat/service/process/UnixProcessUtilities.java Thu Jul 18 18:33:49 2013 -0400 @@ -61,7 +61,7 @@ UnixProcessUtilities() {} @Override - public void sendSignal(String pid, UNIXSignal signal) { + public void sendSignal(Integer pid, UNIXSignal signal) { exec("kill -s " + signal.signalName() + " " + pid); } @@ -75,7 +75,7 @@ } @Override - public String getProcessName(String pid) { + public String getProcessName(Integer pid) { String result = null; @@ -83,7 +83,7 @@ commandLine.add("ps"); commandLine.add("--no-heading"); commandLine.add("-p"); - commandLine.add(pid); + commandLine.add(String.valueOf(pid)); try { Process process = createAndRunProcess(commandLine); diff -r 53d3cab77025 -r f7961cf1e440 unix-process-handler/src/test/java/com/redhat/thermostat/service/process/UnixProcessUtilitiesTest.java --- a/unix-process-handler/src/test/java/com/redhat/thermostat/service/process/UnixProcessUtilitiesTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/unix-process-handler/src/test/java/com/redhat/thermostat/service/process/UnixProcessUtilitiesTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -88,7 +88,7 @@ @Test public void sendKillSignalTest() { - process.sendSignal("12345", UNIXSignal.KILL); + process.sendSignal(12345, UNIXSignal.KILL); Assert.assertTrue(processArguments.contains("kill -s kill 12345")); Assert.assertEquals(1, processArguments.size()); @@ -97,7 +97,7 @@ @Test public void sendTermSignalTest() { - process.sendSignal("12345", UNIXSignal.TERM); + process.sendSignal(12345, UNIXSignal.TERM); Assert.assertTrue(processArguments.contains("kill -s term 12345")); Assert.assertEquals(1, processArguments.size()); @@ -106,7 +106,7 @@ @Test public void getProcessName() { - String result = process.getProcessName("12345"); + String result = process.getProcessName(12345); Assert.assertEquals("fluff", result); Assert.assertTrue(processArguments.contains("12345")); @@ -132,7 +132,7 @@ }; }; - String result = process.getProcessName("12345"); + String result = process.getProcessName(12345); Assert.assertNull(result); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-classstat/agent/src/main/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatBackend.java --- a/vm-classstat/agent/src/main/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatBackend.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-classstat/agent/src/main/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -59,8 +59,8 @@ } @Override - protected VmUpdateListener createVmListener(int pid) { - return new VmClassStatVmListener(vmClassStats, pid); + protected VmUpdateListener createVmListener(String vmId, int pid) { + return new VmClassStatVmListener(vmClassStats, vmId); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-classstat/agent/src/main/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatVmListener.java --- a/vm-classstat/agent/src/main/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatVmListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-classstat/agent/src/main/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatVmListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -51,9 +51,9 @@ private static final Logger logger = LoggingUtils.getLogger(VmClassStatVmListener.class); private VmClassStatDAO dao; - private int vmId; + private String vmId; - VmClassStatVmListener(VmClassStatDAO dao, int vmId) { + VmClassStatVmListener(VmClassStatDAO dao, String vmId) { this.dao = dao; this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-classstat/agent/src/test/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatVmListenerTest.java --- a/vm-classstat/agent/src/test/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatVmListenerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-classstat/agent/src/test/java/com/redhat/thermostat/vm/classstat/agent/internal/VmClassStatVmListenerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -55,7 +55,7 @@ public class VmClassStatVmListenerTest { - private static final Integer VM_ID = 123; + private static final String VM_ID = "vmId"; private static final Long LOADED_CLASSES = 1234L; private VmClassStatDAO dao; @@ -78,7 +78,7 @@ verify(dao).putVmClassStat(arg.capture()); VmClassStat stat = arg.getValue(); assertEquals(LOADED_CLASSES, (Long) stat.getLoadedClasses()); - assertEquals(VM_ID, (Integer) stat.getVmId()); + assertEquals(VM_ID, stat.getVmId()); } @Test diff -r 53d3cab77025 -r f7961cf1e440 vm-classstat/client-core/src/test/java/com/redhat/thermostat/vm/classstat/client/core/internal/VmClassStatControllerTest.java --- a/vm-classstat/client-core/src/test/java/com/redhat/thermostat/vm/classstat/client/core/internal/VmClassStatControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-classstat/client-core/src/test/java/com/redhat/thermostat/vm/classstat/client/core/internal/VmClassStatControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -66,7 +66,7 @@ @Test public void testChartUpdate() { - VmClassStat stat1 = new VmClassStat(123, 12345, 1234); + VmClassStat stat1 = new VmClassStat("vmId", 12345, 1234); List stats = new ArrayList(); stats.add(stat1); diff -r 53d3cab77025 -r f7961cf1e440 vm-classstat/common/src/main/java/com/redhat/thermostat/vm/classstat/common/model/VmClassStat.java --- a/vm-classstat/common/src/main/java/com/redhat/thermostat/vm/classstat/common/model/VmClassStat.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-classstat/common/src/main/java/com/redhat/thermostat/vm/classstat/common/model/VmClassStat.java Thu Jul 18 18:33:49 2013 -0400 @@ -44,27 +44,27 @@ @Entity public class VmClassStat extends BasePojo implements TimeStampedPojo { - private int vmId; + private String vmId; private long timestamp; private long loadedClasses; public VmClassStat() { - this(-1, -1, -1); + this(null, -1, -1); } - public VmClassStat(int vmId, long timestamp, long loadedClasses) { + public VmClassStat(String vmId, long timestamp, long loadedClasses) { this.vmId = vmId; this.timestamp = timestamp; this.loadedClasses = loadedClasses; } @Persist - public int getVmId() { + public String getVmId() { return vmId; } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-classstat/common/src/test/java/com/redhat/thermostat/vm/classstat/common/internal/VmClassStatDAOTest.java --- a/vm-classstat/common/src/test/java/com/redhat/thermostat/vm/classstat/common/internal/VmClassStatDAOTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-classstat/common/src/test/java/com/redhat/thermostat/vm/classstat/common/internal/VmClassStatDAOTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -66,7 +66,7 @@ public class VmClassStatDAOTest { private static final Long TIMESTAMP = 1234L; - private static final Integer VM_ID = 123; + private static final String VM_ID = "vmId"; private static final Long LOADED_CLASSES = 12345L; @Test @@ -100,15 +100,15 @@ when(hostRef.getAgentId()).thenReturn("system"); VmRef vmRef = mock(VmRef.class); - when(vmRef.getAgent()).thenReturn(hostRef); - when(vmRef.getId()).thenReturn(321); + when(vmRef.getHostRef()).thenReturn(hostRef); + when(vmRef.getVmId()).thenReturn(VM_ID); VmClassStatDAO dao = new VmClassStatDAOImpl(storage); List vmClassStats = dao.getLatestClassStats(vmRef, Long.MIN_VALUE); verify(storage).prepareStatement(anyDescriptor()); verify(stmt).setString(0, "system"); - verify(stmt).setInt(1, 321); + verify(stmt).setString(1, VM_ID); verify(stmt).setLong(2, Long.MIN_VALUE); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -117,7 +117,7 @@ VmClassStat stat = vmClassStats.get(0); assertEquals(TIMESTAMP, (Long) stat.getTimeStamp()); assertEquals(LOADED_CLASSES, (Long) stat.getLoadedClasses()); - assertEquals(VM_ID, (Integer) stat.getVmId()); + assertEquals(VM_ID, stat.getVmId()); } @SuppressWarnings("unchecked") diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/agent/src/main/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuBackend.java --- a/vm-cpu/agent/src/main/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuBackend.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/agent/src/main/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -38,8 +38,9 @@ import java.io.BufferedReader; import java.io.IOException; -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.logging.Level; @@ -68,7 +69,7 @@ private VmStatusListenerRegistrar registrar; private boolean started; - private final List pidsToMonitor = new CopyOnWriteArrayList<>(); + private final Map pidsToMonitor = new ConcurrentHashMap<>(); public VmCpuBackend(ScheduledExecutorService executor, VmCpuStatDAO vmCpuStatDao, Version version, VmStatusListenerRegistrar registrar) { @@ -96,9 +97,11 @@ executor.scheduleAtFixedRate(new Runnable() { @Override public void run() { - for (Integer pid : pidsToMonitor) { + for (Entry entry : pidsToMonitor.entrySet()) { + String vmId = entry.getValue(); + Integer pid = entry.getKey(); if (vmCpuStatBuilder.knowsAbout(pid)) { - VmCpuStat dataBuilt = vmCpuStatBuilder.build(pid); + VmCpuStat dataBuilt = vmCpuStatBuilder.build(vmId, pid); if (dataBuilt != null) { vmCpuStats.putVmCpuStat(dataBuilt); } @@ -156,16 +159,15 @@ * Methods implementing VmStatusListener */ @Override - public void vmStatusChanged(Status newStatus, int pid) { + public void vmStatusChanged(Status newStatus, String vmId, int pid) { switch (newStatus) { case VM_STARTED: /* fall-through */ case VM_ACTIVE: - pidsToMonitor.add(pid); + pidsToMonitor.put(pid, vmId); break; case VM_STOPPED: - // the cast is important because it changes the call from remove(index) to remove(Object) - pidsToMonitor.remove((Integer) pid); + pidsToMonitor.remove(pid); vmCpuStatBuilder.forgetAbout(pid); break; } diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/agent/src/main/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuStatBuilder.java --- a/vm-cpu/agent/src/main/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuStatBuilder.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/agent/src/main/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuStatBuilder.java Thu Jul 18 18:33:49 2013 -0400 @@ -67,11 +67,12 @@ } /** + * @param vmId the unique identifier for the VM * @param pid the process id * @return an object representing the cpu usage of the process, or null if * the information can not be found. */ - public synchronized VmCpuStat build(Integer pid) { + public synchronized VmCpuStat build(String vmId, Integer pid) { if (!lastProcessTicks.containsKey(pid) || !lastProcessTickTime.containsKey(pid)) { throw new IllegalArgumentException("unknown pid"); } @@ -103,7 +104,7 @@ lastProcessTicks.put(pid, programTicks); lastProcessTickTime.put(pid, time); - return new VmCpuStat(miliTime, pid, cpuLoad); + return new VmCpuStat(miliTime, vmId, cpuLoad); } public synchronized boolean knowsAbout(int pid) { diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/agent/src/test/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuBackendTest.java --- a/vm-cpu/agent/src/test/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuBackendTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/agent/src/test/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuBackendTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -134,8 +134,8 @@ VmCpuStatBuilder builder = mock(VmCpuStatBuilder.class); VmCpuStat stat0 = mock(VmCpuStat.class); VmCpuStat stat1 = mock(VmCpuStat.class); - when(builder.build(0)).thenReturn(stat0); - when(builder.build(1)).thenReturn(stat1); + when(builder.build("vm1", 0)).thenReturn(stat0); + when(builder.build("vm2", 1)).thenReturn(stat1); backend.setVmCpuStatBuilder(builder); backend.activate(); @@ -145,8 +145,8 @@ verify(executor).scheduleAtFixedRate(captor.capture(), any(Long.class), any(Long.class), any(TimeUnit.class)); assertTrue(backend.isActive()); - backend.vmStatusChanged(Status.VM_ACTIVE, 0); - backend.vmStatusChanged(Status.VM_STARTED, 1); + backend.vmStatusChanged(Status.VM_ACTIVE, "vm1", 0); + backend.vmStatusChanged(Status.VM_STARTED, "vm2", 1); Runnable runnable = captor.getValue(); runnable.run(); @@ -158,8 +158,8 @@ verify(vmCpuStatDao).putVmCpuStat(stat0); verify(vmCpuStatDao).putVmCpuStat(stat1); - backend.vmStatusChanged(Status.VM_STOPPED, 0); - backend.vmStatusChanged(Status.VM_STOPPED, 1); + backend.vmStatusChanged(Status.VM_STOPPED, "vm1", 0); + backend.vmStatusChanged(Status.VM_STOPPED, "vm2", 1); verify(builder).forgetAbout(0); verify(builder).forgetAbout(1); diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/agent/src/test/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuStatBuilderTest.java --- a/vm-cpu/agent/src/test/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuStatBuilderTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/agent/src/test/java/com/redhat/thermostat/vm/cpu/agent/internal/VmCpuStatBuilderTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -77,7 +77,7 @@ long ticksPerSecond = 0; ProcessStatusInfoBuilder statusBuilder = mock(ProcessStatusInfoBuilder.class); VmCpuStatBuilder builder = new VmCpuStatBuilder(clock, cpuCount, ticksPerSecond, statusBuilder); - builder.build(0); + builder.build("vmId", 0); } @Test @@ -99,11 +99,12 @@ VmCpuStatBuilder builder = new VmCpuStatBuilder(clock, cpuCount, ticksPerSecond, statusBuilder); builder.learnAbout(PID); - assertEquals(null, builder.build(PID)); + assertEquals(null, builder.build("vmId", PID)); } @Test public void testSaneBuild() { + final String VM_ID = "vmId"; final int PID = 0; final int CPU_COUNT = 3; @@ -139,10 +140,10 @@ VmCpuStatBuilder builder = new VmCpuStatBuilder(clock, CPU_COUNT, TICKS_PER_SECOND, statusBuilder); builder.learnAbout(PID); - VmCpuStat stat = builder.build(PID); + VmCpuStat stat = builder.build(VM_ID, PID); assertNotNull(stat); - assertEquals(PID, stat.getVmId()); + assertEquals(VM_ID, stat.getVmId()); assertEquals(CLOCK2, stat.getTimeStamp()); assertEquals(CPU_LOAD_PERCENT, stat.getCpuLoad(), 0.0001); } diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/client-cli/src/test/java/com/redhat/thermostat/vm/cpu/client/cli/internal/VmCpuStatPrintDelegateTest.java --- a/vm-cpu/client-cli/src/test/java/com/redhat/thermostat/vm/cpu/client/cli/internal/VmCpuStatPrintDelegateTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/client-cli/src/test/java/com/redhat/thermostat/vm/cpu/client/cli/internal/VmCpuStatPrintDelegateTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -90,9 +90,9 @@ private void setupDAOs() { vmCpuStatDAO = mock(VmCpuStatDAO.class); - int vmId = 234; + String vmId = "vmId"; HostRef host = new HostRef("123", "dummy"); - vm = new VmRef(host, 234, "dummy"); + vm = new VmRef(host, vmId, 234, "dummy"); VmCpuStat cpustat1 = new VmCpuStat(2, vmId, 65); VmCpuStat cpustat2 = new VmCpuStat(3, vmId, 70); cpuStats = Arrays.asList(cpustat1, cpustat2); diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/client-core/src/test/java/com/redhat/thermostat/vm/cpu/client/core/internal/VmCpuControllerTest.java --- a/vm-cpu/client-core/src/test/java/com/redhat/thermostat/vm/cpu/client/core/internal/VmCpuControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/client-core/src/test/java/com/redhat/thermostat/vm/cpu/client/core/internal/VmCpuControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -68,7 +68,7 @@ @Test public void testChartUpdate() { - VmCpuStat stat1 = new VmCpuStat(123, 12345, 50.5); + VmCpuStat stat1 = new VmCpuStat(123, "vmId", 50.5); List stats = new ArrayList(); stats.add(stat1); diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/common/src/main/java/com/redhat/thermostat/vm/cpu/common/model/VmCpuStat.java --- a/vm-cpu/common/src/main/java/com/redhat/thermostat/vm/cpu/common/model/VmCpuStat.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/common/src/main/java/com/redhat/thermostat/vm/cpu/common/model/VmCpuStat.java Thu Jul 18 18:33:49 2013 -0400 @@ -45,14 +45,14 @@ public class VmCpuStat extends BasePojo implements TimeStampedPojo { private long timeStamp; - private int vmId; + private String vmId; private double cpuLoad; public VmCpuStat() { super(); } - public VmCpuStat(long timeStamp, int vmId, double cpuLoad) { + public VmCpuStat(long timeStamp, String vmId, double cpuLoad) { this.timeStamp = timeStamp; this.vmId = vmId; this.cpuLoad = cpuLoad; @@ -70,12 +70,12 @@ } @Persist - public int getVmId() { + public String getVmId() { return vmId; } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-cpu/common/src/test/java/com/redhat/thermostat/vm/cpu/common/internal/VmCpuStatDAOTest.java --- a/vm-cpu/common/src/test/java/com/redhat/thermostat/vm/cpu/common/internal/VmCpuStatDAOTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-cpu/common/src/test/java/com/redhat/thermostat/vm/cpu/common/internal/VmCpuStatDAOTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -67,7 +67,7 @@ public class VmCpuStatDAOTest { private static final Long TIMESTAMP = 1234L; - private static final Integer VM_ID = 321; + private static final String VM_ID = "321"; private static final Double CPU_LOAD = 9.9; private VmCpuStat cpuStat; @@ -106,15 +106,15 @@ when(hostRef.getAgentId()).thenReturn("system"); VmRef vmRef = mock(VmRef.class); - when(vmRef.getAgent()).thenReturn(hostRef); - when(vmRef.getId()).thenReturn(VM_ID); + when(vmRef.getHostRef()).thenReturn(hostRef); + when(vmRef.getVmId()).thenReturn(VM_ID); VmCpuStatDAO dao = new VmCpuStatDAOImpl(storage); List vmCpuStats = dao.getLatestVmCpuStats(vmRef, Long.MIN_VALUE); verify(storage).prepareStatement(anyDescriptor()); verify(stmt).setString(0, "system"); - verify(stmt).setInt(1, VM_ID); + verify(stmt).setString(1, VM_ID); verify(stmt).setLong(2, Long.MIN_VALUE); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -123,7 +123,7 @@ VmCpuStat stat = vmCpuStats.get(0); assertEquals(TIMESTAMP, (Long) stat.getTimeStamp()); assertEquals(CPU_LOAD, stat.getCpuLoad(), 0.001); - assertEquals(VM_ID, (Integer) stat.getVmId()); + assertEquals(VM_ID, stat.getVmId()); } @SuppressWarnings("unchecked") diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackend.java --- a/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackend.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -59,8 +59,8 @@ } @Override - protected VmUpdateListener createVmListener(int pid) { - return new VmGcVmListener(vmGcStats, pid); + protected VmUpdateListener createVmListener(String vmId, int pid) { + return new VmGcVmListener(vmGcStats, vmId); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcVmListener.java --- a/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcVmListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcVmListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -50,10 +50,10 @@ private static final Logger logger = LoggingUtils.getLogger(VmGcVmListener.class); - private final int vmId; + private final String vmId; private final VmGcStatDAO gcDAO; - public VmGcVmListener(VmGcStatDAO vmGcStatDao, int vmId) { + public VmGcVmListener(VmGcStatDAO vmGcStatDao, String vmId) { gcDAO = vmGcStatDao; this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcVmListenerTest.java --- a/vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcVmListenerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcVmListenerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -65,7 +65,7 @@ public void setup() throws VmUpdateException { final int numGCs = 2; vmGcStatDAO = mock(VmGcStatDAO.class); - vmListener = new VmGcVmListener(vmGcStatDAO, 0); + vmListener = new VmGcVmListener(vmGcStatDAO, "vmId"); extractor = mock(VmGcDataExtractor.class); diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/client-core/src/test/java/com/redhat/thermostat/vm/gc/client/core/internal/VmGcControllerTest.java --- a/vm-gc/client-core/src/test/java/com/redhat/thermostat/vm/gc/client/core/internal/VmGcControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/client-core/src/test/java/com/redhat/thermostat/vm/gc/client/core/internal/VmGcControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -90,8 +90,8 @@ // Set up fake data List stats = new ArrayList<>(); - VmGcStat stat1 = new VmGcStat(42, 1, "collector1", 1, 10); - VmGcStat stat2 = new VmGcStat(42, 2, "collector1", 5, 20); + VmGcStat stat1 = new VmGcStat("vmId", 1, "collector1", 1, 10); + VmGcStat stat2 = new VmGcStat("vmId", 2, "collector1", 5, 20); stats.add(stat1); stats.add(stat2); @@ -99,7 +99,7 @@ gen = new Generation(); gen.setName("generation 1"); gen.setCollector("collector1"); - VmMemoryStat memoryStat = new VmMemoryStat(1, 42, new Generation[] { gen }); + VmMemoryStat memoryStat = new VmMemoryStat(1, "vmId", new Generation[] { gen }); // Setup DAO VmGcStatDAO vmGcStatDAO = mock(VmGcStatDAO.class); diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/common/src/main/java/com/redhat/thermostat/vm/gc/common/model/VmGcStat.java --- a/vm-gc/common/src/main/java/com/redhat/thermostat/vm/gc/common/model/VmGcStat.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/common/src/main/java/com/redhat/thermostat/vm/gc/common/model/VmGcStat.java Thu Jul 18 18:33:49 2013 -0400 @@ -45,7 +45,7 @@ public class VmGcStat extends BasePojo implements TimeStampedPojo { private long timeStamp; - private int vmId; + private String vmId; private String collectorName; private long runCount; private long wallTime; @@ -54,7 +54,7 @@ super(); } - public VmGcStat(int vmId, long timestamp, String collectorName, long runCount, long wallTime) { + public VmGcStat(String vmId, long timestamp, String collectorName, long runCount, long wallTime) { this.timeStamp = timestamp; this.vmId = vmId; this.collectorName = collectorName; @@ -63,12 +63,12 @@ } @Persist - public int getVmId() { + public String getVmId() { return vmId; } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/common/src/test/java/com/redhat/thermostat/vm/gc/common/internal/VmGcStatDAOTest.java --- a/vm-gc/common/src/test/java/com/redhat/thermostat/vm/gc/common/internal/VmGcStatDAOTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/common/src/test/java/com/redhat/thermostat/vm/gc/common/internal/VmGcStatDAOTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -65,7 +65,7 @@ public class VmGcStatDAOTest { - private static final Integer VM_ID = 123; + private static final String VM_ID = "VM321"; private static final Long TIMESTAMP = 456L; private static final String COLLECTOR = "collector1"; private static final Long RUN_COUNT = 10L; @@ -104,15 +104,15 @@ when(hostRef.getAgentId()).thenReturn("system"); VmRef vmRef = mock(VmRef.class); - when(vmRef.getAgent()).thenReturn(hostRef); - when(vmRef.getId()).thenReturn(321); + when(vmRef.getHostRef()).thenReturn(hostRef); + when(vmRef.getVmId()).thenReturn("VM321"); VmGcStatDAO dao = new VmGcStatDAOImpl(storage); List vmGcStats = dao.getLatestVmGcStats(vmRef, Long.MIN_VALUE); verify(storage).prepareStatement(anyDescriptor()); verify(stmt).setString(0, "system"); - verify(stmt).setInt(1, 321); + verify(stmt).setString(1, "VM321"); verify(stmt).setLong(2, Long.MIN_VALUE); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -120,7 +120,7 @@ assertEquals(1, vmGcStats.size()); VmGcStat stat = vmGcStats.get(0); assertEquals(TIMESTAMP, (Long) stat.getTimeStamp()); - assertEquals(VM_ID, (Integer) stat.getVmId()); + assertEquals(VM_ID, stat.getVmId()); assertEquals(COLLECTOR, stat.getCollectorName()); assertEquals(RUN_COUNT, (Long) stat.getRunCount()); assertEquals(WALL_TIME, (Long) stat.getWallTime()); diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/remote-collector-client-common/src/main/java/com/redhat/thermostat/gc/remote/common/GCRequest.java --- a/vm-gc/remote-collector-client-common/src/main/java/com/redhat/thermostat/gc/remote/common/GCRequest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/remote-collector-client-common/src/main/java/com/redhat/thermostat/gc/remote/common/GCRequest.java Thu Jul 18 18:33:49 2013 -0400 @@ -58,7 +58,7 @@ public void sendGCRequestToAgent(VmRef vm, AgentInfoDAO agentDAO, RequestResponseListener responseListener) { - HostRef targetHostRef = vm.getAgent(); + HostRef targetHostRef = vm.getHostRef(); String address = agentDAO.getAgentInformation(targetHostRef).getConfigListenAddress(); String [] host = address.split(":"); @@ -70,7 +70,7 @@ gcRequest.setParameter(Request.ACTION, CMD_CHANNEL_ACTION_NAME); gcRequest.setParameter(GCCommand.class.getName(), GCCommand.REQUEST_GC.name()); - gcRequest.setParameter(GCCommand.VM_ID, vm.getIdString()); + gcRequest.setParameter(GCCommand.VM_PID, String.valueOf(vm.getPid())); gcRequest.addListener(responseListener); diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/remote-collector-client-common/src/test/java/com/redhat/thermostat/gc/remote/common/GCRequestTest.java --- a/vm-gc/remote-collector-client-common/src/test/java/com/redhat/thermostat/gc/remote/common/GCRequestTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/remote-collector-client-common/src/test/java/com/redhat/thermostat/gc/remote/common/GCRequestTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -75,8 +75,8 @@ listener = mock(RequestResponseListener.class); HostRef ref = mock(HostRef.class); - when(vm.getAgent()).thenReturn(ref); - when(vm.getIdString()).thenReturn("123456"); + when(vm.getHostRef()).thenReturn(ref); + when(vm.getPid()).thenReturn(123456); AgentInformation info = mock(AgentInformation.class); when(info.getConfigListenAddress()).thenReturn("0.0.42.42:42"); @@ -106,8 +106,8 @@ }; gcRequest.sendGCRequestToAgent(vm, agentDAO, listener); - verify(vm).getAgent(); - verify(vm).getIdString(); + verify(vm).getHostRef(); + verify(vm).getPid(); assertTrue(results[0]); assertTrue(results[1]); @@ -115,7 +115,7 @@ verify(request).setReceiver(GCCommand.RECEIVER); verify(request).setParameter(GCCommand.class.getName(), GCCommand.REQUEST_GC.name()); - verify(request).setParameter(GCCommand.VM_ID, "123456"); + verify(request).setParameter(GCCommand.VM_PID, "123456"); verify(request).addListener(listener); verify(queue).putRequest(request); diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/remote-collector-command/src/main/java/com/redhat/thermostat/gc/remote/command/GCCommandReceiver.java --- a/vm-gc/remote-collector-command/src/main/java/com/redhat/thermostat/gc/remote/command/GCCommandReceiver.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/remote-collector-command/src/main/java/com/redhat/thermostat/gc/remote/command/GCCommandReceiver.java Thu Jul 18 18:33:49 2013 -0400 @@ -36,18 +36,22 @@ package com.redhat.thermostat.gc.remote.command; +import java.util.logging.Level; +import java.util.logging.Logger; + import com.redhat.thermostat.agent.command.RequestReceiver; import com.redhat.thermostat.common.command.Request; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; +import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.gc.remote.command.internal.GC; import com.redhat.thermostat.gc.remote.command.internal.GCException; import com.redhat.thermostat.gc.remote.common.command.GCCommand; -import com.redhat.thermostat.utils.management.MXBeanConnection; import com.redhat.thermostat.utils.management.MXBeanConnectionPool; public class GCCommandReceiver implements RequestReceiver { + private static final Logger logger = LoggingUtils.getLogger(GCCommandReceiver.class); private MXBeanConnectionPool pool; public GCCommandReceiver(MXBeanConnectionPool pool) { @@ -61,11 +65,16 @@ String command = request.getParameter(GCCommand.class.getName()); switch (GCCommand.valueOf(command)) { case REQUEST_GC: - String vmId = request.getParameter(GCCommand.VM_ID); + String strPid = request.getParameter(GCCommand.VM_PID); try { + int vmId = Integer.parseInt(strPid); new GC(pool, vmId).gc(); } catch (GCException gce) { response = new Response(ResponseType.ERROR); + logger.log(Level.WARNING, "GC request failed", gce); + } catch (NumberFormatException e) { + response = new Response(ResponseType.ERROR); + logger.log(Level.WARNING, "Invalid PID: " + strPid, e); } break; default: diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/remote-collector-command/src/main/java/com/redhat/thermostat/gc/remote/command/internal/GC.java --- a/vm-gc/remote-collector-command/src/main/java/com/redhat/thermostat/gc/remote/command/internal/GC.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/remote-collector-command/src/main/java/com/redhat/thermostat/gc/remote/command/internal/GC.java Thu Jul 18 18:33:49 2013 -0400 @@ -52,9 +52,9 @@ private MXBeanConnectionPool pool; private int vmId; - public GC(MXBeanConnectionPool pool, String vmId) { + public GC(MXBeanConnectionPool pool, int vmId) { this.pool = pool; - this.vmId = Integer.valueOf(vmId); + this.vmId = vmId; } public void gc() throws GCException { diff -r 53d3cab77025 -r f7961cf1e440 vm-gc/remote-collector-common/src/main/java/com/redhat/thermostat/gc/remote/common/command/GCCommand.java --- a/vm-gc/remote-collector-common/src/main/java/com/redhat/thermostat/gc/remote/common/command/GCCommand.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-gc/remote-collector-common/src/main/java/com/redhat/thermostat/gc/remote/common/command/GCCommand.java Thu Jul 18 18:33:49 2013 -0400 @@ -40,7 +40,7 @@ REQUEST_GC; - public static final String VM_ID = "VM_ID"; + public static final String VM_PID = "VM_PID"; public static final String RECEIVER = "com.redhat.thermostat.gc.remote.command.GCCommandReceiver"; } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiver.java --- a/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiver.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiver.java Thu Jul 18 18:33:49 2013 -0400 @@ -77,41 +77,45 @@ @Override public Response receive(Request request) { String vmId = request.getParameter("vmId"); + String strPid = request.getParameter("vmPid"); try { - File heapDumpFile = dumpHeap(vmId); + int vmPid = Integer.parseInt(strPid); + File heapDumpFile = dumpHeap(vmPid); ObjectHistogram histogram = loadHistogram(heapDumpFile.getAbsolutePath()); saveHeapDumpInfo(vmId, heapDumpFile, histogram); - } catch (IOException e) { log.log(Level.SEVERE, "Unexpected IO problem while writing heap dump", e); return new Response(ResponseType.ERROR); } catch (HeapDumpException e) { log.log(Level.SEVERE, "Unexpected problem while writing heap dump", e); return new Response(ResponseType.ERROR); + } catch (NumberFormatException e) { + log.log(Level.WARNING, "Invalid PID: " + strPid, e); + return new Response(ResponseType.ERROR); } return new Response(ResponseType.OK); } - private File dumpHeap(String vmId) throws IOException, HeapDumpException { + private File dumpHeap(int pid) throws IOException, HeapDumpException { File tempFile = Files.createTempFile("thermostat-", "-heapdump").toFile(); String tempFileName = tempFile.getAbsolutePath(); tempFile.delete(); // Need to delete before dumping heap, jmap does not override existing file and stop with an error. - dumpHeapUsingJMX(vmId, tempFileName); + dumpHeapUsingJMX(pid, tempFileName); return tempFile; } - private void dumpHeapUsingJMX(String vmId, String filename) throws HeapDumpException { + private void dumpHeapUsingJMX(int pid, String filename) throws HeapDumpException { try { - jmxHeapDumper.dumpHeap(vmId, filename); + jmxHeapDumper.dumpHeap(pid, filename); } catch (HeapDumpException e) { log.log(Level.WARNING, "Heap dump using JMX failed, trying jmap", e); - dumpHeapUsingJMap(vmId, filename); + dumpHeapUsingJMap(pid, filename); } } - private void dumpHeapUsingJMap(String vmId, String filename) throws HeapDumpException { - jmapHeapDumper.dumpHeap(vmId, filename); + private void dumpHeapUsingJMap(int pid, String filename) throws HeapDumpException { + jmapHeapDumper.dumpHeap(pid, filename); } private ObjectHistogram loadHistogram(String heapDumpFilename) throws IOException { @@ -119,7 +123,7 @@ } private void saveHeapDumpInfo(String vmId, File tempFile, ObjectHistogram histogram) throws IOException { - HeapInfo heapInfo = new HeapInfo(Integer.parseInt(vmId), System.currentTimeMillis()); + HeapInfo heapInfo = new HeapInfo(vmId, System.currentTimeMillis()); heapDao.putHeapInfo(heapInfo, tempFile, histogram); } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumper.java --- a/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumper.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumper.java Thu Jul 18 18:33:49 2013 -0400 @@ -52,17 +52,17 @@ private static final String CONNECTOR_ADDRESS_PROPERTY = "com.sun.management.jmxremote.localConnectorAddress"; - void dumpHeap(String vmId, String filename) throws HeapDumpException { + void dumpHeap(int pid, String filename) throws HeapDumpException { try { - doHeapDump(vmId, filename); + doHeapDump(pid, filename); } catch (Exception ex) { throw new HeapDumpException(ex); } } - private void doHeapDump(String vmId, String filename) throws Exception { + private void doHeapDump(int pid, String filename) throws Exception { - VirtualMachine vm = VirtualMachine.attach(vmId); + VirtualMachine vm = VirtualMachine.attach(String.valueOf(pid)); String connectorAddress = getConnectorAddress(vm); JMXServiceURL url = new JMXServiceURL(connectorAddress); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMapHeapDumper.java --- a/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMapHeapDumper.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMapHeapDumper.java Thu Jul 18 18:33:49 2013 -0400 @@ -46,10 +46,10 @@ private static final Logger log = LoggingUtils.getLogger(JMapHeapDumper.class); - void dumpHeap(String vmId, String filename) throws HeapDumpException { + void dumpHeap(int pid, String filename) throws HeapDumpException { try { Process proc = Runtime.getRuntime().exec( - new String[] { "jmap", "-dump:format=b,file=" + filename, vmId }); + new String[] { "jmap", "-dump:format=b,file=" + filename, String.valueOf(pid) }); proc.waitFor(); log.info("Heap dump written to: " + filename); } catch (InterruptedException e) { diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiverTest.java --- a/vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiverTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiverTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -37,6 +37,7 @@ package com.redhat.thermostat.vm.heap.analysis.agent.internal; import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.same; @@ -76,7 +77,8 @@ heapDAO = mock(HeapDAO.class); request = mock(Request.class); - when(request.getParameter("vmId")).thenReturn("123"); + when(request.getParameter("vmId")).thenReturn("vmId"); + when(request.getParameter("vmPid")).thenReturn("42"); jmxDumper = mock(JMXHeapDumper.class); jmapDumper = mock(JMapHeapDumper.class); histogramLoader = mock(HistogramLoader.class); @@ -102,29 +104,29 @@ assertEquals(ResponseType.OK, response.getType()); ArgumentCaptor filename = ArgumentCaptor.forClass(String.class); - verify(jmxDumper).dumpHeap(eq("123"), filename.capture()); + verify(jmxDumper).dumpHeap(eq(42), filename.capture()); verify(histogramLoader).load(filename.getValue()); ArgumentCaptor heapInfo = ArgumentCaptor.forClass(HeapInfo.class); verify(heapDAO).putHeapInfo(heapInfo.capture(), eq(new File(filename.getValue())), same(expectedHistogramData)); - assertEquals(123, heapInfo.getValue().getVmId()); + assertEquals("vmId", heapInfo.getValue().getVmId()); } @Test public void verifyFallbackWhenJMXFails() throws HeapDumpException { - doThrow(new HeapDumpException()).when(jmxDumper).dumpHeap(anyString(), anyString()); + doThrow(new HeapDumpException()).when(jmxDumper).dumpHeap(anyInt(), anyString()); Response response = receiver.receive(request); assertEquals(ResponseType.OK, response.getType()); - verify(jmxDumper).dumpHeap(eq("123"), anyString()); + verify(jmxDumper).dumpHeap(eq(42), anyString()); } @Test public void verifyResponseTypeWhenAllFails() throws HeapDumpException { - doThrow(new HeapDumpException()).when(jmxDumper).dumpHeap(anyString(), anyString()); - doThrow(new HeapDumpException()).when(jmapDumper).dumpHeap(anyString(), anyString()); + doThrow(new HeapDumpException()).when(jmxDumper).dumpHeap(anyInt(), anyString()); + doThrow(new HeapDumpException()).when(jmapDumper).dumpHeap(anyInt(), anyString()); Response response = receiver.receive(request); assertEquals(ResponseType.ERROR, response.getType()); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumper.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumper.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumper.java Thu Jul 18 18:33:49 2013 -0400 @@ -61,7 +61,7 @@ public void dump() throws CommandException { ServiceReference launcherRef = context.getServiceReference(Launcher.class.getName()); Launcher launcher = (Launcher) context.getService(launcherRef); - launcher.run(new String[] { "dump-heap", "--hostId", ref.getAgent().getStringID(), "--vmId", ref.getStringID()}, true); + launcher.run(new String[] { "dump-heap", "--hostId", ref.getHostRef().getStringID(), "--vmId", ref.getVmId() }, true); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpControllerTest.java --- a/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -201,8 +201,8 @@ when(hostRef.getAgentId()).thenReturn("agent-id"); VmRef ref = mock(VmRef.class); - when(ref.getIdString()).thenReturn("vm-id"); - when(ref.getAgent()).thenReturn(hostRef); + when(ref.getVmId()).thenReturn("vm-id"); + when(ref.getHostRef()).thenReturn(hostRef); controller = new HeapDumpController(vmDao, vmInfoDao, heapDao, ref, appService, viewProvider, detailsViewProvider, histogramProvider, diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumperTest.java --- a/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumperTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumperTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -54,7 +54,8 @@ public class HeapDumperTest { private static final String TEST_HOST_ID = "1111111"; - private static final int TEST_VM_ID = 2222222; + private static final String TEST_VM_ID = "vmId"; + private static final Integer TEST_VM_PID = 2222222; private HeapDumper dumper; @@ -69,7 +70,7 @@ bundleContext.registerService(Launcher.class, launcher, null); HostRef hostRef = new HostRef(TEST_HOST_ID, "myHost"); - VmRef ref = new VmRef(hostRef, TEST_VM_ID, "myVM"); + VmRef ref = new VmRef(hostRef, TEST_VM_ID, TEST_VM_PID, "myVM"); dumper = new HeapDumper(ref, bundleContext); } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Thu Jul 18 18:33:49 2013 -0400 @@ -49,6 +49,7 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.shared.locale.Translate; import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; @@ -85,11 +86,17 @@ public void run() { ex[0] = new CommandException(translator.localize( LocaleResources.HEAP_DUMP_ERROR, args.getHost() - .getStringID(), args.getVM().getStringID())); + .getStringID(), args.getVM().getVmId())); s.release(); } }; + ServiceReference vmInfoRef = context.getServiceReference(VmInfoDAO.class.getName()); + if (vmInfoRef == null) { + throw new CommandException(translator.localize(LocaleResources.VM_SERVICE_UNAVAILABLE)); + } + VmInfoDAO vmInfoDAO = (VmInfoDAO) context.getService(vmInfoRef); + ServiceReference agentInfoRef = context.getServiceReference(AgentInfoDAO.class.getName()); if (agentInfoRef == null) { throw new CommandException(translator.localize(LocaleResources.AGENT_SERVICE_UNAVAILABLE)); @@ -102,8 +109,9 @@ } RequestQueue queue = (RequestQueue) context.getService(requestQueueRef); - implementation.execute(agentInfoDAO, args.getVM(), queue, successHandler, errorHandler); + implementation.execute(vmInfoDAO, agentInfoDAO, args.getVM(), queue, successHandler, errorHandler); + context.ungetService(vmInfoRef); context.ungetService(agentInfoRef); context.ungetService(requestQueueRef); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelper.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelper.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelper.java Thu Jul 18 18:33:49 2013 -0400 @@ -47,12 +47,15 @@ import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.dao.VmInfoDAO; +import com.redhat.thermostat.storage.model.VmInfo; public class DumpHeapHelper { private static final String RECEIVER_CLASS_NAME = "com.redhat.thermostat.vm.heap.analysis.agent.internal.HeapDumpReceiver"; private static final String CMD_CHANNEL_ACTION_NAME = "dump-heap"; private static final String VM_ID_PARAM = "vmId"; + private static final String VM_PID_PARAM = "vmPid"; private class HeapDumpListener implements RequestResponseListener { @@ -79,10 +82,14 @@ } - public void execute(AgentInfoDAO agentInfoDAO, VmRef reference, + public void execute(VmInfoDAO vmInfoDAO, AgentInfoDAO agentInfoDAO, VmRef reference, RequestQueue queue, Runnable heapDumpSuccessAction, Runnable heapDumpFailureAction) { - HostRef targetHostRef = reference.getAgent(); + // Get PID + VmInfo info = vmInfoDAO.getVmInfo(reference); + int pid = info.getVmPid(); + + HostRef targetHostRef = reference.getHostRef(); String address = agentInfoDAO.getAgentInformation(targetHostRef).getConfigListenAddress(); String [] host = address.split(":"); @@ -90,7 +97,8 @@ Request req = new Request(RequestType.RESPONSE_EXPECTED, target); req.setReceiver(RECEIVER_CLASS_NAME); req.setParameter(Request.ACTION, CMD_CHANNEL_ACTION_NAME); - req.setParameter(VM_ID_PARAM, reference.getIdString()); + req.setParameter(VM_ID_PARAM, reference.getVmId()); + req.setParameter(VM_PID_PARAM, String.valueOf(pid)); req.addListener(new HeapDumpListener(heapDumpSuccessAction, heapDumpFailureAction)); queue.putRequest(req); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java Thu Jul 18 18:33:49 2013 -0400 @@ -124,8 +124,8 @@ private void printDumpsForVm(HeapDAO heapDAO, HostRef hostRef, VmRef vmRef, TableRenderer renderer) { Collection infos = heapDAO.getAllHeapInfo(vmRef); for (HeapInfo info : infos) { - renderer.printLine(hostRef.getStringID(), - vmRef.getStringID(), + renderer.printLine(hostRef.getAgentId(), + vmRef.getVmId(), info.getHeapId(), new Date(info.getTimeStamp()).toString()); } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -57,6 +57,7 @@ import com.redhat.thermostat.shared.locale.Translate; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.test.TestCommandContextFactory; import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; @@ -68,9 +69,11 @@ @Test public void verifyAcuallyCallsWorker() throws CommandException { + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); RequestQueue queue = mock(RequestQueue.class); StubBundleContext context = new StubBundleContext(); + context.registerService(VmInfoDAO.class, vmInfoDao, null); context.registerService(AgentInfoDAO.class, agentInfoDao, null); context.registerService(RequestQueue.class, queue, null); @@ -84,7 +87,7 @@ successHandler.getValue().run(); return null; } - }).when(impl).execute(eq(agentInfoDao), any(VmRef.class), eq(queue), + }).when(impl).execute(eq(vmInfoDao), eq(agentInfoDao), any(VmRef.class), eq(queue), successHandler.capture(), any(Runnable.class)); DumpHeapCommand command = new DumpHeapCommand(context, impl); @@ -93,20 +96,22 @@ SimpleArguments args = new SimpleArguments(); args.addArgument("hostId", "foo"); - args.addArgument("vmId", "0"); + args.addArgument("vmId", "bar"); command.run(factory.createContext(args)); - verify(impl).execute(eq(agentInfoDao), isA(VmRef.class), eq(queue), + verify(impl).execute(eq(vmInfoDao), eq(agentInfoDao), isA(VmRef.class), eq(queue), any(Runnable.class), any(Runnable.class)); assertEquals("Done\n", factory.getOutput()); } @Test public void verifyNeedsHostAndVmId() throws CommandException { + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); RequestQueue queue = mock(RequestQueue.class); StubBundleContext context = new StubBundleContext(); + context.registerService(VmInfoDAO.class, vmInfoDao, null); context.registerService(AgentInfoDAO.class, agentInfoDao, null); context.registerService(RequestQueue.class, queue, null); @@ -127,8 +132,10 @@ @Test public void verifyFailsIfAgentDaoIsNotAvailable() { + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); RequestQueue queue = mock(RequestQueue.class); StubBundleContext context = new StubBundleContext(); + context.registerService(VmInfoDAO.class, vmInfoDao, null); context.registerService(RequestQueue.class, queue, null); DumpHeapHelper impl = mock(DumpHeapHelper.class); @@ -138,7 +145,7 @@ SimpleArguments args = new SimpleArguments(); args.addArgument("hostId", "foo"); - args.addArgument("vmId", "0"); + args.addArgument("vmId", "bar"); try { command.run(factory.createContext(args)); @@ -150,8 +157,10 @@ @Test public void verifyFailsIfRequestQueueIsNotAvailable() { + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); StubBundleContext context = new StubBundleContext(); + context.registerService(VmInfoDAO.class, vmInfoDao, null); context.registerService(AgentInfoDAO.class, agentInfoDao, null); DumpHeapHelper impl = mock(DumpHeapHelper.class); @@ -161,7 +170,8 @@ SimpleArguments args = new SimpleArguments(); args.addArgument("hostId", "foo"); - args.addArgument("vmId", "0"); + args.addArgument("vmId", "bar"); + args.addArgument("vmPid", "123"); try { command.run(factory.createContext(args)); @@ -170,14 +180,41 @@ assertEquals(TRANSLATOR.localize(LocaleResources.REQUEST_QUEUE_UNAVAILABLE).getContents(), ce.getMessage()); } } + + @Test + public void verifyFailsIfVmDaoIsNotAvailable() { + AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); + RequestQueue queue = mock(RequestQueue.class); + StubBundleContext context = new StubBundleContext(); + context.registerService(AgentInfoDAO.class, agentInfoDao, null); + context.registerService(RequestQueue.class, queue, null); + + DumpHeapHelper impl = mock(DumpHeapHelper.class); + DumpHeapCommand command = new DumpHeapCommand(context, impl); + + TestCommandContextFactory factory = new TestCommandContextFactory(); + + SimpleArguments args = new SimpleArguments(); + args.addArgument("hostId", "foo"); + args.addArgument("vmId", "bar"); + + try { + command.run(factory.createContext(args)); + fail(); + } catch (CommandException ce) { + assertEquals(TRANSLATOR.localize(LocaleResources.VM_SERVICE_UNAVAILABLE).getContents(), ce.getMessage()); + } + } @Test public void verifyErrorMessage() { final String HOST_ID = "myHost"; - final int VM_ID = 9001; + final String VM_ID = "myVm"; + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); RequestQueue queue = mock(RequestQueue.class); StubBundleContext context = new StubBundleContext(); + context.registerService(VmInfoDAO.class, vmInfoDao, null); context.registerService(AgentInfoDAO.class, agentInfoDao, null); context.registerService(RequestQueue.class, queue, null); @@ -187,25 +224,25 @@ SimpleArguments args = new SimpleArguments(); args.addArgument("hostId", HOST_ID); - args.addArgument("vmId", String.valueOf(VM_ID)); + args.addArgument("vmId", VM_ID); doAnswer(new Answer() { @Override public Object answer(InvocationOnMock invocation) throws Throwable { - Runnable failRunnable = (Runnable) invocation.getArguments()[4]; + Runnable failRunnable = (Runnable) invocation.getArguments()[5]; failRunnable.run(); return null; } - }).when(impl).execute(any(AgentInfoDAO.class), any(VmRef.class), any(RequestQueue.class), - any(Runnable.class), any(Runnable.class)); + }).when(impl).execute(any(VmInfoDAO.class), any(AgentInfoDAO.class), any(VmRef.class), + any(RequestQueue.class), any(Runnable.class), any(Runnable.class)); try { command.run(factory.createContext(args)); fail("CommandException expected"); } catch (CommandException e) { assertEquals(TRANSLATOR.localize(LocaleResources.HEAP_DUMP_ERROR, - HOST_ID, String.valueOf(VM_ID)).getContents(), e.getMessage()); + HOST_ID, VM_ID).getContents(), e.getMessage()); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelperTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelperTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelperTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -61,10 +61,13 @@ import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.AgentInformation; +import com.redhat.thermostat.storage.model.VmInfo; public class DumpHeapHelperTest { + private VmInfoDAO vmInfoDAO; private AgentInfoDAO agentInfoDao; private DumpHeapHelper cmd; private VmRef vmRef; @@ -86,8 +89,14 @@ cmd = new DumpHeapHelper(); vmRef = mock(VmRef.class); - when(vmRef.getIdString()).thenReturn("123"); - when(vmRef.getAgent()).thenReturn(host); + when(vmRef.getVmId()).thenReturn("vmId"); + when(vmRef.getHostRef()).thenReturn(host); + + VmInfo vmInfo = mock(VmInfo.class); + when(vmInfo.getVmPid()).thenReturn(123); + vmInfoDAO = mock(VmInfoDAO.class); + when(vmInfoDAO.getVmInfo(vmRef)).thenReturn(vmInfo); + heapDumpCompleteAction = mock(Runnable.class); heapDumpFailedAction = mock(Runnable.class); } @@ -103,7 +112,7 @@ @Test public void testExecute() { - cmd.execute(agentInfoDao, vmRef, reqQueue, heapDumpCompleteAction, heapDumpFailedAction); + cmd.execute(vmInfoDAO, agentInfoDao, vmRef, reqQueue, heapDumpCompleteAction, heapDumpFailedAction); ArgumentCaptor reqArg = ArgumentCaptor.forClass(Request.class); verify(reqQueue).putRequest(reqArg.capture()); @@ -111,7 +120,8 @@ assertEquals("com.redhat.thermostat.vm.heap.analysis.agent.internal.HeapDumpReceiver", req.getReceiver()); verifyClassExists(req.getReceiver()); assertEquals(RequestType.RESPONSE_EXPECTED, req.getType()); - assertEquals("123", req.getParameter("vmId")); + assertEquals("vmId", req.getParameter("vmId")); + assertEquals("123", req.getParameter("vmPid")); assertEquals(new InetSocketAddress("test", 123), req.getTarget()); Collection ls = req.getListeners(); @@ -125,7 +135,7 @@ @Test public void testExecuteFailure() { - cmd.execute(agentInfoDao, vmRef, reqQueue, heapDumpCompleteAction, heapDumpFailedAction); + cmd.execute(vmInfoDAO, agentInfoDao, vmRef, reqQueue, heapDumpCompleteAction, heapDumpFailedAction); ArgumentCaptor reqArg = ArgumentCaptor.forClass(Request.class); verify(reqQueue).putRequest(reqArg.capture()); @@ -133,7 +143,8 @@ assertEquals("com.redhat.thermostat.vm.heap.analysis.agent.internal.HeapDumpReceiver", req.getReceiver()); verifyClassExists(req.getReceiver()); assertEquals(RequestType.RESPONSE_EXPECTED, req.getType()); - assertEquals("123", req.getParameter("vmId")); + assertEquals("vmId", req.getParameter("vmId")); + assertEquals("123", req.getParameter("vmPid")); assertEquals(new InetSocketAddress("test", 123), req.getTarget()); Collection ls = req.getListeners(); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -112,9 +112,9 @@ @Test public void verifyWorks() throws CommandException { HostRef hostRef = mock(HostRef.class); - when(hostRef.getStringID()).thenReturn("host-id"); + when(hostRef.getAgentId()).thenReturn("host-id"); VmRef vmRef = mock(VmRef.class); - when(vmRef.getStringID()).thenReturn("1"); + when(vmRef.getVmId()).thenReturn("1"); HeapInfo heapInfo = mock(HeapInfo.class); Calendar timestamp = Calendar.getInstance(); @@ -152,12 +152,12 @@ HostRef hostRef1 = mock(HostRef.class); when(hostRef1.getStringID()).thenReturn("host1"); VmRef vmRef1 = mock(VmRef.class); - when(vmRef1.getStringID()).thenReturn("1"); + when(vmRef1.getVmId()).thenReturn("1"); HostRef hostRef2 = mock(HostRef.class); when(hostRef2.getStringID()).thenReturn("host2"); VmRef vmRef2 = mock(VmRef.class); - when(vmRef2.getStringID()).thenReturn("2"); + when(vmRef2.getVmId()).thenReturn("2"); HeapInfo heapInfo = mock(HeapInfo.class); Calendar timestamp = Calendar.getInstance(); @@ -201,12 +201,12 @@ when(hostRef1.getStringID()).thenReturn("host1"); when(hostRef1.getAgentId()).thenReturn("host1"); VmRef vmRef1 = mock(VmRef.class); - when(vmRef1.getStringID()).thenReturn("1"); + when(vmRef1.getVmId()).thenReturn("1"); HostRef hostRef2 = mock(HostRef.class); when(hostRef2.getStringID()).thenReturn("host2"); VmRef vmRef2 = mock(VmRef.class); - when(vmRef2.getStringID()).thenReturn("2"); + when(vmRef2.getVmId()).thenReturn("2"); HeapInfo heapInfo = mock(HeapInfo.class); Calendar timestamp = Calendar.getInstance(); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/internal/HeapDAOImpl.java --- a/vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/internal/HeapDAOImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/internal/HeapDAOImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -71,7 +71,7 @@ static final String QUERY_ALL_HEAPS = "QUERY " + heapInfoCategory.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i"; + + Key.VM_ID.getName() + "' = ?s"; static final String QUERY_HEAP_INFO = "QUERY " + heapInfoCategory.getName() + " WHERE '" + heapIdKey.getName() + "' = ?s LIMIT 1"; @@ -125,8 +125,8 @@ Cursor cursor; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, vm.getAgent().getAgentId()); - stmt.setInt(1, vm.getId()); + stmt.setString(0, vm.getHostRef().getAgentId()); + stmt.setString(1, vm.getVmId()); cursor = stmt.executeQuery(); } catch (DescriptorParsingException e) { // should not happen, but if it *does* happen, at least log it diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/model/HeapInfo.java --- a/vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/model/HeapInfo.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/model/HeapInfo.java Thu Jul 18 18:33:49 2013 -0400 @@ -46,7 +46,7 @@ @Entity public class HeapInfo extends BasePojo implements TimeStampedPojo { - private int vmId; + private String vmId; private long timeStamp; private String heapId; @@ -54,21 +54,21 @@ private String histogramId; public HeapInfo() { - this(-1, -1); + this(null, -1); } - public HeapInfo(int vmId, long timestamp) { + public HeapInfo(String vmId, long timestamp) { this.vmId = vmId; this.timeStamp = timestamp; } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } @Persist - public int getVmId() { + public String getVmId() { return vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/common/src/test/java/com/redhat/thermostat/vm/heap/analysis/common/internal/HeapDAOTest.java --- a/vm-heap-analysis/common/src/test/java/com/redhat/thermostat/vm/heap/analysis/common/internal/HeapDAOTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/common/src/test/java/com/redhat/thermostat/vm/heap/analysis/common/internal/HeapDAOTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -106,7 +106,7 @@ dao = new HeapDAOImpl(storage); - heapInfo = new HeapInfo(123, 12345); + heapInfo = new HeapInfo("vm1", 12345); byte[] data = new byte[] { 1, 2, 3 }; heapDumpData = File.createTempFile("test", "test"); FileOutputStream out = new FileOutputStream(heapDumpData); @@ -115,13 +115,13 @@ histogramData = createHistogramData(); Cursor cursor = (Cursor) mock(Cursor.class); - HeapInfo info1 = new HeapInfo(234, 12345L); + HeapInfo info1 = new HeapInfo("vm2", 12345L); info1.setAgentId("123"); info1.setHeapId("testheap1"); info1.setHeapDumpId("test1"); info1.setHistogramId("histotest1"); - HeapInfo info2 = new HeapInfo(234, 23456L); + HeapInfo info2 = new HeapInfo("vm2", 23456L); info2.setAgentId("123"); info2.setHeapId("testheap2"); info2.setHeapDumpId("test2"); @@ -188,7 +188,7 @@ public void preparedQueryDescriptorsAreSane() { String expectedQueryHeapInfo = "QUERY vm-heap-info WHERE 'heapId' = ?s LIMIT 1"; assertEquals(expectedQueryHeapInfo, HeapDAOImpl.QUERY_HEAP_INFO); - String expectedQueryAllHeaps = "QUERY vm-heap-info WHERE 'agentId' = ?s AND 'vmId' = ?i"; + String expectedQueryAllHeaps = "QUERY vm-heap-info WHERE 'agentId' = ?s AND 'vmId' = ?s"; assertEquals(expectedQueryAllHeaps, HeapDAOImpl.QUERY_ALL_HEAPS); } @@ -216,15 +216,15 @@ verify(add).apply(); ArgumentCaptor data = ArgumentCaptor.forClass(InputStream.class); - verify(storage).saveFile(eq("heapdump-test-123-12345"), data.capture()); + verify(storage).saveFile(eq("heapdump-test-vm1-12345"), data.capture()); InputStream in = data.getValue(); assertEquals(1, in.read()); assertEquals(2, in.read()); assertEquals(3, in.read()); assertEquals(-1, in.read()); - assertEquals("test-123-12345", heapInfo.getHeapId()); + assertEquals("test-vm1-12345", heapInfo.getHeapId()); ArgumentCaptor histoStream = ArgumentCaptor.forClass(InputStream.class); - verify(storage).saveFile(eq("histogram-test-123-12345"), histoStream.capture()); + verify(storage).saveFile(eq("histogram-test-vm1-12345"), histoStream.capture()); InputStream histoActual = histoStream.getValue(); int expected; int actual; @@ -244,7 +244,7 @@ verify(add).apply(); verify(storage, never()).saveFile(anyString(), any(InputStream.class)); - assertEquals("test-123-12345", heapInfo.getHeapId()); + assertEquals("test-vm1-12345", heapInfo.getHeapId()); } @Test @@ -254,21 +254,21 @@ verify(storage).registerCategory(HeapDAO.heapInfoCategory); HostRef host = new HostRef("123", "test-host"); - VmRef vm = new VmRef(host, 234, "test-vm"); + VmRef vm = new VmRef(host, "vm2", 234, "test-vm"); Collection heapInfos = dao.getAllHeapInfo(vm); verify(storage).prepareStatement(anyDescriptor()); verify(stmt).setString(0, "123"); - verify(stmt).setInt(1, 234); + verify(stmt).setString(1, "vm2"); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); - HeapInfo info1 = new HeapInfo(234, 12345); + HeapInfo info1 = new HeapInfo("vm2", 12345); info1.setHeapDumpId("test1"); info1.setHeapId("testheap1"); info1.setHistogramId("histotest1"); - HeapInfo info2 = new HeapInfo(234, 23456); + HeapInfo info2 = new HeapInfo("vm2", 23456); info2.setHeapDumpId("test2"); info2.setHeapId("testheap2"); info2.setHistogramId("histotest2"); @@ -289,7 +289,7 @@ verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); - HeapInfo info = new HeapInfo(234, 12345); + HeapInfo info = new HeapInfo("vm2", 12345); info.setHeapDumpId("test1"); info.setHeapId("testheap1"); info.setHistogramId("histotest1"); diff -r 53d3cab77025 -r f7961cf1e440 vm-heap-analysis/common/src/test/java/com/redhat/thermostat/vm/heap/analysis/common/model/HeapInfoTest.java --- a/vm-heap-analysis/common/src/test/java/com/redhat/thermostat/vm/heap/analysis/common/model/HeapInfoTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-heap-analysis/common/src/test/java/com/redhat/thermostat/vm/heap/analysis/common/model/HeapInfoTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -50,14 +50,14 @@ @Before public void setUp() { - heapInfo = new HeapInfo(321, 12345); + heapInfo = new HeapInfo("vmId", 12345); heapInfo.setAgentId("test-agent"); } @Test public void testProperties() { assertEquals("test-agent", heapInfo.getAgentId()); - assertEquals(321, heapInfo.getVmId()); + assertEquals("vmId", heapInfo.getVmId()); assertEquals(12345, heapInfo.getTimeStamp()); } diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/agent/src/main/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxBackend.java --- a/vm-jmx/agent/src/main/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxBackend.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/agent/src/main/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -135,8 +135,7 @@ return isActive; } - public void enableNotificationsFor(String vmId) { - int pid = Integer.valueOf(vmId); + public void enableNotificationsFor(int pid) { try { MXBeanConnection connection = pool.acquire(pid); connections.put(pid, connection); @@ -159,9 +158,7 @@ } } - public void disableNotificationsFor(String vmId) { - int pid = Integer.valueOf(vmId); - + public void disableNotificationsFor(int pid) { MXBeanConnection connection = connections.get(pid); JmxNotificationStatus update = new JmxNotificationStatus(); diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/agent/src/main/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxRequestListener.java --- a/vm-jmx/agent/src/main/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxRequestListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/agent/src/main/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxRequestListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -36,14 +36,19 @@ package com.redhat.thermostat.vm.jmx.agent.internal; +import java.util.logging.Level; +import java.util.logging.Logger; + import com.redhat.thermostat.agent.command.RequestReceiver; import com.redhat.thermostat.common.command.Request; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; +import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.vm.jmx.common.JmxCommand; public class JmxRequestListener implements RequestReceiver { + private static final Logger logger = LoggingUtils.getLogger(JmxRequestListener.class); private JmxBackend backend; public void setBackend(JmxBackend backend) { @@ -53,16 +58,22 @@ @Override public Response receive(Request request) { Response response = new Response(ResponseType.OK); - String vmId = request.getParameter(JmxCommand.VM_ID); - String command = request.getParameter(JmxCommand.class.getName()); + String strPid = request.getParameter(JmxCommand.VM_PID); + try { + int pid = Integer.parseInt(strPid); + String command = request.getParameter(JmxCommand.class.getName()); - switch (JmxCommand.valueOf(command)) { - case DISABLE_JMX_NOTIFICATIONS: - backend.disableNotificationsFor(vmId); - break; - case ENABLE_JMX_NOTIFICATIONS: - backend.enableNotificationsFor(vmId); - break; + switch (JmxCommand.valueOf(command)) { + case DISABLE_JMX_NOTIFICATIONS: + backend.disableNotificationsFor(pid); + break; + case ENABLE_JMX_NOTIFICATIONS: + backend.enableNotificationsFor(pid); + break; + } + } catch (NumberFormatException e) { + logger.log(Level.WARNING, "Invalid PID: " + strPid, e); + response = new Response(ResponseType.ERROR); } return response; diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/agent/src/test/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxBackendTest.java --- a/vm-jmx/agent/src/test/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxBackendTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/agent/src/test/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxBackendTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -145,9 +145,9 @@ MXBeanConnection connection = mock(MXBeanConnection.class); when(connection.get()).thenReturn(actual); - when(pool.acquire(0)).thenReturn(connection); + when(pool.acquire(42)).thenReturn(connection); - backend.enableNotificationsFor("0"); + backend.enableNotificationsFor(42); verify(actual).addNotificationListener(eq(name1), any(NotificationListener.class), eq((NotificationFilter) null), any()); } @@ -167,9 +167,9 @@ MXBeanConnection connection = mock(MXBeanConnection.class); when(connection.get()).thenReturn(actual); - when(pool.acquire(0)).thenReturn(connection); + when(pool.acquire(42)).thenReturn(connection); - backend.enableNotificationsFor("0"); + backend.enableNotificationsFor(42); ArgumentCaptor listenerCaptor = ArgumentCaptor.forClass(NotificationListener.class); ArgumentCaptor handbackCaptor = ArgumentCaptor.forClass(Object.class); @@ -210,9 +210,9 @@ MXBeanConnection connection = mock(MXBeanConnection.class); when(connection.get()).thenReturn(actual); - when(pool.acquire(0)).thenReturn(connection); + when(pool.acquire(42)).thenReturn(connection); - backend.enableNotificationsFor("0"); + backend.enableNotificationsFor(42); ArgumentCaptor listenerCaptor = ArgumentCaptor.forClass(NotificationListener.class); ArgumentCaptor handbackCaptor = ArgumentCaptor.forClass(Object.class); @@ -244,9 +244,9 @@ MXBeanConnection connection = mock(MXBeanConnection.class); when(connection.get()).thenReturn(actual); - when(pool.acquire(0)).thenReturn(connection); + when(pool.acquire(42)).thenReturn(connection); - backend.enableNotificationsFor("0"); + backend.enableNotificationsFor(42); ArgumentCaptor listenerCaptor = ArgumentCaptor.forClass(NotificationListener.class); ArgumentCaptor handbackCaptor = ArgumentCaptor.forClass(Object.class); @@ -272,12 +272,12 @@ MXBeanConnection connection = mock(MXBeanConnection.class); when(connection.get()).thenReturn(actual); - when(pool.acquire(0)).thenReturn(connection); + when(pool.acquire(42)).thenReturn(connection); - backend.enableNotificationsFor("0"); + backend.enableNotificationsFor(42); - backend.disableNotificationsFor("0"); + backend.disableNotificationsFor(42); - verify(pool).release(0, connection); + verify(pool).release(42, connection); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/agent/src/test/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxRequestListenerTest.java --- a/vm-jmx/agent/src/test/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxRequestListenerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/agent/src/test/java/com/redhat/thermostat/vm/jmx/agent/internal/JmxRequestListenerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -71,11 +71,11 @@ public void testEnableNotifications() { Request req = new Request(RequestType.RESPONSE_EXPECTED, new InetSocketAddress(HOST, PORT)); req.setParameter(JmxCommand.class.getName(), JmxCommand.ENABLE_JMX_NOTIFICATIONS.name()); - req.setParameter(JmxCommand.VM_ID, "0"); + req.setParameter(JmxCommand.VM_PID, "42"); Response result = requestListener.receive(req); - verify(backend).enableNotificationsFor("0"); + verify(backend).enableNotificationsFor(42); assertEquals(ResponseType.OK, result.getType()); } @@ -84,11 +84,11 @@ public void testDisableNotifications() { Request req = new Request(RequestType.RESPONSE_EXPECTED, new InetSocketAddress(HOST, PORT)); req.setParameter(JmxCommand.class.getName(), JmxCommand.DISABLE_JMX_NOTIFICATIONS.name()); - req.setParameter(JmxCommand.VM_ID, "0"); + req.setParameter(JmxCommand.VM_PID, "42"); Response result = requestListener.receive(req); - verify(backend).disableNotificationsFor("0"); + verify(backend).disableNotificationsFor(42); assertEquals(ResponseType.OK, result.getType()); } diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/client-core/src/main/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxToggleNotificationRequest.java --- a/vm-jmx/client-core/src/main/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxToggleNotificationRequest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/client-core/src/main/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxToggleNotificationRequest.java Thu Jul 18 18:33:49 2013 -0400 @@ -57,7 +57,7 @@ public void sendEnableNotificationsRequestToAgent(VmRef vm, AgentInfoDAO agentDAO, boolean enable, RequestResponseListener responseListener) { - HostRef targetHostRef = vm.getAgent(); + HostRef targetHostRef = vm.getHostRef(); String address = agentDAO.getAgentInformation(targetHostRef).getConfigListenAddress(); String[] host = address.split(":"); @@ -68,7 +68,7 @@ gcRequest.setReceiver(JmxCommand.RECEIVER); gcRequest.setParameter(JmxCommand.class.getName(), enable ? JmxCommand.ENABLE_JMX_NOTIFICATIONS.name() : JmxCommand.DISABLE_JMX_NOTIFICATIONS.name()); - gcRequest.setParameter(JmxCommand.VM_ID, vm.getIdString()); + gcRequest.setParameter(JmxCommand.VM_PID, String.valueOf(vm.getPid())); gcRequest.addListener(responseListener); diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/client-core/src/test/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxNotificationsViewControllerTest.java --- a/vm-jmx/client-core/src/test/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxNotificationsViewControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/client-core/src/test/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxNotificationsViewControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -95,7 +95,7 @@ host = mock(HostRef.class); vm = mock(VmRef.class); - when(vm.getAgent()).thenReturn(host); + when(vm.getHostRef()).thenReturn(host); controller = new JmxNotificationsViewController(agentDao, notificationDao, timerFactory, queue, viewProvider, vm); } diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/client-core/src/test/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxToggleNotificationRequestTest.java --- a/vm-jmx/client-core/src/test/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxToggleNotificationRequestTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/client-core/src/test/java/com/redhat/thermostat/vm/jmx/client/core/internal/JmxToggleNotificationRequestTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -76,7 +76,7 @@ host = mock(HostRef.class); vm = mock(VmRef.class); - when(vm.getAgent()).thenReturn(host); + when(vm.getHostRef()).thenReturn(host); agentDAO = mock(AgentInfoDAO.class); @@ -97,7 +97,7 @@ assertEquals(new InetSocketAddress(HOST, PORT), req.getTarget()); assertEquals(JmxCommand.RECEIVER, req.getReceiver()); - assertEquals(vm.getIdString(), req.getParameter(JmxCommand.VM_ID)); + assertEquals(String.valueOf(vm.getPid()), req.getParameter(JmxCommand.VM_PID)); assertEquals(JmxCommand.ENABLE_JMX_NOTIFICATIONS.name(), req.getParameter(JmxCommand.class.getName())); } @@ -112,7 +112,7 @@ assertEquals(new InetSocketAddress(HOST, PORT), req.getTarget()); assertEquals(JmxCommand.RECEIVER, req.getReceiver()); - assertEquals(vm.getIdString(), req.getParameter(JmxCommand.VM_ID)); + assertEquals(String.valueOf(vm.getPid()), req.getParameter(JmxCommand.VM_PID)); assertEquals(JmxCommand.DISABLE_JMX_NOTIFICATIONS.name(), req.getParameter(JmxCommand.class.getName())); } diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/common/src/main/java/com/redhat/thermostat/vm/jmx/common/JmxCommand.java --- a/vm-jmx/common/src/main/java/com/redhat/thermostat/vm/jmx/common/JmxCommand.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/common/src/main/java/com/redhat/thermostat/vm/jmx/common/JmxCommand.java Thu Jul 18 18:33:49 2013 -0400 @@ -42,6 +42,6 @@ DISABLE_JMX_NOTIFICATIONS, ; - public static final String VM_ID = "VM_ID"; + public static final String VM_PID = "VM_PID"; public static final String RECEIVER = "com.redhat.thermostat.vm.jmx.agent.internal.JmxRequestListener"; } diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/common/src/main/java/com/redhat/thermostat/vm/jmx/common/internal/JmxNotificationDAOImpl.java --- a/vm-jmx/common/src/main/java/com/redhat/thermostat/vm/jmx/common/internal/JmxNotificationDAOImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/common/src/main/java/com/redhat/thermostat/vm/jmx/common/internal/JmxNotificationDAOImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -81,12 +81,12 @@ static final String QUERY_LATEST_NOTIFICATION_STATUS = "QUERY " + NOTIFICATION_STATUS.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i SORT '" + + Key.VM_ID.getName() + "' = ?s SORT '" + Key.TIMESTAMP.getName() + "' DSC LIMIT 1"; static final String QUERY_NOTIFICATIONS = "QUERY " + NOTIFICATIONS.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i AND '" + + Key.VM_ID.getName() + "' = ?s AND '" + Key.TIMESTAMP.getName() + "' > ?l"; private Storage storage; @@ -112,8 +112,8 @@ Cursor cursor; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, statusFor.getAgent().getAgentId()); - stmt.setInt(1, statusFor.getId()); + stmt.setString(0, statusFor.getHostRef().getAgentId()); + stmt.setString(1, statusFor.getVmId()); cursor = stmt.executeQuery(); } catch (DescriptorParsingException e) { // should not happen, but if it *does* happen, at least log it @@ -147,8 +147,8 @@ Cursor cursor; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, notificationsFor.getAgent().getAgentId()); - stmt.setInt(1, notificationsFor.getId()); + stmt.setString(0, notificationsFor.getHostRef().getAgentId()); + stmt.setString(1, notificationsFor.getVmId()); stmt.setLong(2, timeStampSince); cursor = stmt.executeQuery(); } catch (DescriptorParsingException e) { diff -r 53d3cab77025 -r f7961cf1e440 vm-jmx/common/src/test/java/com/redhat/thermostat/vm/jmx/common/internal/JmxNotificationDAOImplTest.java --- a/vm-jmx/common/src/test/java/com/redhat/thermostat/vm/jmx/common/internal/JmxNotificationDAOImplTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-jmx/common/src/test/java/com/redhat/thermostat/vm/jmx/common/internal/JmxNotificationDAOImplTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -66,7 +66,7 @@ public class JmxNotificationDAOImplTest { private final String AGENT_ID = "an-agent's-id"; - private final int VM_ID = -1; + private final String VM_ID = "vmId"; private Storage storage; @@ -80,8 +80,8 @@ when(host.getAgentId()).thenReturn(AGENT_ID); vm = mock(VmRef.class); - when(vm.getAgent()).thenReturn(host); - when(vm.getId()).thenReturn(VM_ID); + when(vm.getHostRef()).thenReturn(host); + when(vm.getVmId()).thenReturn(VM_ID); storage = mock(Storage.class); @@ -90,9 +90,9 @@ @Test public void preparedQueryDescriptorsAreSane() { - String expectedQueryLatestNotificationStatus = "QUERY vm-jmx-notification-status WHERE 'agentId' = ?s AND 'vmId' = ?i SORT 'timeStamp' DSC LIMIT 1"; + String expectedQueryLatestNotificationStatus = "QUERY vm-jmx-notification-status WHERE 'agentId' = ?s AND 'vmId' = ?s SORT 'timeStamp' DSC LIMIT 1"; assertEquals(expectedQueryLatestNotificationStatus, JmxNotificationDAOImpl.QUERY_LATEST_NOTIFICATION_STATUS); - String expectedQueryNotifications = "QUERY vm-jmx-notification WHERE 'agentId' = ?s AND 'vmId' = ?i AND 'timeStamp' > ?l"; + String expectedQueryNotifications = "QUERY vm-jmx-notification WHERE 'agentId' = ?s AND 'vmId' = ?s AND 'timeStamp' > ?l"; assertEquals(expectedQueryNotifications, JmxNotificationDAOImpl.QUERY_NOTIFICATIONS); } @@ -129,7 +129,7 @@ verify(storage).prepareStatement(anyDescriptor(JmxNotificationStatus.class)); verify(stmt).setString(0, AGENT_ID); - verify(stmt).setInt(1, VM_ID); + verify(stmt).setString(1, VM_ID); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -176,7 +176,7 @@ verify(storage).prepareStatement(anyDescriptor(JmxNotification.class)); verify(stmt).setString(0, AGENT_ID); - verify(stmt).setInt(1, VM_ID); + verify(stmt).setString(1, VM_ID); verify(stmt).setLong(2, timeStamp); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/agent/src/main/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryBackend.java --- a/vm-memory/agent/src/main/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryBackend.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/agent/src/main/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryBackend.java Thu Jul 18 18:33:49 2013 -0400 @@ -59,8 +59,8 @@ } @Override - protected VmUpdateListener createVmListener(int pid) { - return new VmMemoryVmListener(vmMemoryStats, pid); + protected VmUpdateListener createVmListener(String vmId, int pid) { + return new VmMemoryVmListener(vmMemoryStats, vmId); } } diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/agent/src/main/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryVmListener.java --- a/vm-memory/agent/src/main/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryVmListener.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/agent/src/main/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryVmListener.java Thu Jul 18 18:33:49 2013 -0400 @@ -52,10 +52,10 @@ private static final Logger logger = LoggingUtils.getLogger(VmMemoryVmListener.class); - private final int vmId; + private final String vmId; private final VmMemoryStatDAO memDAO; - public VmMemoryVmListener(VmMemoryStatDAO vmMemoryStatDao, int vmId) { + public VmMemoryVmListener(VmMemoryStatDAO vmMemoryStatDao, String vmId) { memDAO = vmMemoryStatDao; this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/agent/src/test/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryVmListenerTest.java --- a/vm-memory/agent/src/test/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryVmListenerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/agent/src/test/java/com/redhat/thermostat/vm/memory/agent/internal/VmMemoryVmListenerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -85,7 +85,7 @@ public void setup() throws VmUpdateException { final int numGens = 2; vmMemoryStatDAO = mock(VmMemoryStatDAO.class); - vmListener = new VmMemoryVmListener(vmMemoryStatDAO, 0); + vmListener = new VmMemoryVmListener(vmMemoryStatDAO, "vmId"); extractor = mock(VmMemoryDataExtractor.class); mockTotalGenerations(numGens); diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/client-cli/src/test/java/com/redhat/thermostat/vm/memory/client/cli/internal/VmMemoryStatPrintDelegateTest.java --- a/vm-memory/client-cli/src/test/java/com/redhat/thermostat/vm/memory/client/cli/internal/VmMemoryStatPrintDelegateTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/client-cli/src/test/java/com/redhat/thermostat/vm/memory/client/cli/internal/VmMemoryStatPrintDelegateTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -76,9 +76,9 @@ } private void setupDAOs() { - int vmId = 234; + String vmId = "vmId"; HostRef host = new HostRef("123", "dummy"); - vm = new VmRef(host, 234, "dummy"); + vm = new VmRef(host, vmId, 234, "dummy"); VmMemoryStat.Space space1_1_1 = newSpace("space1", 123456, 12345, 1, 0); VmMemoryStat.Space space1_1_2 = newSpace("space2", 123456, 12345, 1, 0); diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/client-core/src/main/java/com/redhat/thermostat/vm/memory/client/core/internal/MemoryStatsController.java --- a/vm-memory/client-core/src/main/java/com/redhat/thermostat/vm/memory/client/core/internal/MemoryStatsController.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/client-core/src/main/java/com/redhat/thermostat/vm/memory/client/core/internal/MemoryStatsController.java Thu Jul 18 18:33:49 2013 -0400 @@ -201,8 +201,8 @@ if (response.getType() == ResponseType.ERROR) { view.displayWarning(translate.localize( LocaleResources.ERROR_PERFORMING_GC, - ref.getAgent().getAgentId(), - ref.getIdString())); + ref.getHostRef().getAgentId(), + ref.getVmId())); } } }; diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/client-core/src/test/java/com/redhat/thermostat/vm/memory/client/core/internal/MemoryStatsControllerTest.java --- a/vm-memory/client-core/src/test/java/com/redhat/thermostat/vm/memory/client/core/internal/MemoryStatsControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/client-core/src/test/java/com/redhat/thermostat/vm/memory/client/core/internal/MemoryStatsControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -151,7 +151,7 @@ } long timestamp = 1; - int vmID = 1; + String vmID = "vmId"; for (int i = 0; i < 5; i++) { VmMemoryStat vmMemory = new VmMemoryStat(timestamp++, vmID, generations); vmInfo.add(vmMemory); diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/common/src/main/java/com/redhat/thermostat/vm/memory/common/internal/VmMemoryStatDAOImpl.java --- a/vm-memory/common/src/main/java/com/redhat/thermostat/vm/memory/common/internal/VmMemoryStatDAOImpl.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/common/src/main/java/com/redhat/thermostat/vm/memory/common/internal/VmMemoryStatDAOImpl.java Thu Jul 18 18:33:49 2013 -0400 @@ -60,7 +60,7 @@ static final String QUERY_LATEST = "QUERY " + vmMemoryStatsCategory.getName() + " WHERE '" + Key.AGENT_ID.getName() + "' = ?s AND '" - + Key.VM_ID.getName() + "' = ?i SORT '" + + Key.VM_ID.getName() + "' = ?s SORT '" + Key.TIMESTAMP.getName() + "' DSC LIMIT 1"; private final Storage storage; @@ -79,8 +79,8 @@ Cursor cursor; try { stmt = storage.prepareStatement(desc); - stmt.setString(0, ref.getAgent().getAgentId()); - stmt.setInt(1, ref.getId()); + stmt.setString(0, ref.getHostRef().getAgentId()); + stmt.setString(1, ref.getVmId()); cursor = stmt.executeQuery(); } catch (DescriptorParsingException e) { // should not happen, but if it *does* happen, at least log it diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/common/src/main/java/com/redhat/thermostat/vm/memory/common/model/VmMemoryStat.java --- a/vm-memory/common/src/main/java/com/redhat/thermostat/vm/memory/common/model/VmMemoryStat.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/common/src/main/java/com/redhat/thermostat/vm/memory/common/model/VmMemoryStat.java Thu Jul 18 18:33:49 2013 -0400 @@ -177,13 +177,13 @@ private Generation[] generations; private long timestamp; - private int vmId; + private String vmId; public VmMemoryStat() { - this(-1, -1, null); + this(-1, null, null); } - public VmMemoryStat(long timestamp, int vmId, Generation[] generations) { + public VmMemoryStat(long timestamp, String vmId, Generation[] generations) { this.timestamp = timestamp; this.vmId = vmId; if (generations != null) { @@ -192,12 +192,12 @@ } @Persist - public int getVmId() { + public String getVmId() { return vmId; } @Persist - public void setVmId(int vmId) { + public void setVmId(String vmId) { this.vmId = vmId; } diff -r 53d3cab77025 -r f7961cf1e440 vm-memory/common/src/test/java/com/redhat/thermostat/vm/memory/common/internal/VmMemoryStatDAOTest.java --- a/vm-memory/common/src/test/java/com/redhat/thermostat/vm/memory/common/internal/VmMemoryStatDAOTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-memory/common/src/test/java/com/redhat/thermostat/vm/memory/common/internal/VmMemoryStatDAOTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -70,7 +70,7 @@ public class VmMemoryStatDAOTest { - private static final int VM_ID = 0xcafe; + private static final String VM_ID = "0xcafe"; private static final String AGENT_ID = "agent"; private Storage storage; @@ -86,8 +86,8 @@ when(hostRef.getAgentId()).thenReturn(AGENT_ID); vmRef = mock(VmRef.class); - when(vmRef.getAgent()).thenReturn(hostRef); - when(vmRef.getId()).thenReturn(VM_ID); + when(vmRef.getHostRef()).thenReturn(hostRef); + when(vmRef.getVmId()).thenReturn(VM_ID); storage = mock(Storage.class); stmt = (PreparedStatement) mock(PreparedStatement.class); @@ -114,7 +114,7 @@ @Test public void preparedQueryDescriptorsAreSane() { - String expectedQueryThreadCaps = "QUERY vm-memory-stats WHERE 'agentId' = ?s AND 'vmId' = ?i SORT 'timeStamp' DSC LIMIT 1"; + String expectedQueryThreadCaps = "QUERY vm-memory-stats WHERE 'agentId' = ?s AND 'vmId' = ?s SORT 'timeStamp' DSC LIMIT 1"; assertEquals(expectedQueryThreadCaps, VmMemoryStatDAOImpl.QUERY_LATEST); } @@ -137,8 +137,8 @@ impl.getLatestMemoryStat(vmRef); verify(storage).prepareStatement(anyDescriptor()); - verify(stmt).setString(0, vmRef.getAgent().getAgentId()); - verify(stmt).setInt(1, vmRef.getId()); + verify(stmt).setString(0, vmRef.getHostRef().getAgentId()); + verify(stmt).setString(1, vmRef.getVmId()); verify(stmt).executeQuery(); } @@ -148,8 +148,8 @@ impl.getLatestVmMemoryStats(vmRef, 123L); verify(storage).prepareStatement(anyDescriptor()); - verify(stmt).setString(0, vmRef.getAgent().getAgentId()); - verify(stmt).setInt(1, vmRef.getId()); + verify(stmt).setString(0, vmRef.getHostRef().getAgentId()); + verify(stmt).setString(1, vmRef.getVmId()); verify(stmt).setLong(2, 123L); verify(stmt).executeQuery(); verifyNoMoreInteractions(stmt); @@ -202,7 +202,7 @@ } gen.setSpaces(spaces.toArray(new Space[spaces.size()])); } - VmMemoryStat stat = new VmMemoryStat(1, 2, generations.toArray(new Generation[generations.size()])); + VmMemoryStat stat = new VmMemoryStat(1, "vmId", generations.toArray(new Generation[generations.size()])); Storage storage = mock(Storage.class); Add add = mock(Add.class); diff -r 53d3cab77025 -r f7961cf1e440 vm-overview/client-core/src/test/java/com/redhat/thermostat/vm/overview/client/core/internal/VmOverviewControllerTest.java --- a/vm-overview/client-core/src/test/java/com/redhat/thermostat/vm/overview/client/core/internal/VmOverviewControllerTest.java Thu Jul 18 16:31:24 2013 -0400 +++ b/vm-overview/client-core/src/test/java/com/redhat/thermostat/vm/overview/client/core/internal/VmOverviewControllerTest.java Thu Jul 18 18:33:49 2013 -0400 @@ -71,6 +71,7 @@ public class VmOverviewControllerTest { private static final Translate translator = LocaleResources.createLocalizer(); + private static final String VM_ID = "vmId"; private static final int VM_PID = 1337; private static final long START_TIME = 10000; private static final long STOP_TIME = 20000; @@ -128,7 +129,7 @@ } private VmInfo createVmInfo() { - VmInfo vmInfo = new VmInfo(VM_PID, START_TIME, STOP_TIME, JAVA_VERSION, + VmInfo vmInfo = new VmInfo(VM_ID, VM_PID, START_TIME, STOP_TIME, JAVA_VERSION, JAVA_HOME, MAIN_CLASS, COMMAND_LINE, VM_NAME, VM_INFO, VM_VERSION, VM_ARGS, PROPS, ENV, LIBS, UID, USERNAME); return vmInfo; @@ -157,7 +158,7 @@ @Test public void verifyViewIsUpdatedWithDataNoUid() { - VmInfo vmInfo = new VmInfo(VM_PID, START_TIME, STOP_TIME, JAVA_VERSION, + VmInfo vmInfo = new VmInfo(VM_ID, VM_PID, START_TIME, STOP_TIME, JAVA_VERSION, JAVA_HOME, MAIN_CLASS, COMMAND_LINE, VM_NAME, VM_INFO, VM_VERSION, VM_ARGS, PROPS, ENV, LIBS, -1, null); createController(vmInfo); @@ -183,7 +184,7 @@ @Test public void verifyViewIsUpdatedWithDataNoUsername() { - VmInfo vmInfo = new VmInfo(VM_PID, START_TIME, STOP_TIME, JAVA_VERSION, + VmInfo vmInfo = new VmInfo(VM_ID, VM_PID, START_TIME, STOP_TIME, JAVA_VERSION, JAVA_HOME, MAIN_CLASS, COMMAND_LINE, VM_NAME, VM_INFO, VM_VERSION, VM_ARGS, PROPS, ENV, LIBS, UID, null); createController(vmInfo);