[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16660260#comment-16660260
]
ASF GitHub Bot commented on SCB-837:
liubao68 closed pull request #947: [SCB-837] make http2 production ready
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 98f5fd205..d8d5286b1 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
@@ -34,6 +34,7 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
private final DefaultClientEndpointMetricManager clientEndpointMetricManager;
private final HttpClient client;
@@ -77,8 +78,9 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
}
@Override
@@ -133,7 +135,22 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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) {
+ 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);
}
@Override
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
index e7455da26..ced194cd7 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
@@ -104,7 +104,12 @@ protected static void run() throws Throwable {
// ..
testSpringBoot2Standalone();
+
+testHttp2CStandalone();
+
testSpringBoot2Servlet();
+//http2
+testHttp2Standalone();
deploys.getEdge().stop();
}
@@ -145,6 +150,42 @@ private static void testStandalone() throws Throwable {
deploys.getBaseProducer().stop();
}
+ private static void testHttp2CStandalone() throws Throwable {
+deploys.getBaseHttp2CProducer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work for http2, do not need
+// ITJUnitUtils.runWithRest(TestRestServerConfig.class)
+ITJUnitUtils.run(TestRestServerConfigEdge.class);
+
+ITJUnitUtils.popProducer();
+deploys.getBaseHttp2CProducer().stop();
+ }
+
+ private static void testHttp2Standalone() throws Throwable {
+deploys.getBaseHttp2Producer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16660171#comment-16660171 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19662386) Coverage increased (+0.03%) to 86.432% when pulling **bbe340b678bab5414efdbd8e6b2e00b4bded709f on heyile:metrics_http2** into **56d8899c4039813045ff11c004c55114bc6ec12b on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16660145#comment-16660145
]
ASF GitHub Bot commented on SCB-837:
heyile closed pull request #947: [SCB-837] make http2 production ready
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 98f5fd205..d8d5286b1 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
@@ -34,6 +34,7 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
private final DefaultClientEndpointMetricManager clientEndpointMetricManager;
private final HttpClient client;
@@ -77,8 +78,9 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
}
@Override
@@ -133,7 +135,22 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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) {
+ 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);
}
@Override
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
index e7455da26..ced194cd7 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
@@ -104,7 +104,12 @@ protected static void run() throws Throwable {
// ..
testSpringBoot2Standalone();
+
+testHttp2CStandalone();
+
testSpringBoot2Servlet();
+//http2
+testHttp2Standalone();
deploys.getEdge().stop();
}
@@ -145,6 +150,42 @@ private static void testStandalone() throws Throwable {
deploys.getBaseProducer().stop();
}
+ private static void testHttp2CStandalone() throws Throwable {
+deploys.getBaseHttp2CProducer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work for http2, do not need
+// ITJUnitUtils.runWithRest(TestRestServerConfig.class)
+ITJUnitUtils.run(TestRestServerConfigEdge.class);
+
+ITJUnitUtils.popProducer();
+deploys.getBaseHttp2CProducer().stop();
+ }
+
+ private static void testHttp2Standalone() throws Throwable {
+deploys.getBaseHttp2Producer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16660146#comment-16660146 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659997#comment-16659997 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659996#comment-16659996
]
ASF GitHub Bot commented on SCB-837:
heyile closed pull request #947: [SCB-837] make http2 production ready
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 98f5fd205..d8d5286b1 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
@@ -34,6 +34,7 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
private final DefaultClientEndpointMetricManager clientEndpointMetricManager;
private final HttpClient client;
@@ -77,8 +78,9 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
}
@Override
@@ -133,7 +135,22 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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) {
+ 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);
}
@Override
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
index e7455da26..ced194cd7 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
@@ -104,7 +104,12 @@ protected static void run() throws Throwable {
// ..
testSpringBoot2Standalone();
+
+testHttp2CStandalone();
+
testSpringBoot2Servlet();
+//http2
+testHttp2Standalone();
deploys.getEdge().stop();
}
@@ -145,6 +150,42 @@ private static void testStandalone() throws Throwable {
deploys.getBaseProducer().stop();
}
+ private static void testHttp2CStandalone() throws Throwable {
+deploys.getBaseHttp2CProducer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work for http2, do not need
+// ITJUnitUtils.runWithRest(TestRestServerConfig.class)
+ITJUnitUtils.run(TestRestServerConfigEdge.class);
+
+ITJUnitUtils.popProducer();
+deploys.getBaseHttp2CProducer().stop();
+ }
+
+ private static void testHttp2Standalone() throws Throwable {
+deploys.getBaseHttp2Producer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659468#comment-16659468 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659465#comment-16659465
]
ASF GitHub Bot commented on SCB-837:
heyile closed pull request #947: [SCB-837] make http2 production ready
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 98f5fd205..d8d5286b1 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
@@ -34,6 +34,7 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
private final DefaultClientEndpointMetricManager clientEndpointMetricManager;
private final HttpClient client;
@@ -77,8 +78,9 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
}
@Override
@@ -133,7 +135,22 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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) {
+ 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);
}
@Override
diff --git
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
index e7455da26..ced194cd7 100644
---
a/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
+++
b/integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/ConsumerMain.java
@@ -104,7 +104,12 @@ protected static void run() throws Throwable {
// ..
testSpringBoot2Standalone();
+
+testHttp2CStandalone();
+
testSpringBoot2Servlet();
+//http2
+testHttp2Standalone();
deploys.getEdge().stop();
}
@@ -145,6 +150,42 @@ private static void testStandalone() throws Throwable {
deploys.getBaseProducer().stop();
}
+ private static void testHttp2CStandalone() throws Throwable {
+deploys.getBaseHttp2CProducer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work for http2, do not need
+// ITJUnitUtils.runWithRest(TestRestServerConfig.class)
+ITJUnitUtils.run(TestRestServerConfigEdge.class);
+
+ITJUnitUtils.popProducer();
+deploys.getBaseHttp2CProducer().stop();
+ }
+
+ private static void testHttp2Standalone() throws Throwable {
+deploys.getBaseHttp2Producer().ensureReady();
+
+ITJUnitUtils.addProducer("it-producer");
+
+runShareTestCases();
+
+// currently not support update 3rd url, so only test once
+ITJUnitUtils.run(Test3rdPartyInvocation.class);
+
+//as setMaxInitialLineLength() is not work
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659458#comment-16659458 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659453#comment-16659453
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226934287
##
File path:
integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/deploy/Deploys.java
##
@@ -187,6 +194,23 @@ private void initBaseProducer() {
baseProducer = new MicroserviceDeploy(definition);
}
+
+ private void initBaseHttp2Producer() {
+MicroserviceDeployDefinition definition = new
MicroserviceDeployDefinition();
+definition.setDeployName("baseHttp2Producer");
+definition.setCmd("it-producer");
+definition.setArgs(new String[] {});
+definition.setArgs(new String[]
{"-Dservicecomb.rest.address=0.0.0.0:0?protocol=http2",
+"-Dservicecomb.highway.address=0.0.0.0:0?protocol=http2"});
Review comment:
do not need ? or do need ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16659454#comment-16659454
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226935547
##
File path:
integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/deploy/Deploys.java
##
@@ -187,6 +194,23 @@ private void initBaseProducer() {
baseProducer = new MicroserviceDeploy(definition);
}
+
+ private void initBaseHttp2Producer() {
+MicroserviceDeployDefinition definition = new
MicroserviceDeployDefinition();
+definition.setDeployName("baseHttp2Producer");
+definition.setCmd("it-producer");
+definition.setArgs(new String[] {});
+definition.setArgs(new String[]
{"-Dservicecomb.rest.address=0.0.0.0:0?protocol=http2",
+"-Dservicecomb.highway.address=0.0.0.0:0?protocol=http2"});
Review comment:
you means do not need to overide ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16658805#comment-16658805
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226934287
##
File path:
integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/deploy/Deploys.java
##
@@ -187,6 +194,23 @@ private void initBaseProducer() {
baseProducer = new MicroserviceDeploy(definition);
}
+
+ private void initBaseHttp2Producer() {
+MicroserviceDeployDefinition definition = new
MicroserviceDeployDefinition();
+definition.setDeployName("baseHttp2Producer");
+definition.setCmd("it-producer");
+definition.setArgs(new String[] {});
+definition.setArgs(new String[]
{"-Dservicecomb.rest.address=0.0.0.0:0?protocol=http2",
+"-Dservicecomb.highway.address=0.0.0.0:0?protocol=http2"});
Review comment:
do not need ? or do need ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16658807#comment-16658807
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226935547
##
File path:
integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/deploy/Deploys.java
##
@@ -187,6 +194,23 @@ private void initBaseProducer() {
baseProducer = new MicroserviceDeploy(definition);
}
+
+ private void initBaseHttp2Producer() {
+MicroserviceDeployDefinition definition = new
MicroserviceDeployDefinition();
+definition.setDeployName("baseHttp2Producer");
+definition.setCmd("it-producer");
+definition.setArgs(new String[] {});
+definition.setArgs(new String[]
{"-Dservicecomb.rest.address=0.0.0.0:0?protocol=http2",
+"-Dservicecomb.highway.address=0.0.0.0:0?protocol=http2"});
Review comment:
you means do not need to overide ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16658806#comment-16658806
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226935470
##
File path:
integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/deploy/Deploys.java
##
@@ -187,6 +194,23 @@ private void initBaseProducer() {
baseProducer = new MicroserviceDeploy(definition);
}
+
+ private void initBaseHttp2Producer() {
+MicroserviceDeployDefinition definition = new
MicroserviceDeployDefinition();
+definition.setDeployName("baseHttp2Producer");
+definition.setCmd("it-producer");
+definition.setArgs(new String[] {});
+definition.setArgs(new String[]
{"-Dservicecomb.rest.address=0.0.0.0:0?protocol=http2",
+"-Dservicecomb.highway.address=0.0.0.0:0?protocol=http2"});
Review comment:
you means do not need to overide ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16657815#comment-16657815 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19630471) Coverage increased (+0.03%) to 86.436% when pulling **b7d006c021cdfcfbf7ddbec0638e5a31cd57e2be on heyile:metrics_http2** into **56d8899c4039813045ff11c004c55114bc6ec12b on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16656151#comment-16656151
]
ASF GitHub Bot commented on SCB-837:
liubao68 commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226514969
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
Review comment:
We can set default to invoke both of h2 and h2c, but make it configurable is
necessary, I think.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16655723#comment-16655723
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226419894
##
File path:
integration-tests/it-consumer/src/main/java/org/apache/servicecomb/it/deploy/Deploys.java
##
@@ -187,6 +194,23 @@ private void initBaseProducer() {
baseProducer = new MicroserviceDeploy(definition);
}
+
+ private void initBaseHttp2Producer() {
+MicroserviceDeployDefinition definition = new
MicroserviceDeployDefinition();
+definition.setDeployName("baseHttp2Producer");
+definition.setCmd("it-producer");
+definition.setArgs(new String[] {});
+definition.setArgs(new String[]
{"-Dservicecomb.rest.address=0.0.0.0:0?protocol=http2",
+"-Dservicecomb.highway.address=0.0.0.0:0?protocol=http2"});
Review comment:
do need to overide highway address
only test h2c, not test h2?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16655086#comment-16655086 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19591448) Coverage increased (+0.03%) to 86.362% when pulling **54d1cdc25ddec3961bc49e4177509d25fae94189 on heyile:metrics_http2** into **abdb21adee72717f8fadd6f5fa2a89ebe4e00920 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16654468#comment-16654468
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226137536
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
Review comment:
when I use setHttp2ClearTextUpgrade(true), I find that both h2 and h2c can
run as expect.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16654442#comment-16654442
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226137536
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
Review comment:
when I use setHttp2ClearTextUpgrade(true), I find that both h2 and h2c can
run as except.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16654406#comment-16654406
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r226131786
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +139,24 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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 {}",
Review comment:
ok, get
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16652837#comment-16652837
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225760967
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -48,9 +75,11 @@ public static int getConnectionIdleTimeoutInSeconds() {
}
public static boolean getConnectionKeepAlive() {
-return
DynamicPropertyFactory.getInstance().getBooleanProperty("servicecomb.rest.client.connection.keepAlive",
true).get();
+return
DynamicPropertyFactory.getInstance().getBooleanProperty("servicecomb.rest.client.connection.keepAlive",
true)
Review comment:
io.vertx.core.http.HttpClientOptions#DEFAULT_KEEP_ALIVE
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16652839#comment-16652839
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225761072
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -48,9 +75,11 @@ public static int getConnectionIdleTimeoutInSeconds() {
}
public static boolean getConnectionKeepAlive() {
-return
DynamicPropertyFactory.getInstance().getBooleanProperty("servicecomb.rest.client.connection.keepAlive",
true).get();
+return
DynamicPropertyFactory.getInstance().getBooleanProperty("servicecomb.rest.client.connection.keepAlive",
true)
+.get();
}
+
public static boolean getConnectionCompression() {
return DynamicPropertyFactory.getInstance()
.getBooleanProperty("servicecomb.rest.client.connection.compression",
false)
Review comment:
io.vertx.core.http.HttpClientOptions#DEFAULT_TRY_USE_COMPRESSION
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16652836#comment-16652836
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225760905
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -37,8 +39,33 @@ public static int getThreadCount() {
return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.thread-count",
1).get();
}
+ public static int getHttp2ConnectionMaxPoolSize() {
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
+HttpClientOptions.DEFAULT_HTTP2_MAX_POOL_SIZE)
+.get();
+ }
+
+ public static int getHttp2MultiplexingLimit() {
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.multiplexingLimit",
+HttpClientOptions.DEFAULT_HTTP2_MULTIPLEXING_LIMIT)
+.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();
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.connection.maxPoolSize",
5)
Review comment:
io.vertx.core.http.HttpClientOptions#DEFAULT_MAX_POOL_SIZE
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16652831#comment-16652831
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225760102
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -37,8 +39,33 @@ public static int getThreadCount() {
return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.thread-count",
1).get();
}
+ public static int getHttp2ConnectionMaxPoolSize() {
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
+HttpClientOptions.DEFAULT_HTTP2_MAX_POOL_SIZE)
+.get();
+ }
+
+ public static int getHttp2MultiplexingLimit() {
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.multiplexingLimit",
+HttpClientOptions.DEFAULT_HTTP2_MULTIPLEXING_LIMIT)
+.get();
+ }
+
+ public static int getHttp2ConnectionIdleTimeoutInSeconds() {
+return DynamicPropertyFactory.getInstance()
+.getIntProperty("servicecomb.rest.client.http2.idleTimeoutInSeconds",
0)
Review comment:
io.vertx.core.net.TCPSSLOptions#DEFAULT_IDLE_TIMEOUT
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16652829#comment-16652829
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225759853
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +139,24 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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 {}",
Review comment:
line from 142 to 145 explain when will get a null result.
but i think the warn log is not necessary, it's a normal status, no need to
warn
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16652825#comment-16652825
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225759652
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
Review comment:
last developer said that must set to false
but he did not make http2 production ready
so we better to confirm this again:
producer a is h2
producer b is h2c
one consumer can invoke both a and b
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16650935#comment-16650935
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225343801
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +139,24 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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 {}",
Review comment:
I'm not sure about it. maybe debug is better
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16650930#comment-16650930
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225343519
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
Review comment:
@wujimin should I add config with it ? I remember that I have asked you
about it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16650929#comment-16650929
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225343149
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
+.setProtocolVersion(HttpVersion.HTTP_2)
+
.setIdleTimeout(TransportClientConfig.getHttp2ConnectionIdleTimeoutInSeconds())
+
.setHttp2MultiplexingLimit(TransportClientConfig.getHttp2MultiplexingLimit())
+
.setHttp2MaxPoolSize(TransportClientConfig.getHttp2ConnectionMaxPoolSize())
Review comment:
It doesn't matter in fact. but i will change it .
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16650108#comment-16650108
]
ASF GitHub Bot commented on SCB-837:
liubao68 commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225134555
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +139,24 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+// 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 {}",
Review comment:
When will this happen? If it's too frequently, we can add only debug log.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16650105#comment-16650105
]
ASF GitHub Bot commented on SCB-837:
liubao68 commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225134320
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
Review comment:
If setting Http2ClearTextUpgrade to false on default, is it possible to
access the service using http protocol or we must using https? Maybe we can
add a configuration for this.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16650095#comment-16650095
]
ASF GitHub Bot commented on SCB-837:
liubao68 commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r225131283
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(TransportClientConfig.getUseAlpn())
+.setHttp2ClearTextUpgrade(false)
+.setProtocolVersion(HttpVersion.HTTP_2)
+
.setIdleTimeout(TransportClientConfig.getHttp2ConnectionIdleTimeoutInSeconds())
+
.setHttp2MultiplexingLimit(TransportClientConfig.getHttp2MultiplexingLimit())
+
.setHttp2MaxPoolSize(TransportClientConfig.getHttp2ConnectionMaxPoolSize())
Review comment:
Do we need to setMaxPoolSize & setHttp2MaxPoolSize both?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649252#comment-16649252 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19508805) Coverage increased (+0.007%) to 86.235% when pulling **8d6462e676671a87f07101adb258a54205dfd05b on heyile:metrics_http2** into **ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649242#comment-16649242
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224978733
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -38,7 +38,7 @@ public static int getThreadCount() {
}
public static int getHttp2ConnectionMaxPoolSize() {
-return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
3)
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
1)
Review comment:
ok, I will change it soon. (没往这里想都)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649216#comment-16649216
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224976892
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -38,7 +38,7 @@ public static int getThreadCount() {
}
public static int getHttp2ConnectionMaxPoolSize() {
-return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
3)
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
1)
Review comment:
why not just use vertx defined const value?
eg: io.vertx.core.http.HttpClientOptions#DEFAULT_HTTP2_MAX_POOL_SIZE
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647733#comment-16647733 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19487166) Coverage increased (+0.007%) to 86.235% when pulling **2946c4fe3b9f100179c3d21e5468ecd7b09986ee on heyile:metrics_http2** into **ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647734#comment-16647734 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19487163) Coverage increased (+0.007%) to 86.235% when pulling **2946c4fe3b9f100179c3d21e5468ecd7b09986ee on heyile:metrics_http2** into **ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647681#comment-16647681 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19485960) Coverage increased (+0.002%) to 86.23% when pulling **e402f088f88a8ea5591570223c252f6a32d5ba03 on heyile:metrics_http2** into **ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647641#comment-16647641
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224707000
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -37,8 +37,19 @@ public static int getThreadCount() {
return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.thread-count",
1).get();
}
+ public static int getHttp2ConnectionMaxPoolSize() {
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
3)
Review comment:
ok, get
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647637#comment-16647637
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224706326
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
##
@@ -37,8 +37,19 @@ public static int getThreadCount() {
return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.thread-count",
1).get();
}
+ public static int getHttp2ConnectionMaxPoolSize() {
+return
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
3)
Review comment:
use vertx default value
same problem about other config.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647636#comment-16647636 ] ASF GitHub Bot commented on SCB-837: wujimin commented on a change in pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224706048 ## File path: transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/TransportConfig.java ## @@ -38,6 +38,8 @@ public static final boolean DEFAULT_SERVER_COMPRESSION_SUPPORT = false; + public static final long DEFAULT_MAX_CONCURRENT_STREAMS = 200L; Review comment: just use vertx default value, no need to create new definition This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647631#comment-16647631
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224705287
##
File path:
transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
##
@@ -253,7 +254,8 @@ private HttpServerOptions createDefaultHttpServerOptions()
{
serverOptions.setMaxHeaderSize(TransportConfig.getMaxHeaderSize());
serverOptions.setMaxInitialLineLength(TransportConfig.getMaxInitialLineLength());
if (endpointObject.isHttp2Enabled()) {
- serverOptions.setUseAlpn(true);
+ serverOptions.setUseAlpn(true)
+ .setInitialSettings(new
Http2Settings().setMaxConcurrentStreams(TransportConfig.getMaxConcurrentStreams()));
Review comment:
get and set?
otherwise maybe lost default values in furture versions.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647628#comment-16647628
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224704790
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(true)
+
.setIdleTimeout(TransportClientConfig.getConnectionIdleTimeoutInSeconds())
Review comment:
create http2 idle timeout config
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647601#comment-16647601
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224703936
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
Review comment:
is it possible based on http1X options, and override new option?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647598#comment-16647598
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224703936
##
File path:
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/RestTransportClient.java
##
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
Review comment:
is it possible based on http1X options, and override new option?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647597#comment-16647597
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224703607
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -34,6 +36,9 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DefaultHttpClientMetrics.class);
Review comment:
declare but not used?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16647580#comment-16647580 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19485315) Coverage increased (+0.001%) to 86.229% when pulling **1ef9fd4a738e71548ab5185a0265a67000b65881 on heyile:metrics_http2** into **ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16646527#comment-16646527 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19470366) Coverage increased (+0.02%) to 86.234% when pulling **5da315c611aeb8206fa62d3964c3707fbe88e212 on heyile:metrics_http2** into **c3f1d662cd5b41b17b58e553a45b144c8101e8ad on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16646195#comment-16646195
]
ASF GitHub Bot commented on SCB-837:
heyile closed pull request #947: [SCB-837] make http2 production ready
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 98f5fd205..3fb9af337 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
@@ -19,6 +19,8 @@
import
org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultClientEndpointMetric;
import
org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultClientEndpointMetricManager;
import
org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultHttpSocketMetric;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientOptions;
@@ -34,6 +36,9 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DefaultHttpClientMetrics.class);
+
private final DefaultClientEndpointMetricManager clientEndpointMetricManager;
private final HttpClient client;
@@ -77,8 +82,9 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
}
@Override
@@ -133,7 +139,11 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+//we can get endpointMetric info here, so set the endpointMetric info
directly
+DefaultClientEndpointMetric clientEndpointMetric =
this.clientEndpointMetricManager
+.getClientEndpointMetric(remoteAddress);
+clientEndpointMetric.onConnect();
+return new DefaultHttpSocketMetric(clientEndpointMetric);
}
@Override
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 05f73a7f8..2ce5620fa 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
@@ -61,9 +61,7 @@ public void init(Vertx vertx) throws Exception {
HttpClientOptions httpClientOptions = createHttpClientOptions();
clientMgr = new ClientPoolManager<>(vertx, new
HttpClientPoolFactory(httpClientOptions));
-HttpClientOptions httpClientOptionshttp2 = createHttpClientOptions();
-
httpClientOptionshttp2.setUseAlpn(true).setProtocolVersion(HttpVersion.HTTP_2);
-httpClientOptionshttp2.setHttp2ClearTextUpgrade(false);
+HttpClientOptions httpClientOptionshttp2 = createHttp2ClientOptions();
clientMgrHttp2 = new ClientPoolManager<>(vertx, new
HttpClientPoolFactory(httpClientOptionshttp2));
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(true)
+
.setIdleTimeout(TransportClientConfig.getConnectionIdleTimeoutInSeconds())
+
.setHttp2MultiplexingLimit(TransportClientConfig.getHttp2MultiplexingLimit())
+
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16646196#comment-16646196 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16646193#comment-16646193
]
ASF GitHub Bot commented on SCB-837:
heyile closed pull request #947: [SCB-837] make http2 production ready
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 98f5fd205..3fb9af337 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
@@ -19,6 +19,8 @@
import
org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultClientEndpointMetric;
import
org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultClientEndpointMetricManager;
import
org.apache.servicecomb.foundation.vertx.metrics.metric.DefaultHttpSocketMetric;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientOptions;
@@ -34,6 +36,9 @@
*/
public class DefaultHttpClientMetrics implements
HttpClientMetrics {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DefaultHttpClientMetrics.class);
+
private final DefaultClientEndpointMetricManager clientEndpointMetricManager;
private final HttpClient client;
@@ -77,8 +82,9 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
}
@Override
@@ -133,7 +139,11 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+//we can get endpointMetric info here, so set the endpointMetric info
directly
+DefaultClientEndpointMetric clientEndpointMetric =
this.clientEndpointMetricManager
+.getClientEndpointMetric(remoteAddress);
+clientEndpointMetric.onConnect();
+return new DefaultHttpSocketMetric(clientEndpointMetric);
}
@Override
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 05f73a7f8..2ce5620fa 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
@@ -61,9 +61,7 @@ public void init(Vertx vertx) throws Exception {
HttpClientOptions httpClientOptions = createHttpClientOptions();
clientMgr = new ClientPoolManager<>(vertx, new
HttpClientPoolFactory(httpClientOptions));
-HttpClientOptions httpClientOptionshttp2 = createHttpClientOptions();
-
httpClientOptionshttp2.setUseAlpn(true).setProtocolVersion(HttpVersion.HTTP_2);
-httpClientOptionshttp2.setHttp2ClearTextUpgrade(false);
+HttpClientOptions httpClientOptionshttp2 = createHttp2ClientOptions();
clientMgrHttp2 = new ClientPoolManager<>(vertx, new
HttpClientPoolFactory(httpClientOptionshttp2));
@@ -87,6 +85,21 @@ private static HttpClientOptions createHttpClientOptions() {
return httpClientOptions;
}
+ private static HttpClientOptions createHttp2ClientOptions() {
+HttpClientOptions httpClientOptions = new HttpClientOptions();
+
httpClientOptions.setMaxPoolSize(TransportClientConfig.getConnectionMaxPoolSize())
+.setUseAlpn(true)
+
.setIdleTimeout(TransportClientConfig.getConnectionIdleTimeoutInSeconds())
+
.setHttp2MultiplexingLimit(TransportClientConfig.getHttp2MultiplexingLimit())
+
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16646194#comment-16646194 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16646111#comment-16646111
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224357351
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -83,13 +82,10 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-//only http1.1 will invoke this method, just make a check
-if (endpointMetric != null) {
- if (endpointMetric != socketMetric.getEndpointMetric()) {
-socketMetric.setEndpointMetric(endpointMetric);
- }
- endpointMetric.onConnect();
-}
+// as http2 client will not invoke this method, the endpointMetric info
will lost.
+// you can get more details from
https://github.com/eclipse-vertx/vert.x/issues/2660
+// hence, we will set endpointMetric info in the method
connected(SocketAddress remoteAddress, String remoteName)
+endpointMetric.onConnect();
Review comment:
move to "connected", so that will depend if vertx fixed the bug.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645858#comment-16645858
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224294134
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
+ .getClientEndpointMetricMap().get(remoteAddress);
+ // set endPointMetric when use http2
+ socketMetric.setEndpointMetric(clientEndpointMetric);
+} catch (Exception e) {
+ LOGGER.warn("if you use http2, there may cause a null pointer exception.
{}/{}", remoteAddress, remoteName);
Review comment:
because I have cast client to DefaultHttpClientMetrics and invoke the
method getMetrics(). I'm afraid that the method getMetrics() may return null,
so I just catch it for safety. But now, it seems impossible, I will change it
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645855#comment-16645855
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224294360
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
+ .getClientEndpointMetricMap().get(remoteAddress);
+ // set endPointMetric when use http2
+ socketMetric.setEndpointMetric(clientEndpointMetric);
Review comment:
ok, I will finish it soon
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645854#comment-16645854
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224294134
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
+ .getClientEndpointMetricMap().get(remoteAddress);
+ // set endPointMetric when use http2
+ socketMetric.setEndpointMetric(clientEndpointMetric);
+} catch (Exception e) {
+ LOGGER.warn("if you use http2, there may cause a null pointer exception.
{}/{}", remoteAddress, remoteName);
Review comment:
because I have cast client to DefaultHttpClientMetrics and invoke the
method getMetrics(). I'm afraid that the method getMetrics() may return null,
so I just catch it for safety. But now, it seems impossible, I will change it
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645853#comment-16645853
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224294246
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
+ .getClientEndpointMetricMap().get(remoteAddress);
+ // set endPointMetric when use http2
+ socketMetric.setEndpointMetric(clientEndpointMetric);
+} catch (Exception e) {
+ LOGGER.warn("if you use http2, there may cause a null pointer exception.
{}/{}", remoteAddress, remoteName);
Review comment:
because I have cast client to DefaultHttpClientMetrics and invoke the method
getMetrics(). I'm afraid that the method getMetrics() may return null, so I
just catch it for safety. But now, it seems impossible, I will modify it
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645822#comment-16645822
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224290996
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
+ .getClientEndpointMetricMap().get(remoteAddress);
+ // set endPointMetric when use http2
+ socketMetric.setEndpointMetric(clientEndpointMetric);
Review comment:
just clean all logic in endpointConnected, move them to this place
and add comments to describe why not just write them in endpointConnected
and not care if vertx fix this bug, just use the new logic is no problem.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645823#comment-16645823
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224291077
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
+ .getClientEndpointMetricMap().get(remoteAddress);
+ // set endPointMetric when use http2
+ socketMetric.setEndpointMetric(clientEndpointMetric);
+} catch (Exception e) {
+ LOGGER.warn("if you use http2, there may cause a null pointer exception.
{}/{}", remoteAddress, remoteName);
Review comment:
when will this exception happened?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645805#comment-16645805
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224288301
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
+ DefaultClientEndpointMetric clientEndpointMetric =
clientMetrics.clientEndpointMetricManager
Review comment:
just invoke:
clientEndpointMetricManager.getClientEndpointMetric(SocketAddress serverAddress)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645802#comment-16645802
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224288214
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -133,7 +144,18 @@ public void disconnected(Object webSocketMetric) {
@Override
public DefaultHttpSocketMetric connected(SocketAddress remoteAddress, String
remoteName) {
-return new DefaultHttpSocketMetric(null);
+
+DefaultHttpSocketMetric socketMetric = new DefaultHttpSocketMetric(null);
+try {
+ DefaultHttpClientMetrics clientMetrics = (DefaultHttpClientMetrics)
((HttpClientImpl) client).getMetrics();
Review comment:
seems clientMetrics is this.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645721#comment-16645721 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19458537) Coverage increased (+0.04%) to 86.254% when pulling **1dd35ee1f4b50195f098cf6695fd496e3968fb66 on heyile:metrics_http2** into **c3f1d662cd5b41b17b58e553a45b144c8101e8ad on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645697#comment-16645697
]
ASF GitHub Bot commented on SCB-837:
heyile commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224271531
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -77,8 +83,13 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+//only http1.1 will invoke this method, just make a check
+if (endpointMetric != null) {
Review comment:
ok, I see
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16645332#comment-16645332
]
ASF GitHub Bot commented on SCB-837:
wujimin commented on a change in pull request #947: [SCB-837] make http2
production ready
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224179606
##
File path:
foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/metrics/DefaultHttpClientMetrics.java
##
@@ -77,8 +83,13 @@ public void dequeueRequest(DefaultClientEndpointMetric
endpointMetric, Object ta
@Override
public void endpointConnected(DefaultClientEndpointMetric endpointMetric,
DefaultHttpSocketMetric socketMetric) {
-socketMetric.setEndpointMetric(endpointMetric);
-endpointMetric.onConnect();
+//only http1.1 will invoke this method, just make a check
+if (endpointMetric != null) {
Review comment:
it's vertx bug: https://github.com/eclipse-vertx/vert.x/issues/2660
please add comments, and remove the hack code after vertx fix the bug.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client
> performance is so bad.
>
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent
> stream count
>
> we must perform a performance test, that make sure got a good result, and
> then set the setting to be our default setting.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16644995#comment-16644995 ] ASF GitHub Bot commented on SCB-837: coveralls edited a comment on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19446905) Coverage increased (+0.02%) to 86.238% when pulling **9f011c965526a369db7183cb1be3c548593ce9fc on heyile:metrics_http2** into **c3f1d662cd5b41b17b58e553a45b144c8101e8ad on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16644979#comment-16644979 ] ASF GitHub Bot commented on SCB-837: coveralls commented on issue #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957 [](https://coveralls.io/builds/19446733) Coverage increased (+0.02%) to 86.238% when pulling **9f011c965526a369db7183cb1be3c548593ce9fc on heyile:metrics_http2** into **c3f1d662cd5b41b17b58e553a45b144c8101e8ad on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-837) make http2 production ready
[ https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16644950#comment-16644950 ] ASF GitHub Bot commented on SCB-837: heyile opened a new pull request #947: [SCB-837] make http2 production ready URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/947 Follow this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > make http2 production ready > --- > > Key: SCB-837 > URL: https://issues.apache.org/jira/browse/SCB-837 > Project: Apache ServiceComb > Issue Type: Improvement > Components: Java-Chassis >Reporter: wujimin >Assignee: 何一乐 >Priority: Major > Fix For: java-chassis-1.1.0 > > > currenty, http2 client use all http1.1 settings, that cause http2 client > performance is so bad. > > we need to conside http2 client settings at least: > 1.concurrent stream in one connection, default value is 3, we must make it > bigger > 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent > stream count > > we must perform a performance test, that make sure got a good result, and > then set the setting to be our default setting. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
