This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository 
https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git

commit 8cd9cb8c7e53333327e95b99e37b879c5c4e5381
Author: heyile <[email protected]>
AuthorDate: Thu Oct 11 22:20:33 2018 +0800

    [SCB-837] make http2 production ready: modify config settings
---
 .../vertx/metrics/DefaultHttpClientMetrics.java           | 15 ++++++++++++++-
 .../transport/rest/client/RestTransportClient.java        |  8 ++++----
 .../transport/rest/client/TransportClientConfig.java      | 15 ++++++++++++++-
 .../transport/rest/client/TestTransportClientConfig.java  | 12 +++++++++++-
 .../transport/rest/vertx/RestServerVerticle.java          |  2 +-
 .../servicecomb/transport/rest/vertx/TransportConfig.java | 12 +++++++++---
 .../transport/rest/vertx/TestTransportConfig.java         | 11 +++++++++--
 7 files changed, 62 insertions(+), 13 deletions(-)

diff --git 
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
 
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
index 3fb9af3..c184072 100644
--- 
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
+++ 
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
@@ -139,9 +139,22 @@ public class DefaultHttpClientMetrics implements
 
   @Override
   public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String 
remoteName) {
-    //we can get endpointMetric info here, so set the endpointMetric info 
directly
+    // when host of createEndpoint is not ip but a hostName
+    // get from remoteAddress will return null
+    // in this time need to try again with remoteName
+    // connected is a low frequency method, this try logic will not cause 
performance problem
+
     DefaultClientEndpointMetric clientEndpointMetric = 
this.clientEndpointMetricManager
         .getClientEndpointMetric(remoteAddress);
+    if (clientEndpointMetric == null) {
+      LOGGER.warn("can not find endpointMetric directly by remoteAddress {}, 
try again with remoteName {}",
+          remoteAddress, remoteName);
+      SocketAddressImpl address = new SocketAddressImpl(remoteAddress.port(), 
remoteName);
+      clientEndpointMetric = 
this.clientEndpointMetricManager.getClientEndpointMetric(address);
+    }
+    // it's better to be done in endpointConnected
+    // but there is bug before vertx 3.6.0 vertx not invoke endpointConnected 
for http2
+    // to avoid this bug, we move the logic here
     clientEndpointMetric.onConnect();
     return new DefaultHttpSocketMetric(clientEndpointMetric);
   }
diff --git 
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
 
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
index 2ce5620..133c180 100644
--- 
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
+++ 
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
@@ -88,12 +88,12 @@ public class RestTransportClient {
   private static HttpClientOptions createHttp2ClientOptions() {
     HttpClientOptions httpClientOptions = new HttpClientOptions();
     
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
-        .setUseAlpn(true)
-        
.setIdleTimeout(TransportClientConfig.getConnectionIdleTimeoutInSeconds())
+        .setUseAlpn(TransportClientConfig.getUseAlpn())
+        .setHttp2ClearTextUpgrade(false)
+        .setProtocolVersion(HttpVersion.HTTP_2)
+        
.setIdleTimeout(TransportClientConfig.getHttp2ConnectionIdleTimeoutInSeconds())
         
.setHttp2MultiplexingLimit(TransportClientConfig.getHttp2MultiplexingLimit())
         
.setHttp2MaxPoolSize(TransportClientConfig.getHttp2ConnectionMaxPoolSize())
-        .setProtocolVersion(HttpVersion.HTTP_2)
-        .setHttp2ClearTextUpgrade(false)
         
.setTryUseCompression(TransportClientConfig.getConnectionCompression());
 
     VertxTLSBuilder.buildHttpClientOptions(SSL_KEY, httpClientOptions);
diff --git 
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
 
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
index d3e82da..a8ae6c6 100644
--- 
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
+++ 
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
@@ -38,7 +38,7 @@ public final class TransportClientConfig {
   }
 
   public static int getHttp2ConnectionMaxPoolSize() {
-    return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
 3)
+    return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
 1)
         .get();
   }
 
@@ -47,6 +47,18 @@ public final class TransportClientConfig {
         .get();
   }
 
+  public static int getHttp2ConnectionIdleTimeoutInSeconds() {
+    return DynamicPropertyFactory.getInstance()
+        .getIntProperty("servicecomb.rest.client.http2.idleTimeoutInSeconds", 
0)
+        .get();
+  }
+
+  public static boolean getUseAlpn() {
+    return DynamicPropertyFactory.getInstance()
+        .getBooleanProperty("servicecomb.rest.client.http2.useAlpnEnabled", 
true)
+        .get();
+  }
+
   public static int getConnectionMaxPoolSize() {
     return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.connection.maxPoolSize",
 5)
         .get();
@@ -63,6 +75,7 @@ public final class TransportClientConfig {
         .get();
   }
 
