Mercurial > hg > release > thermostat-1.6
changeset 2035:fa302aa73bae
Improve gc-command wait-for-completion logic
Reusing a listener instance leads to reuse of the same CountDownLatch, which
does not work as expected in a Thermostat shell use case. Additionally, a 1
second latch timeout leads to very frequent timeouts before the request
response message is received, defeating the purpose of the latch.
PR3241
Reviewed-by: jerboaa, omajid
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-November/021731.html
| author | Andrew Azores <aazores@redhat.com> |
|---|---|
| date | Tue, 22 Nov 2016 12:48:19 -0500 |
| parents | 92de1fab95ec |
| children | b188a7f445f2 |
| files | vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/Activator.java vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommand.java vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListener.java vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListenerFactory.java vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListenerFactoryImpl.java vm-gc/command/src/test/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandTest.java |
| diffstat | 6 files changed, 109 insertions(+), 13 deletions(-) [+] |
line wrap: on
line diff
--- a/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/Activator.java Mon Nov 14 18:07:21 2016 +0100 +++ b/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/Activator.java Tue Nov 22 12:48:19 2016 -0500 @@ -53,7 +53,7 @@ public class Activator implements BundleActivator { - private final GCCommand gcCommand = new GCCommand(new GCCommandListener()); + private final GCCommand gcCommand = new GCCommand(new GCCommandListenerFactoryImpl()); private final ShowGcNameCommand showGcNameCmd = new ShowGcNameCommand(); private MultipleServiceTracker showGcNameCmdTracker;
--- a/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommand.java Mon Nov 14 18:07:21 2016 +0100 +++ b/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommand.java Tue Nov 22 12:48:19 2016 -0500 @@ -64,11 +64,13 @@ private GCRequest request; private AgentInfoDAO agentInfoDAO; private VmInfoDAO vmInfoDAO; - private final GCCommandListener listener; + + private final GCCommandListenerFactory listenerFactory; + private GCCommandListener listener; private Semaphore servicesAvailable = new Semaphore(0); - public GCCommand(GCCommandListener listener) { - this.listener = listener; + GCCommand(GCCommandListenerFactory listenerFactory) { + this.listenerFactory = listenerFactory; } @Override @@ -79,8 +81,7 @@ requireNonNull(agentInfoDAO, translator.localize(LocaleResources.AGENT_SERVICE_UNAVAILABLE)); requireNonNull(request, translator.localize(LocaleResources.GCREQUEST_SERVICE_UNAVAILABLE)); - listener.setOut(ctx.getConsole().getOutput()); - listener.setErr(ctx.getConsole().getError()); + listener = listenerFactory.createListener(ctx.getConsole().getOutput(), ctx.getConsole().getError()); HostVMArguments args = new HostVMArguments(ctx.getArguments(), false, true); @@ -120,7 +121,7 @@ private void waitForListenerResponse() { try { - listener.await(1000l); + listener.await(30_000L); } catch (InterruptedException e) { e.printStackTrace(); }
--- a/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListener.java Mon Nov 14 18:07:21 2016 +0100 +++ b/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListener.java Tue Nov 22 12:48:19 2016 -0500 @@ -92,8 +92,8 @@ latch.countDown(); } - public synchronized void await(long timeout) throws InterruptedException { - this.latch.await(timeout, TimeUnit.MILLISECONDS); + public synchronized void await(long milliseconds) throws InterruptedException { + this.latch.await(milliseconds, TimeUnit.MILLISECONDS); } public void setOut(PrintStream out) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListenerFactory.java Tue Nov 22 12:48:19 2016 -0500 @@ -0,0 +1,43 @@ +/* + * Copyright 2012-2016 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.vm.gc.command.internal; + +import java.io.PrintStream; + +interface GCCommandListenerFactory { + GCCommandListener createListener(PrintStream out, PrintStream err); +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/vm-gc/command/src/main/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandListenerFactoryImpl.java Tue Nov 22 12:48:19 2016 -0500 @@ -0,0 +1,49 @@ +/* + * Copyright 2012-2016 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.vm.gc.command.internal; + +import java.io.PrintStream; + +class GCCommandListenerFactoryImpl implements GCCommandListenerFactory { + @Override + public GCCommandListener createListener(PrintStream out, PrintStream err) { + GCCommandListener gcCommandListener = new GCCommandListener(); + gcCommandListener.setOut(out); + gcCommandListener.setErr(err); + return gcCommandListener; + } +}
--- a/vm-gc/command/src/test/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandTest.java Mon Nov 14 18:07:21 2016 +0100 +++ b/vm-gc/command/src/test/java/com/redhat/thermostat/vm/gc/command/internal/GCCommandTest.java Tue Nov 22 12:48:19 2016 -0500 @@ -61,6 +61,8 @@ import com.redhat.thermostat.storage.model.VmInfo; import com.redhat.thermostat.test.TestCommandContextFactory; +import java.io.PrintStream; + public class GCCommandTest { private GCCommand command; @@ -69,7 +71,6 @@ private VmInfoDAO vmInfoDAO; private AgentInfoDAO agentInfoDAO; private GCRequest gcRequest; - private GCCommandListener listener; @Before public void setup() { @@ -79,9 +80,11 @@ agentInfoDAO = mock(AgentInfoDAO.class); gcRequest = mock(GCRequest.class); - listener = mock(GCCommandListener.class); + GCCommandListenerFactory listenerFactory = mock(GCCommandListenerFactory.class); + GCCommandListener listener = mock(GCCommandListener.class); + when(listenerFactory.createListener(any(PrintStream.class), any(PrintStream.class))).thenReturn(listener); - command = new GCCommand(listener); + command = new GCCommand(listenerFactory); } @Test @@ -108,7 +111,7 @@ complete[0] = true; return null; } - }).when(gcRequest).sendGCRequestToAgent(vmRef, agentInfoDAO, listener); + }).when(gcRequest).sendGCRequestToAgent(eq(vmRef), eq(agentInfoDAO), any(GCCommandListener.class)); CommandContext context = createVmIdArgs(vmId);