You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2016/01/21 02:23:29 UTC
[2/6] incubator-brooklyn git commit: better shutdown
better shutdown
call Main.terminate() in the main thread, rather than relying on shutdown hooks
fixes rest-initiated shutdown when using BrooklynJavascriptGuiLauncher
(looks like that has been broken since #771)
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/2a432cf3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/2a432cf3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/2a432cf3
Branch: refs/heads/master
Commit: 2a432cf38a2fb0571e4f76604c3a532b1f89c2c6
Parents: 8303fea
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Jan 20 15:56:07 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Jan 20 21:07:12 2016 +0000
----------------------------------------------------------------------
.../mgmt/internal/LocalManagementContext.java | 29 ++++++---
.../brooklyn/rest/resources/ServerResource.java | 1 +
.../brooklyn/rest/BrooklynRestApiLauncher.java | 32 ++++++++--
.../rest/testing/BrooklynRestApiTest.java | 8 +--
.../rest/util/NoOpRecordingShutdownHandler.java | 39 ++++++++++++
.../util/ServerStoppingShutdownHandler.java | 67 ++++++++++++++++++++
.../brooklyn/rest/util/TestShutdownHandler.java | 39 ------------
.../main/java/org/apache/brooklyn/cli/Main.java | 45 +++++++------
8 files changed, 181 insertions(+), 79 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
index d88a500..76500bc 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalManagementContext.java
@@ -30,6 +30,7 @@ import java.util.Set;
import java.util.WeakHashMap;
import java.util.concurrent.Callable;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.brooklyn.api.effector.Effector;
import org.apache.brooklyn.api.entity.Application;
@@ -116,6 +117,7 @@ public class LocalManagementContext extends AbstractManagementContext {
return closed;
}
+ private AtomicBoolean terminated = new AtomicBoolean(false);
private String managementPlaneId;
private String managementNodeId;
private BasicExecutionManager execution;
@@ -316,15 +318,26 @@ public class LocalManagementContext extends AbstractManagementContext {
@Override
public void terminate() {
- INSTANCES.remove(this);
- super.terminate();
- if (osgiManager!=null) {
- osgiManager.stop();
- osgiManager = null;
+ synchronized (terminated) {
+ if (terminated.getAndSet(true)) {
+ log.trace("Already terminated management context "+this);
+ // no harm in doing it twice, but it makes logs ugly!
+ return;
+ }
+ log.debug("Terminating management context "+this);
+
+ INSTANCES.remove(this);
+ super.terminate();
+ if (osgiManager!=null) {
+ osgiManager.stop();
+ osgiManager = null;
+ }
+ if (usageManager != null) usageManager.terminate();
+ if (execution != null) execution.shutdownNow();
+ if (gc != null) gc.shutdownNow();
+
+ log.debug("Terminated management context "+this);
}
- if (usageManager != null) usageManager.terminate();
- if (execution != null) execution.shutdownNow();
- if (gc != null) gc.shutdownNow();
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
index a9fa4dc..e624d89 100644
--- a/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
+++ b/brooklyn-server/rest/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ServerResource.java
@@ -218,6 +218,7 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
if (shutdownHandler != null) {
shutdownHandler.onShutdownRequest();
} else {
+ // should always be set as it is required by jersey injection?
log.warn("ShutdownHandler not set, exiting process");
System.exit(0);
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
index f641267..92ac8f7 100644
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -48,8 +48,8 @@ import org.apache.brooklyn.rest.security.provider.AnyoneSecurityProvider;
import org.apache.brooklyn.rest.security.provider.SecurityProvider;
import org.apache.brooklyn.rest.util.ManagementContextProvider;
import org.apache.brooklyn.rest.util.OsgiCompat;
+import org.apache.brooklyn.rest.util.ServerStoppingShutdownHandler;
import org.apache.brooklyn.rest.util.ShutdownHandlerProvider;
-import org.apache.brooklyn.rest.util.TestShutdownHandler;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.net.Networking;
@@ -94,7 +94,7 @@ public class BrooklynRestApiLauncher {
public static final String SCANNING_CATALOG_BOM_URL = "classpath://brooklyn/scanning.catalog.bom";
enum StartMode {
- FILTER, SERVLET, WEB_XML
+ FILTER, SERVLET, /** web-xml is not fully supported */ @Beta WEB_XML
}
public static final List<Class<? extends Filter>> DEFAULT_FILTERS = ImmutableList.of(
@@ -112,7 +112,7 @@ public class BrooklynRestApiLauncher {
private ContextHandler customContext;
private boolean deployJsgui = true;
private boolean disableHighAvailability = true;
- private final TestShutdownHandler shutdownListener = new TestShutdownHandler();
+ private ServerStoppingShutdownHandler shutdownListener;
protected BrooklynRestApiLauncher() {}
@@ -215,7 +215,12 @@ public class BrooklynRestApiLauncher {
((BrooklynProperties) mgmt.getConfig()).put(BrooklynServerConfig.BROOKLYN_CATALOG_URL, ManagementContextInternal.EMPTY_CATALOG_URL);
}
- return startServer(mgmt, context, summary, disableHighAvailability);
+ Server server = startServer(mgmt, context, summary, disableHighAvailability);
+ if (shutdownListener!=null) {
+ // not available in some modes, eg webapp
+ shutdownListener.setServer(server);
+ }
+ return server;
}
private ContextHandler filterContextHandler(ManagementContext mgmt) {
@@ -241,7 +246,7 @@ public class BrooklynRestApiLauncher {
ResourceConfig config = new DefaultResourceConfig();
for (Object r: BrooklynRestApi.getAllResources())
config.getSingletons().add(r);
- config.getSingletons().add(new ShutdownHandlerProvider(shutdownListener));
+ addShutdownListener(config, mgmt);
ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, managementContext);
@@ -253,6 +258,7 @@ public class BrooklynRestApiLauncher {
return context;
}
+ /** NB: not fully supported; use one of the other {@link StartMode}s */
private ContextHandler webXmlContextHandler(ManagementContext mgmt) {
// TODO add security to web.xml
WebAppContext context;
@@ -266,12 +272,15 @@ public class BrooklynRestApiLauncher {
throw new IllegalStateException("Cannot find WAR for REST API. Expected in target/*.war, Maven repo, or in source directories.");
}
context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, mgmt);
-
+ // TODO shutdown hook
+
return context;
}
/** starts a server, on all NICs if security is configured,
- * otherwise (no security) only on loopback interface */
+ * otherwise (no security) only on loopback interface
+ * @deprecated since 0.9.0 becoming private */
+ @Deprecated
public static Server startServer(ManagementContext mgmt, ContextHandler context, String summary, boolean disableHighAvailability) {
// TODO this repeats code in BrooklynLauncher / WebServer. should merge the two paths.
boolean secure = mgmt != null && !BrooklynWebConfig.hasNoSecurityOptions(mgmt.getConfig());
@@ -292,8 +301,11 @@ public class BrooklynRestApiLauncher {
return startServer(context, summary, bindLocation);
}
+ /** @deprecated since 0.9.0 becoming private */
+ @Deprecated
public static Server startServer(ContextHandler context, String summary, InetSocketAddress bindLocation) {
Server server = new Server(bindLocation);
+
server.setHandler(context);
try {
server.start();
@@ -361,6 +373,12 @@ public class BrooklynRestApiLauncher {
ManagementContext mgmt = OsgiCompat.getManagementContext(context);
config.getSingletons().add(new ManagementContextProvider(mgmt));
+ addShutdownListener(config, mgmt);
+ }
+
+ protected synchronized void addShutdownListener(ResourceConfig config, ManagementContext mgmt) {
+ if (shutdownListener!=null) throw new IllegalStateException("Can only retrieve one shutdown listener");
+ shutdownListener = new ServerStoppingShutdownHandler(mgmt);
config.getSingletons().add(new ShutdownHandlerProvider(shutdownListener));
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
index dea8bca..e63a239 100644
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
@@ -47,7 +47,7 @@ import org.apache.brooklyn.rest.util.BrooklynRestResourceUtils;
import org.apache.brooklyn.rest.util.NullHttpServletRequestProvider;
import org.apache.brooklyn.rest.util.NullServletConfigProvider;
import org.apache.brooklyn.rest.util.ShutdownHandlerProvider;
-import org.apache.brooklyn.rest.util.TestShutdownHandler;
+import org.apache.brooklyn.rest.util.NoOpRecordingShutdownHandler;
import org.apache.brooklyn.rest.util.json.BrooklynJacksonJsonProvider;
import org.apache.brooklyn.util.exceptions.Exceptions;
@@ -56,7 +56,7 @@ public abstract class BrooklynRestApiTest {
protected ManagementContext manager;
protected boolean useLocalScannedCatalog = false;
- protected TestShutdownHandler shutdownListener = createShutdownHandler();
+ protected NoOpRecordingShutdownHandler shutdownListener = createShutdownHandler();
@BeforeMethod(alwaysRun = true)
public void setUpMethod() {
@@ -69,8 +69,8 @@ public abstract class BrooklynRestApiTest {
useLocalScannedCatalog = true;
}
- private TestShutdownHandler createShutdownHandler() {
- return new TestShutdownHandler();
+ private NoOpRecordingShutdownHandler createShutdownHandler() {
+ return new NoOpRecordingShutdownHandler();
}
protected synchronized ManagementContext getManagementContext() {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java
new file mode 100644
index 0000000..a99d3d9
--- /dev/null
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/NoOpRecordingShutdownHandler.java
@@ -0,0 +1,39 @@
+/*
+ * 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.brooklyn.rest.util;
+
+import org.apache.brooklyn.rest.util.ShutdownHandler;
+
+public class NoOpRecordingShutdownHandler implements ShutdownHandler {
+ private volatile boolean isRequested;
+
+ @Override
+ public void onShutdownRequest() {
+ isRequested = true;
+ }
+
+ public boolean isRequested() {
+ return isRequested;
+ }
+
+ public void reset() {
+ isRequested = false;
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
new file mode 100644
index 0000000..7244378
--- /dev/null
+++ b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/ServerStoppingShutdownHandler.java
@@ -0,0 +1,67 @@
+/*
+ * 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.brooklyn.rest.util;
+
+import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.eclipse.jetty.server.Server;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/* not the cleanest way to enforce a clean shutdown, but a simple and effective way;
+ * usage is restricted to BrooklynRestApiLauncher and subclasses, to stop it inline.
+ * the main app stops the server in a more principled way using callbacks. */
+public class ServerStoppingShutdownHandler implements ShutdownHandler {
+
+ private static final Logger log = LoggerFactory.getLogger(ServerStoppingShutdownHandler.class);
+
+ private final ManagementContext mgmt;
+ private Server server;
+
+ public ServerStoppingShutdownHandler(ManagementContext mgmt) {
+ this.mgmt = mgmt;
+ }
+
+ @Override
+ public void onShutdownRequest() {
+ log.info("Shutting down (when running in rest-api dev mode)...");
+
+ // essentially same as BrooklynLauncher.terminate() but cut down as this is only used in dev mode
+
+ if (server!=null) {
+ try {
+ server.stop();
+ server.join();
+ } catch (Exception e) {
+ log.debug("Stopping server gave an error (not usually a concern): "+e);
+ /* NPE may be thrown e.g. if threadpool not started */
+ }
+ }
+
+ if (mgmt instanceof ManagementContextInternal) {
+ ((ManagementContextInternal)mgmt).terminate();
+ }
+ }
+
+ /** Expect this to be injeted; typically it is not known when this is created, but we need it to trigger shutdown. */
+ public void setServer(Server server) {
+ this.server = server;
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java b/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java
deleted file mode 100644
index 87fbb24..0000000
--- a/brooklyn-server/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/TestShutdownHandler.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * 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.brooklyn.rest.util;
-
-import org.apache.brooklyn.rest.util.ShutdownHandler;
-
-public class TestShutdownHandler implements ShutdownHandler {
- private volatile boolean isRequested;
-
- @Override
- public void onShutdownRequest() {
- isRequested = true;
- }
-
- public boolean isRequested() {
- return isRequested;
- }
-
- public void reset() {
- isRequested = false;
- }
-
-}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2a432cf3/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java b/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
index 1c66a04..96527db 100644
--- a/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
+++ b/brooklyn-server/server-cli/src/main/java/org/apache/brooklyn/cli/Main.java
@@ -19,6 +19,12 @@
package org.apache.brooklyn.cli;
import static com.google.common.base.Preconditions.checkNotNull;
+import groovy.lang.GroovyClassLoader;
+import groovy.lang.GroovyShell;
+import io.airlift.command.Cli;
+import io.airlift.command.Cli.CliBuilder;
+import io.airlift.command.Command;
+import io.airlift.command.Option;
import java.io.Console;
import java.io.IOException;
@@ -32,17 +38,6 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.annotations.Beta;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
-import com.google.common.base.Objects.ToStringHelper;
-import com.google.common.base.Stopwatch;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
-
import org.apache.brooklyn.api.catalog.BrooklynCatalog;
import org.apache.brooklyn.api.catalog.CatalogItem;
import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
@@ -89,16 +84,19 @@ import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.Enums;
import org.apache.brooklyn.util.net.Networking;
import org.apache.brooklyn.util.text.Identifiers;
-import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
-import groovy.lang.GroovyClassLoader;
-import groovy.lang.GroovyShell;
-import io.airlift.command.Cli;
-import io.airlift.command.Cli.CliBuilder;
-import io.airlift.command.Command;
-import io.airlift.command.Option;
+import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Objects.ToStringHelper;
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
/**
* This class is the primary CLI for brooklyn.
@@ -474,17 +472,22 @@ public class Main extends AbstractMain {
}
BrooklynServerDetails server = launcher.getServerDetails();
- ManagementContext ctx = server.getManagementContext();
+ ManagementContext mgmt = server.getManagementContext();
if (verbose) {
Entities.dumpInfo(launcher.getApplications());
}
if (!exitAndLeaveAppsRunningAfterStarting) {
- waitAfterLaunch(ctx, shutdownHandler);
+ waitAfterLaunch(mgmt, shutdownHandler);
}
- // will call mgmt.terminate() in BrooklynShutdownHookJob
+ // BrooklynShutdownHookJob will invoke terminate() to do mgmt.terminate() and BrooklynWebServer.stop();
+ // and System.exit is invoked immediately after ...
+ // but seems better to do it explicitly here.
+ // ('launcher' is local to us so the caller *cannot* do it, and no harm in doing it twice.)
+ launcher.terminate();
+
return null;
}