+
   public static boolean getConnectionCompression() {
     return DynamicPropertyFactory.getInstance()
         .getBooleanProperty("servicecomb.rest.client.connection.compression", 
false)
diff --git 
a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/TestTransportClientConfig.java
 
b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/TestTransportClientConfig.java
index 13a3d7a..d4ebd26 100644
--- 
a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/TestTransportClientConfig.java
+++ 
b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/TestTransportClientConfig.java
@@ -43,7 +43,17 @@ public class TestTransportClientConfig {
 
   @Test
   public void getHttp2ConnectionMaxPoolSize() {
-    Assert.assertEquals(3, 
TransportClientConfig.getHttp2ConnectionMaxPoolSize());
+    Assert.assertEquals(1, 
TransportClientConfig.getHttp2ConnectionMaxPoolSize());
+  }
+
+  @Test
+  public void getHttp2ConnectionIdleTimeoutInSeconds() {
+    Assert.assertEquals(0, 
TransportClientConfig.getHttp2ConnectionIdleTimeoutInSeconds());
+  }
+
+  @Test
+  public void getUseAplnEnabled() {
+    Assert.assertTrue(TransportClientConfig.getUseAlpn());
   }
 
   @Test
diff --git 
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
 
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
index 0d10f18..a61577d 100644
--- 
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
+++ 
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
@@ -254,7 +254,7 @@ public class RestServerVerticle extends AbstractVerticle {
     serverOptions.setMaxHeaderSize(TransportConfig.getMaxHeaderSize());
     
serverOptions.setMaxInitialLineLength(TransportConfig.getMaxInitialLineLength());
     if (endpointObject.isHttp2Enabled()) {
-      serverOptions.setUseAlpn(true)
+      serverOptions.setUseAlpn(TransportConfig.getUseAlpn())
           .setInitialSettings(new 
Http2Settings().setMaxConcurrentStreams(TransportConfig.getMaxConcurrentStreams()));
     }
     if (endpointObject.isSslEnabled()) {
diff --git 
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/TransportConfig.java
 
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/TransportConfig.java
index c1efa59..c9cb96d 100644
--- 
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/TransportConfig.java
+++ 
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/TransportConfig.java
@@ -38,8 +38,6 @@ public final class TransportConfig {
 
   public static final boolean DEFAULT_SERVER_COMPRESSION_SUPPORT = false;
 
-  public static final long DEFAULT_MAX_CONCURRENT_STREAMS = 200L;
-
   // 32K
   public static final int DEFAULT_SERVER_MAX_HEADER_SIZE = 32 * 1024;
 
@@ -83,11 +81,19 @@ public final class TransportConfig {
         .getBooleanProperty("servicecomb.rest.server.compression", 
DEFAULT_SERVER_COMPRESSION_SUPPORT)
         .get();
   }
+
   public static long getMaxConcurrentStreams() {
     return DynamicPropertyFactory.getInstance()
-        .getLongProperty("servicecomb.rest.server.http2.concurrentStreams", 
DEFAULT_MAX_CONCURRENT_STREAMS)
+        .getLongProperty("servicecomb.rest.server.http2.concurrentStreams", 
100L)
         .get();
   }
+
+  public static boolean getUseAlpn() {
+    return DynamicPropertyFactory.getInstance()
+        .getBooleanProperty("servicecomb.rest.server.http2.useAlpnEnabled", 
true)
+        .get();
+  }
+
   public static int getMaxHeaderSize() {
     return DynamicPropertyFactory.getInstance()
         .getIntProperty("servicecomb.rest.server.maxHeaderSize", 
DEFAULT_SERVER_MAX_HEADER_SIZE)
diff --git 
a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestTransportConfig.java
 
b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestTransportConfig.java
index 7493c04..a3970f4 100644
--- 
a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestTransportConfig.java
+++ 
b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/TestTransportConfig.java
@@ -143,9 +143,16 @@ public class TestTransportConfig {
 
   @Test
   public void testMaxConcurrentStreams() {
-    Assert.assertEquals(200L, TransportConfig.getMaxConcurrentStreams());
-    
ArchaiusUtils.setProperty("servicecomb.rest.server.http2.concurrentStreams", 
100L);
     Assert.assertEquals(100L, TransportConfig.getMaxConcurrentStreams());
+    
ArchaiusUtils.setProperty("servicecomb.rest.server.http2.concurrentStreams", 
200L);
+    Assert.assertEquals(200L, TransportConfig.getMaxConcurrentStreams());
+  }
+
+  @Test
+  public void testUseAlpn() {
+    Assert.assertTrue(TransportConfig.getUseAlpn());
+    ArchaiusUtils.setProperty("servicecomb.rest.server.http2.useAlpnEnabled", 
false);
+    Assert.assertFalse(TransportConfig.getUseAlpn());
   }
 
 

Reply via email to