# HG changeset patch # User Omair Majid # Date 1357833822 18000 # Node ID d8f4cdaed16e55eec6612573dcee9d85002779b2 # Parent 4de458324cf930ddfca3672d9f06e4ba1a38ef10 Disable 'Heap Dump' button for dead vms There's no point showing a Heap Dump button for dead VMs. It can't do anything and is misleading. Reviewed-by: vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-December/004396.html PR1229 diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/HeapDumperService.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/HeapDumperService.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/HeapDumperService.java Thu Jan 10 11:03:42 2013 -0500 @@ -41,6 +41,7 @@ import com.redhat.thermostat.client.core.controllers.InformationServiceController; import com.redhat.thermostat.client.core.NameMatchingRefFilter; import com.redhat.thermostat.common.ApplicationService; +import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.common.dao.VmRef; import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.vm.heap.analysis.client.core.internal.HeapDumpController; @@ -51,12 +52,14 @@ private static final int ORDER = ORDER_MEMORY_GROUP + 60; private ApplicationService appService; + private VmInfoDAO vmInfoDao; private VmMemoryStatDAO vmMemoryStatDao; private HeapDAO heapDao; private Filter filter = new NameMatchingRefFilter<>(); - public HeapDumperService(ApplicationService appService, VmMemoryStatDAO vmMemoryStatDao, HeapDAO heapDao) { + public HeapDumperService(ApplicationService appService, VmInfoDAO vmInfoDao, VmMemoryStatDAO vmMemoryStatDao, HeapDAO heapDao) { + this.vmInfoDao = vmInfoDao; this.vmMemoryStatDao = vmMemoryStatDao; this.heapDao = heapDao; this.appService = appService; @@ -69,7 +72,8 @@ HeapHistogramViewProvider histogramViewProvider = OSGIUtils.getInstance().getService(HeapHistogramViewProvider.class); ObjectDetailsViewProvider objectDetailsViewProvider = OSGIUtils.getInstance().getService(ObjectDetailsViewProvider.class); ObjectRootsViewProvider objectRootsViewProvider = OSGIUtils.getInstance().getService(ObjectRootsViewProvider.class); - return new HeapDumpController(vmMemoryStatDao, heapDao, ref, appService, viewProvider, detailsViewProvider, histogramViewProvider, objectDetailsViewProvider, objectRootsViewProvider); + return new HeapDumpController(vmMemoryStatDao, vmInfoDao, heapDao, ref, appService, + viewProvider, detailsViewProvider, histogramViewProvider, objectDetailsViewProvider, objectRootsViewProvider); } @Override diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/HeapView.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/HeapView.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/HeapView.java Thu Jan 10 11:03:42 2013 -0500 @@ -50,9 +50,14 @@ public enum HeapDumperAction { DUMP_REQUESTED, ANALYSE, - REQUEST_ABORTED + REQUEST_ABORTED, } - + + public enum DumpDisabledReason { + DUMP_IN_PROGRESS, + PROCESS_DEAD, + } + protected final ActionNotifier heapDumperNotifier; protected HeapView() { heapDumperNotifier = new ActionNotifier(this); @@ -66,18 +71,21 @@ heapDumperNotifier.removeActionListener(listener); } - abstract public void updateUsedAndCapacity(String used, String capacity); + public abstract void updateUsedAndCapacity(String used, String capacity); /** View updates automatically based on the model */ - abstract public void setModel(OverviewChart model); - abstract public void addHeapDump(HeapDump dump); - abstract public void clearHeapDumpList(); - - abstract public void openDumpView(); - abstract public void setChildView(BasicView childView); + public abstract void setModel(OverviewChart model); + + public abstract void enableHeapDumping(); + public abstract void disableHeapDumping(DumpDisabledReason reason); + + public abstract void addHeapDump(HeapDump dump); + public abstract void updateHeapDumpList(List heapDumps); + public abstract void clearHeapDumpList(); + + public abstract void openDumpView(); + public abstract void setChildView(BasicView childView); public abstract void notifyHeapDumpComplete(); - public abstract void updateHeapDumpList(List heapDumps); - public abstract void displayWarning(String string); } diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/Activator.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/Activator.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/Activator.java Thu Jan 10 11:03:42 2013 -0500 @@ -50,6 +50,7 @@ import com.redhat.thermostat.common.Constants; import com.redhat.thermostat.common.MultipleServiceTracker; import com.redhat.thermostat.common.MultipleServiceTracker.Action; +import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.common.dao.VmRef; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapDumperService; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; @@ -64,6 +65,7 @@ public void start(final BundleContext context) throws Exception { Class[] deps = new Class[] { ApplicationService.class, + VmInfoDAO.class, VmMemoryStatDAO.class, HeapDAO.class, }; @@ -74,12 +76,13 @@ public void dependenciesAvailable(Map services) { ApplicationService appSvc = (ApplicationService) services.get(ApplicationService.class.getName()); Objects.requireNonNull(appSvc); + VmInfoDAO vmInfoDao = Objects.requireNonNull((VmInfoDAO) services.get(VmInfoDAO.class.getName())); VmMemoryStatDAO vmMemoryStatDao = (VmMemoryStatDAO) services.get(VmMemoryStatDAO.class.getName()); Objects.requireNonNull(vmMemoryStatDao); HeapDAO heapDao = (HeapDAO) services.get(HeapDAO.class.getName()); Objects.requireNonNull(heapDao); - HeapDumperService service = new HeapDumperService(appSvc, vmMemoryStatDao, heapDao); + HeapDumperService service = new HeapDumperService(appSvc, vmInfoDao, vmMemoryStatDao, heapDao); Dictionary properties = new Hashtable<>(); properties.put(Constants.GENERIC_SERVICE_CLASSNAME, VmRef.class.getName()); reg = context.registerService(InformationService.class.getName(), service , properties); diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpController.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpController.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpController.java Thu Jan 10 11:03:42 2013 -0500 @@ -54,6 +54,7 @@ import com.redhat.thermostat.common.Timer; import com.redhat.thermostat.common.Timer.SchedulingType; import com.redhat.thermostat.common.cli.CommandException; +import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.common.dao.VmRef; import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.storage.model.HeapInfo; @@ -63,6 +64,7 @@ import com.redhat.thermostat.vm.heap.analysis.client.core.HeapDumpDetailsViewProvider; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapHistogramViewProvider; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView; +import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView.DumpDisabledReason; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView.HeapDumperAction; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapViewProvider; import com.redhat.thermostat.vm.heap.analysis.client.core.ObjectDetailsViewProvider; @@ -77,6 +79,7 @@ private static final Translate translator = LocaleResources.createLocalizer(); + private final VmInfoDAO vmInfoDao; private final VmMemoryStatDAO vmDao; private final VmRef ref; @@ -92,19 +95,22 @@ private ObjectDetailsViewProvider objectDetailsViewProvider; private ObjectRootsViewProvider objectRootsViewProvider; + public HeapDumpController(final VmMemoryStatDAO vmMemoryStatDao, + final VmInfoDAO vmInfoDao, final HeapDAO heapDao, final VmRef ref, final ApplicationService appService, HeapViewProvider viewProvider, HeapDumpDetailsViewProvider detailsViewProvider, HeapHistogramViewProvider histogramProvider, ObjectDetailsViewProvider objectDetailsProvider, ObjectRootsViewProvider objectRootsProvider) { - this(vmMemoryStatDao, heapDao, ref, appService, viewProvider, + this(vmMemoryStatDao, vmInfoDao, heapDao, ref, appService, viewProvider, detailsViewProvider, histogramProvider, objectDetailsProvider, objectRootsProvider, new HeapDumper(ref)); } HeapDumpController(final VmMemoryStatDAO vmMemoryStatDao, + final VmInfoDAO vmInfoDao, final HeapDAO heapDao, final VmRef ref, final ApplicationService appService, HeapViewProvider viewProvider, HeapDumpDetailsViewProvider detailsViewProvider, @@ -118,6 +124,7 @@ this.detailsViewProvider = detailsViewProvider; this.appService = appService; this.ref = ref; + this.vmInfoDao = vmInfoDao; this.vmDao = vmMemoryStatDao; this.heapDAO = heapDao; @@ -178,6 +185,7 @@ HeapDump dump = null; switch (actionEvent.getActionId()) { case DUMP_REQUESTED: + view.disableHeapDumping(DumpDisabledReason.DUMP_IN_PROGRESS); requestDump(heapDumper); break; case ANALYSE: @@ -189,6 +197,11 @@ } } }); + + boolean vmIsAlive = vmInfoDao.getVmInfo(ref).isAlive(); + if (!vmIsAlive) { + view.disableHeapDumping(DumpDisabledReason.PROCESS_DEAD); + } } private void requestDump(final HeapDumper heapDumper) { @@ -198,6 +211,7 @@ public void run() { try { heapDumper.dump(); + view.enableHeapDumping(); view.notifyHeapDumpComplete(); } catch (CommandException e) { view.displayWarning(e.getMessage()); diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/locale/LocaleResources.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/locale/LocaleResources.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/locale/LocaleResources.java Thu Jan 10 11:03:42 2013 -0500 @@ -68,6 +68,10 @@ HEAP_DUMP_OBJECT_FIND_ROOT, OBJECT_ROOTS_VIEW_TITLE, + + TRIGGER_HEAP_DUMP, + HEAP_DUMP_IN_PROGRESS, + PROCESS_EXITED, ; static final String RESOURCE_BUNDLE = "com.redhat.thermostat.vm.heap.analysis.client.locale.strings"; diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/main/resources/com/redhat/thermostat/vm/heap/analysis/client/locale/strings.properties --- a/vm-heap-analysis/client-core/src/main/resources/com/redhat/thermostat/vm/heap/analysis/client/locale/strings.properties Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/main/resources/com/redhat/thermostat/vm/heap/analysis/client/locale/strings.properties Thu Jan 10 11:03:42 2013 -0500 @@ -26,4 +26,8 @@ HEAP_DUMP_OBJECT_BROWSE_REFERENCES = References HEAP_DUMP_OBJECT_FIND_ROOT = Find Root -OBJECT_ROOTS_VIEW_TITLE = Object Roots \ No newline at end of file +OBJECT_ROOTS_VIEW_TITLE = Object Roots + +TRIGGER_HEAP_DUMP = Heap Dump +HEAP_DUMP_IN_PROGRESS = dumping... +PROCESS_EXITED = Process exited. diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/ActivatorTest.java --- a/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/ActivatorTest.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/ActivatorTest.java Thu Jan 10 11:03:42 2013 -0500 @@ -45,6 +45,7 @@ import com.redhat.thermostat.client.core.InformationService; import com.redhat.thermostat.common.ApplicationService; +import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.test.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapDumperService; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; @@ -71,10 +72,12 @@ @Test public void verifyActivatorRegistersServices() throws Exception { StubBundleContext context = new StubBundleContext(); + VmInfoDAO vmInfoDao = mock(VmInfoDAO.class); VmMemoryStatDAO vmMemoryStatDAO = mock(VmMemoryStatDAO.class); HeapDAO heapDAO = mock(HeapDAO.class); ApplicationService appSvc = mock(ApplicationService.class); + context.registerService(VmInfoDAO.class, vmInfoDao, null); context.registerService(VmMemoryStatDAO.class, vmMemoryStatDAO, null); context.registerService(HeapDAO.class, heapDAO, null); context.registerService(ApplicationService.class, appSvc, null); @@ -88,7 +91,7 @@ activator.stop(context); assertEquals(0, context.getServiceListeners().size()); - assertEquals(3, context.getAllServices().size()); + assertEquals(4, context.getAllServices().size()); } } diff -r 4de458324cf9 -r d8f4cdaed16e 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 Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpControllerTest.java Thu Jan 10 11:03:42 2013 -0500 @@ -71,8 +71,10 @@ import com.redhat.thermostat.common.TimerFactory; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.dao.HostRef; +import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.common.dao.VmRef; import com.redhat.thermostat.storage.model.HeapInfo; +import com.redhat.thermostat.storage.model.VmInfo; import com.redhat.thermostat.storage.model.VmMemoryStat; import com.redhat.thermostat.storage.model.VmMemoryStat.Generation; import com.redhat.thermostat.storage.model.VmMemoryStat.Space; @@ -81,6 +83,7 @@ import com.redhat.thermostat.vm.heap.analysis.client.core.HeapHistogramView; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapHistogramViewProvider; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView; +import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView.DumpDisabledReason; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView.HeapDumperAction; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapViewProvider; import com.redhat.thermostat.vm.heap.analysis.client.core.ObjectDetailsView; @@ -100,6 +103,7 @@ private Timer timer; private HeapDAO heapDao; + private VmInfoDAO vmInfoDao; private VmMemoryStatDAO vmDao; private HeapView view; private HeapDumpDetailsView detailsView; @@ -120,6 +124,10 @@ public void setUp() { heapDao = mock(HeapDAO.class); vmDao = mock(VmMemoryStatDAO.class); + vmInfoDao = mock(VmInfoDAO.class); + VmInfo vmInfo = mock(VmInfo.class); + when(vmInfo.isAlive()).thenReturn(true); + when(vmInfoDao.getVmInfo(isA(VmRef.class))).thenReturn(vmInfo); appService = mock(ApplicationService.class); heapDumper = mock(HeapDumper.class); @@ -184,7 +192,7 @@ when(ref.getIdString()).thenReturn("vm-id"); when(ref.getAgent()).thenReturn(hostRef); - controller = new HeapDumpController(vmDao, heapDao, ref, appService, + controller = new HeapDumpController(vmDao, vmInfoDao, heapDao, ref, appService, viewProvider, detailsViewProvider, histogramProvider, objectDetailsProvider, objectRootsProvider, heapDumper); } @@ -268,7 +276,7 @@ when(cache.getAttribute(any(VmRef.class))).thenReturn(dump); when(appService.getApplicationCache()).thenReturn(cache); VmRef ref = mock(VmRef.class); - controller = new HeapDumpController(vmDao, heapDao, ref, appService, + controller = new HeapDumpController(vmDao, vmInfoDao, heapDao, ref, appService, viewProvider, detailsViewProvider, histogramProvider, objectDetailsProvider, objectRootsProvider, heapDumper); @@ -293,7 +301,7 @@ when(appService.getApplicationCache()).thenReturn(cache); VmRef ref = mock(VmRef.class); - controller = new HeapDumpController(vmDao, heapDao, ref, appService, + controller = new HeapDumpController(vmDao, vmInfoDao, heapDao, ref, appService, viewProvider, detailsViewProvider, histogramProvider, objectDetailsProvider, objectRootsProvider, heapDumper); @@ -301,6 +309,26 @@ } @Test + public void testHeapDumpingDisabledForDeadVms() { + ApplicationCache cache = mock(ApplicationCache.class); + when(appService.getApplicationCache()).thenReturn(cache); + setUpTimers(); + + VmRef ref = mock(VmRef.class); + + VmInfo vmInfo = mock(VmInfo.class); + when(vmInfo.isAlive()).thenReturn(false); + + when(vmInfoDao.getVmInfo(ref)).thenReturn(vmInfo); + + controller = new HeapDumpController(vmDao, vmInfoDao, heapDao, ref, appService, + viewProvider, detailsViewProvider, histogramProvider, + objectDetailsProvider, objectRootsProvider, heapDumper); + + verify(view).disableHeapDumping(DumpDisabledReason.PROCESS_DEAD); + } + + @Test public void testRequestHeapDump() throws CommandException, InterruptedException { final CountDownLatch latch = new CountDownLatch(1); mockExecutorService(latch); diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-swing/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/swing/internal/HeapSwingView.java --- a/vm-heap-analysis/client-swing/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/swing/internal/HeapSwingView.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-swing/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/swing/internal/HeapSwingView.java Thu Jan 10 11:03:42 2013 -0500 @@ -77,9 +77,6 @@ stats.addHeapDumperListener(new ActionListener() { @Override public void actionPerformed(ActionEvent e) { - - stats.disableHeapDumperControl(); - heapDumperNotifier.fireAction(HeapDumperAction.DUMP_REQUESTED); } }); @@ -156,6 +153,26 @@ } @Override + public void enableHeapDumping() { + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + stats.enableHeapDumperControl(); + } + }); + } + + @Override + public void disableHeapDumping(final DumpDisabledReason reason) { + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + stats.disableHeapDumperControl(reason); + } + }); + } + + @Override public void addHeapDump(final HeapDump dump) { SwingUtilities.invokeLater(new Runnable() { @Override @@ -211,12 +228,7 @@ @Override public void notifyHeapDumpComplete() { - EventQueue.invokeLater(new Runnable() { - @Override - public void run() { - stats.enableHeapDumperControl(); - } - }); + enableHeapDumping(); } @Override diff -r 4de458324cf9 -r d8f4cdaed16e vm-heap-analysis/client-swing/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/swing/internal/StatsPanel.java --- a/vm-heap-analysis/client-swing/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/swing/internal/StatsPanel.java Thu Jan 10 15:09:23 2013 +0100 +++ b/vm-heap-analysis/client-swing/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/swing/internal/StatsPanel.java Thu Jan 10 11:03:42 2013 -0500 @@ -53,11 +53,16 @@ import javax.swing.SwingConstants; import javax.swing.event.ListSelectionListener; +import com.redhat.thermostat.common.locale.Translate; +import com.redhat.thermostat.vm.heap.analysis.client.core.HeapView.DumpDisabledReason; +import com.redhat.thermostat.vm.heap.analysis.client.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; @SuppressWarnings("serial") public class StatsPanel extends JPanel { + private static final Translate translator = LocaleResources.createLocalizer(); + private JPanel leftPanel; private JButton heapDumpButton; @@ -99,7 +104,7 @@ max = new JLabel("-"); max.setHorizontalAlignment(SwingConstants.RIGHT); - heapDumpButton = new JButton("Heap Dump"); + heapDumpButton = new JButton(translator.localize(LocaleResources.TRIGGER_HEAP_DUMP)); heapDumpButton.setName("heapDumpButton"); JScrollPane dumpListScrollPane = new JScrollPane(); @@ -173,13 +178,22 @@ dumpList.addListSelectionListener(listener); } - public void disableHeapDumperControl() { - heapDumpButton.setText("dumping..."); + public void disableHeapDumperControl(DumpDisabledReason reason) { + String text = null; + switch (reason) { + case DUMP_IN_PROGRESS: + text = translator.localize(LocaleResources.HEAP_DUMP_IN_PROGRESS); + break; + case PROCESS_DEAD: + text = translator.localize(LocaleResources.PROCESS_EXITED); + break; + } + heapDumpButton.setText(text); heapDumpButton.setEnabled(false); } public void enableHeapDumperControl() { - heapDumpButton.setText("Heap Dump"); + heapDumpButton.setText(translator.localize(LocaleResources.TRIGGER_HEAP_DUMP)); heapDumpButton.setEnabled(true); }