Return-Path: X-Original-To: apmail-accumulo-commits-archive@www.apache.org Delivered-To: apmail-accumulo-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 8597D11BCF for ; Fri, 11 Jul 2014 17:31:05 +0000 (UTC) Received: (qmail 3485 invoked by uid 500); 11 Jul 2014 17:31:05 -0000 Delivered-To: apmail-accumulo-commits-archive@accumulo.apache.org Received: (qmail 3386 invoked by uid 500); 11 Jul 2014 17:31:05 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 3213 invoked by uid 99); 11 Jul 2014 17:31:05 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 11 Jul 2014 17:31:05 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 17D2883C772; Fri, 11 Jul 2014 17:31:05 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: elserj@apache.org To: commits@accumulo.apache.org Date: Fri, 11 Jul 2014 17:31:07 -0000 Message-Id: <1afca67ff3704be081bed92bc57f6c65@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [3/6] git commit: ACCUMULO-2985 Make sure the ES used to stop processes with a timeout is stopped itself. ACCUMULO-2985 Make sure the ES used to stop processes with a timeout is stopped itself. To use a Callable to apply a timeout when trying to stop the accumulo processes in MiniAccumuloCluster, an ExecutorService is required. A single-thread ES was added as a part of ACCUMULO-2764, however, it was neglected to be stopped itself which added a non-daemon'ized thread to the JVM. This non-daemonized thread prevents the JVM from exiting cleanly. After using the ES to stop the Accumulo processes with a timeout, we need to be sure to stop the ES as well. Test with a Mocked ES ensures that the ES is stopped before MAC.stop() returns. Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/609c7267 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/609c7267 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/609c7267 Branch: refs/heads/master Commit: 609c7267c33368ff9cfbb459153546d22bc5d2ce Parents: dec0384 Author: Josh Elser Authored: Fri Jul 11 12:04:09 2014 -0400 Committer: Josh Elser Committed: Fri Jul 11 12:15:18 2014 -0400 ---------------------------------------------------------------------- minicluster/pom.xml | 5 ++ .../minicluster/MiniAccumuloCluster.java | 26 ++++++++- .../minicluster/CleanShutdownMacTest.java | 61 ++++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/609c7267/minicluster/pom.xml ---------------------------------------------------------------------- diff --git a/minicluster/pom.xml b/minicluster/pom.xml index 9d2b7cb..7284ca4 100644 --- a/minicluster/pom.xml +++ b/minicluster/pom.xml @@ -77,6 +77,11 @@ test + org.easymock + easymock + test + + org.slf4j slf4j-api test http://git-wip-us.apache.org/repos/asf/accumulo/blob/609c7267/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java ---------------------------------------------------------------------- diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java b/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java index 9b0e196..10fed73 100644 --- a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java +++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java @@ -130,6 +130,9 @@ public class MiniAccumuloCluster { private MiniAccumuloConfig config; private Process[] tabletServerProcesses; + + // Non-final for testing purposes + private ExecutorService executor; private Process exec(Class clazz, String... args) throws IOException { String javaHome = System.getProperty("java.home"); @@ -295,7 +298,8 @@ public class MiniAccumuloCluster { zooCfg.store(fileWriter, null); fileWriter.close(); - + + this.executor = Executors.newSingleThreadExecutor(); } /** @@ -407,9 +411,27 @@ public class MiniAccumuloCluster { log.warn("GarbageCollector did not fully stop after 30 seconds", e); } } + + // ACCUMULO-2985 stop the ExecutorService after we finished using it to stop accumulo procs + if (null != executor) { + List tasksRemaining = executor.shutdownNow(); + + // the single thread executor shouldn't have any pending tasks, but check anyways + if (!tasksRemaining.isEmpty()) { + log.warn("Unexpectedly had " + tasksRemaining.size() + " task(s) remaining in threadpool for execution when being stopped"); + } + } } - private final ExecutorService executor = Executors.newSingleThreadExecutor(); + // Visible for testing + protected void setShutdownExecutor(ExecutorService svc) { + this.executor = svc; + } + + // Visible for testing + protected ExecutorService getShutdownExecutor() { + return executor; + } private int stopProcessWithTimeout(final Process proc, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { FutureTask future = new FutureTask(new Callable() { http://git-wip-us.apache.org/repos/asf/accumulo/blob/609c7267/minicluster/src/test/java/org/apache/accumulo/minicluster/CleanShutdownMacTest.java ---------------------------------------------------------------------- diff --git a/minicluster/src/test/java/org/apache/accumulo/minicluster/CleanShutdownMacTest.java b/minicluster/src/test/java/org/apache/accumulo/minicluster/CleanShutdownMacTest.java new file mode 100644 index 0000000..b22a45e --- /dev/null +++ b/minicluster/src/test/java/org/apache/accumulo/minicluster/CleanShutdownMacTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.minicluster; + +import java.io.File; +import java.util.Collections; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; + +import org.easymock.EasyMock; +import org.junit.Test; + +import com.google.common.io.Files; + +/** + * + */ +public class CleanShutdownMacTest { + + @SuppressWarnings("unchecked") + @Test + public void testExecutorServiceShutdown() throws Exception { + File tmp = Files.createTempDir(); + MiniAccumuloCluster cluster = new MiniAccumuloCluster(tmp, "foo"); + ExecutorService service = cluster.getShutdownExecutor(); + + // Don't leak the one that gets created + service.shutdownNow(); + + ExecutorService mockService = EasyMock.createMock(ExecutorService.class); + Future future = EasyMock.createMock(Future.class); + + cluster.setShutdownExecutor(mockService); + + EasyMock.expect(future.get()).andReturn(0).anyTimes(); + EasyMock.expect(mockService.submit(EasyMock.anyObject(Callable.class))).andReturn(future).anyTimes(); + EasyMock.expect(mockService.shutdownNow()).andReturn(Collections. emptyList()).once(); + + EasyMock.replay(mockService); + + cluster.stop(); + + EasyMock.verify(mockService); + } + +}