This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 0089307 GEODE-6492: Ensure that starting HttpService can fail
gracefully in the face of errors (#3275)
0089307 is described below
commit 008930774c2479cc460dfee91cf131c7908d0eff
Author: Jens Deppe <[email protected]>
AuthorDate: Fri Mar 8 05:44:21 2019 -0800
GEODE-6492: Ensure that starting HttpService can fail gracefully in the
face of errors (#3275)
- If the HttpService (or war deployment) fails, simply log a warning
message and continue.
- Add a simple gradle building test which ensures that 'geode-core' is
the only module required to start a basic Cache.
---
geode-assembly/build.gradle | 8 ++-
.../GradleBuildWithGeodeCoreAcceptanceTest.java | 70 ++++++++++++++++++++++
.../gradle-test-projects/management/build.gradle | 39 ++++++++++++
.../management/src/main/java/ServerTestApp.java | 27 +++++++++
.../distributed/internal/InternalLocator.java | 15 ++---
.../geode/internal/cache/GemFireCacheImpl.java | 19 +++---
.../apache/geode/internal/cache/HttpService.java | 2 +
.../apache/geode/internal/cache/InternalCache.java | 3 +-
.../cache/InternalCacheForClientAccess.java | 3 +-
.../internal/cache/xmlcache/CacheCreation.java | 3 +-
.../geode/management/internal/ManagementAgent.java | 7 ++-
.../geode/management/internal/RestAgent.java | 38 ++++++------
.../geode/test/junit/rules/RequiresGeodeHome.java | 5 ++
.../org/apache/geode/test/util/ResourceUtils.java | 15 +++++
14 files changed, 212 insertions(+), 42 deletions(-)
diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index 8c4133f..cf718aa 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -100,6 +100,10 @@ repositories {
artifact
'/dist/tomcat/tomcat-6/v6.0.37/bin/[organisation]-[module]-[revision].[ext]'
}
}
+ // For gradle tooling dependencies
+ maven {
+ url 'https://repo.gradle.org/gradle/libs-releases'
+ }
}
configurations {
@@ -211,6 +215,7 @@ dependencies {
}
acceptanceTestCompile(project(':geode-assembly:geode-assembly-test'))
acceptanceTestCompile('org.apache.httpcomponents:httpclient')
+ acceptanceTestCompile('org.gradle:gradle-tooling-api:5.2.1')
uiTestCompile(project(':geode-core'))
@@ -537,7 +542,7 @@ distributions {
}
}
-tasks.withType(Test){
+tasks.withType(Test) {
dependsOn installDist
environment 'GEODE_HOME', "$buildDir/install/${distributions.main.baseName}"
}
@@ -545,6 +550,7 @@ tasks.withType(Test){
// Make build final task to generate all test and product resources
build.dependsOn installDist
+acceptanceTest.dependsOn(rootProject.getTasksByName("publishToMavenLocal",
true))
installDist.dependsOn ':extensions:geode-modules-assembly:dist'
distributedTest.dependsOn ':extensions:session-testing-war:war'
distributedTest.dependsOn ':geode-old-versions:build'
diff --git
a/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/GradleBuildWithGeodeCoreAcceptanceTest.java
b/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/GradleBuildWithGeodeCoreAcceptanceTest.java
new file mode 100644
index 0000000..6859331
--- /dev/null
+++
b/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/GradleBuildWithGeodeCoreAcceptanceTest.java
@@ -0,0 +1,70 @@
+/*
+ * 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.geode.management.internal.rest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.net.URL;
+
+import org.gradle.tooling.BuildLauncher;
+import org.gradle.tooling.GradleConnector;
+import org.gradle.tooling.ProjectConnection;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.GemFireVersion;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+import org.apache.geode.test.util.ResourceUtils;
+
+public class GradleBuildWithGeodeCoreAcceptanceTest {
+
+ @Rule
+ public RequiresGeodeHome geodeHome = new RequiresGeodeHome();
+
+ @Rule
+ public TemporaryFolder temp = new TemporaryFolder();
+
+ @Test
+ public void testBasicGradleBuild() throws Exception {
+ URL projectDir =
ResourceUtils.getResource("/gradle-test-projects/management");
+ assertThat(projectDir).isNotNull();
+
+ String geodeVersion = GemFireVersion.getGemFireVersion();
+
+ File buildDir = temp.getRoot();
+ ResourceUtils.copyDirectoryResource(projectDir, buildDir);
+
+ GradleConnector connector = GradleConnector.newConnector();
+ connector.useBuildDistribution();
+ connector.forProjectDirectory(buildDir);
+
+ ProjectConnection connection = connector.connect();
+ BuildLauncher build = connection.newBuild();
+
+ build.setStandardError(System.err);
+ build.setStandardOutput(System.out);
+ build.withArguments("-PgeodeVersion=" + geodeVersion,
+ "-PgeodeHome=" + geodeHome.toString());
+
+ build.forTasks("installDist", "run");
+ build.run();
+
+ connection.close();
+ }
+
+}
diff --git
a/geode-assembly/src/acceptanceTest/resources/gradle-test-projects/management/build.gradle
b/geode-assembly/src/acceptanceTest/resources/gradle-test-projects/management/build.gradle
new file mode 100644
index 0000000..46c3118
--- /dev/null
+++
b/geode-assembly/src/acceptanceTest/resources/gradle-test-projects/management/build.gradle
@@ -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.
+ */
+
+plugins {
+ id 'java'
+ id 'application'
+}
+
+version = '1.0'
+
+repositories {
+ mavenLocal()
+ mavenCentral()
+}
+
+dependencies {
+ compile("org.apache.geode:geode-core:${findProperty('geodeVersion')}")
+ runtime('org.apache.logging.log4j:log4j-slf4j-impl:2.11.1')
+}
+
+application {
+ mainClassName = 'ServerTestApp'
+}
+
+run {
+ environment['GEODE_HOME'] = "${findProperty('geodeHome')}"
+}
diff --git
a/geode-assembly/src/acceptanceTest/resources/gradle-test-projects/management/src/main/java/ServerTestApp.java
b/geode-assembly/src/acceptanceTest/resources/gradle-test-projects/management/src/main/java/ServerTestApp.java
new file mode 100644
index 0000000..92fd76c
--- /dev/null
+++
b/geode-assembly/src/acceptanceTest/resources/gradle-test-projects/management/src/main/java/ServerTestApp.java
@@ -0,0 +1,27 @@
+/*
+ * 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.
+ */
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.RegionShortcut;
+
+public class ServerTestApp {
+ public static void main(String[] args) throws Exception {
+ Cache cache = new CacheFactory()
+ .set("start-dev-rest-api", "true")
+ .create();
+ cache.createRegionFactory(RegionShortcut.REPLICATE).create("REGION1");
+ }
+}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
index 75200bf..73dced1 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
@@ -708,13 +708,14 @@ public class InternalLocator extends Locator implements
ConnectListener, LogConf
new
ImmutablePair<>(HttpService.CLUSTER_MANAGEMENT_SERVICE_CONTEXT_PARAM,
clusterManagementService);
- try {
- myCache.getHttpService()
- .addWebApplication("/geode-management", gemfireManagementWar,
securityServiceAttr,
- cmServiceAttr);
- } catch (Exception e) {
- logger.error(e.getMessage(), e);
- }
+ myCache.getHttpService().ifPresent(x -> {
+ try {
+ x.addWebApplication("/geode-management", gemfireManagementWar,
securityServiceAttr,
+ cmServiceAttr);
+ } catch (Throwable e) {
+ logger.warn("Unable to start geode-management service: {}",
e.getMessage());
+ }
+ });
}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index 7f5caef..c864172 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -47,6 +47,7 @@ import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
+import java.util.Optional;
import java.util.Properties;
import java.util.ServiceLoader;
import java.util.Set;
@@ -609,7 +610,7 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
private final ClusterConfigurationLoader ccLoader = new
ClusterConfigurationLoader();
- private HttpService httpService;
+ private Optional<HttpService> httpService = Optional.ofNullable(null);
static {
// this works around jdk bug 6427854, reported in ticket #44434
@@ -933,9 +934,13 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
addRegionEntrySynchronizationListener(new
GatewaySenderQueueEntrySynchronizationListener());
backupService = new BackupService(this);
if (!this.isClient) {
- httpService = new HttpService(systemConfig.getHttpServiceBindAddress(),
- systemConfig.getHttpServicePort(), SSLConfigurationFactory
- .getSSLConfigForComponent(systemConfig,
SecurableCommunicationChannel.WEB));
+ try {
+ httpService = Optional.of(new
HttpService(systemConfig.getHttpServiceBindAddress(),
+ systemConfig.getHttpServicePort(), SSLConfigurationFactory
+ .getSSLConfigForComponent(systemConfig,
SecurableCommunicationChannel.WEB)));
+ } catch (Throwable ex) {
+ logger.warn("Could not enable HttpService: {}", ex.getMessage());
+ }
}
} // synchronized
}
@@ -947,7 +952,7 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
}
@Override
- public HttpService getHttpService() {
+ public Optional<HttpService> getHttpService() {
return httpService;
}
@@ -2193,9 +2198,7 @@ public class GemFireCacheImpl implements InternalCache,
InternalClientCache, Has
stopRedisServer();
- if (httpService != null) {
- httpService.stop();
- }
+ httpService.ifPresent(HttpService::stop);
// no need to track PR instances since we won't create any more
// cacheServers or gatewayHubs
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/HttpService.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/HttpService.java
index 62f8ac9..e7b0bcf 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/HttpService.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/HttpService.java
@@ -157,6 +157,8 @@ public class HttpService {
this.bindAddress = bindAddress;
}
this.port = port;
+
+ logger.info("Enabled HttpService on port {}", port);
}
public Server getHttpServer() {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
index 87fc1dc..a3d38ba 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java
@@ -20,6 +20,7 @@ import java.net.URL;
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.Executor;
@@ -371,7 +372,7 @@ public interface InternalCache extends Cache,
Extensible<Cache>, CacheTime {
*/
InternalCacheForClientAccess getCacheForProcessingClientRequests();
- HttpService getHttpService();
+ Optional<HttpService> getHttpService();
void initialize();
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
index e64c6a7..bde6506 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java
@@ -23,6 +23,7 @@ import java.net.URL;
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.Executor;
@@ -1220,7 +1221,7 @@ public class InternalCacheForClientAccess implements
InternalCache {
}
@Override
- public HttpService getHttpService() {
+ public Optional<HttpService> getHttpService() {
return delegate.getHttpService();
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
index 325ea81..7f47d09 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java
@@ -31,6 +31,7 @@ import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.Executor;
@@ -2437,7 +2438,7 @@ public class CacheCreation implements InternalCache {
}
@Override
- public HttpService getHttpService() {
+ public Optional<HttpService> getHttpService() {
throw new UnsupportedOperationException("Should not be invoked");
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
index ed99159..191a53a 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
@@ -196,7 +196,9 @@ public class ManagementAgent {
}
try {
- if (agentUtil.isAnyWarFileAvailable(adminRestWar, pulseWar)) {
+ if (cache.getHttpService().isPresent()
+ && agentUtil.isAnyWarFileAvailable(adminRestWar, pulseWar)) {
+ HttpService httpService = cache.getHttpService().get();
final String bindAddress = this.config.getHttpServiceBindAddress();
final int port = this.config.getHttpServicePort();
@@ -208,7 +210,6 @@ public class ManagementAgent {
new
ImmutablePair<>(HttpService.GEODE_SSLCONFIG_SERVLET_CONTEXT_PARAM,
createSslProps());
- HttpService httpService = cache.getHttpService();
// if jmx manager is running, admin rest should be available, either
on locator or server
if (agentUtil.isAnyWarFileAvailable(adminRestWar)) {
httpService.addWebApplication("/gemfire", adminRestWar,
securityServiceAttr);
@@ -234,7 +235,7 @@ public class ManagementAgent {
.concat(String.valueOf(port)).concat("/pulse/"));
}
}
- } catch (Exception e) {
+ } catch (Throwable e) {
setStatusMessage(managerBean, "HTTP service failed to start with "
+ e.getClass().getSimpleName() + " '" + e.getMessage() + "'");
throw new ManagementException("HTTP service failed to start", e);
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
b/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
index 7dc06f6..b9edf1a 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java
@@ -71,9 +71,8 @@ public class RestAgent {
// create region to hold query information (queryId, queryString).
Added
// for the developer REST APIs
RestAgent.createParameterizedQueryRegion();
-
- } catch (RuntimeException e) {
- logger.debug(e.getMessage(), e);
+ } catch (Throwable e) {
+ logger.warn("Unable to start dev REST API: {}", e.toString());
}
}
}
@@ -87,7 +86,7 @@ public class RestAgent {
}
// Start HTTP service in embedded mode
- public void startHttpService(InternalCache cache) {
+ public void startHttpService(InternalCache cache) throws Exception {
// Find the developer REST WAR file
final String gemfireAPIWar = agentUtil.findWarLocation("geode-web-api");
if (gemfireAPIWar == null) {
@@ -95,24 +94,23 @@ public class RestAgent {
"Unable to find GemFire Developer REST API WAR file; the Developer
REST Interface for GemFire will not be accessible.");
}
- try {
- // Check if we're already running inside Tomcat
- if (isRunningInTomcat()) {
- logger.warn(
- "Detected presence of catalina system properties. HTTP service
will not be started. To enable the GemFire Developer REST API, please deploy
the /geode-web-api WAR file in your application server.");
- } else if (agentUtil.isAnyWarFileAvailable(gemfireAPIWar)) {
-
- Pair<String, Object> securityServiceAttr =
- new
ImmutablePair<>(HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM,
- securityService);
-
- HttpService httpService = cache.getHttpService();
- httpService
- .addWebApplication("/gemfire-api", gemfireAPIWar,
securityServiceAttr);
+ // Check if we're already running inside Tomcat
+ if (isRunningInTomcat()) {
+ logger.warn(
+ "Detected presence of catalina system properties. HTTP service will
not be started. To enable the GemFire Developer REST API, please deploy the
/geode-web-api WAR file in your application server.");
+ } else if (agentUtil.isAnyWarFileAvailable(gemfireAPIWar)) {
+
+ Pair<String, Object> securityServiceAttr =
+ new
ImmutablePair<>(HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM,
+ securityService);
+
+ if (cache.getHttpService().isPresent()) {
+ HttpService httpService = cache.getHttpService().get();
+ httpService.addWebApplication("/gemfire-api", gemfireAPIWar,
securityServiceAttr);
httpService.addWebApplication("/geode", gemfireAPIWar,
securityServiceAttr);
+ } else {
+ logger.warn("HttpService is not available - could not start Dev REST
API");
}
- } catch (Exception e) {
- throw new RuntimeException("HTTP service failed to start due to " +
e.getMessage());
}
}
diff --git
a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RequiresGeodeHome.java
b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RequiresGeodeHome.java
index 9c205e4..65dc402 100644
---
a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RequiresGeodeHome.java
+++
b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RequiresGeodeHome.java
@@ -48,4 +48,9 @@ public class RequiresGeodeHome extends
SerializableExternalResource {
return geodeHome;
}
+
+ @Override
+ public String toString() {
+ return getGeodeHome().getAbsolutePath();
+ }
}
diff --git
a/geode-junit/src/main/java/org/apache/geode/test/util/ResourceUtils.java
b/geode-junit/src/main/java/org/apache/geode/test/util/ResourceUtils.java
index 58f78ca..9631531 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/util/ResourceUtils.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/util/ResourceUtils.java
@@ -25,6 +25,7 @@ import java.net.URISyntaxException;
import java.net.URL;
import com.google.common.io.Resources;
+import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
/**
@@ -84,4 +85,18 @@ public class ResourceUtils {
assertThat(targetFile).hasSameContentAs(new File(resource.toURI()));
return targetFile;
}
+
+ /**
+ * Copies a directory, pointed to by a {@code resource}, to a {@code
targetFolder}
+ *
+ * @param resource a file-based resource referencing a directory
+ * @param targetFolder the directory to which to copy the resource and all
files within that
+ * resource.
+ */
+ public static void copyDirectoryResource(final URL resource, final File
targetFolder)
+ throws IOException {
+ File source = new File(resource.getPath());
+ assertThat(source.exists()).as("Source does not exist: " +
resource.getPath());
+ FileUtils.copyDirectory(source, targetFolder);
+ }
}
