This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git
commit cb2be07d285cc83c92148b82a2a6ae29c8e2e3e3
Author: maheshrajus <mahesh.somalar...@huawei.com>
AuthorDate: Tue Jun 5 11:12:36 2018 +0530
Client Request Timeout support for operation/schema/service level:Review fix
---
.../core/transport/AbstractTransport.java | 108 +++++----------------
.../core/transport/TestAbstractTransport.java | 42 +++-----
.../src/main/resources/microservice.yaml | 14 ++-
.../transport/highway/HighwayClient.java | 4 +-
.../transport/highway/TestHighwayClient.java | 2 +-
.../rest/client/http/RestClientInvocation.java | 4 +-
6 files changed, 57 insertions(+), 117 deletions(-)
diff --git
a/core/src/main/java/org/apache/servicecomb/core/transport/AbstractTransport.java
b/core/src/main/java/org/apache/servicecomb/core/transport/AbstractTransport.java
index a02f14e..f02b971 100644
---
a/core/src/main/java/org/apache/servicecomb/core/transport/AbstractTransport.java
+++
b/core/src/main/java/org/apache/servicecomb/core/transport/AbstractTransport.java
@@ -23,16 +23,13 @@ import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.Map;
-import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import org.apache.http.client.utils.URLEncodedUtils;
import org.apache.http.message.BasicNameValuePair;
import org.apache.servicecomb.core.Const;
import org.apache.servicecomb.core.Endpoint;
-import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.core.Transport;
-import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
import org.apache.servicecomb.foundation.common.net.NetUtils;
import org.apache.servicecomb.foundation.common.net.URIEndpointObject;
@@ -41,7 +38,6 @@ import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.netflix.config.DynamicLongProperty;
import com.netflix.config.DynamicPropertyFactory;
import io.vertx.core.Vertx;
@@ -49,21 +45,17 @@ import io.vertx.core.Vertx;
public abstract class AbstractTransport implements Transport {
private static final Logger LOGGER =
LoggerFactory.getLogger(AbstractTransport.class);
- private static final String CONSUMER_REQUEST_TIMEOUT = "cse.request.timeout";
+ public static final String PROP_ROOT = "cse.request";
- // key is configuration parameter.
- private static Map<String, String> cfgCallback = new ConcurrentHashMapEx<>();
-
- // key is config paramter
- private static Map<String, AtomicLong> configCenterValue = new
ConcurrentHashMapEx<>();
-
- private static final long REQUEST_TIMEOUT_CFG_FAIL = -1;
+ public static final String PROP_TIMEOUT = ".timeout";
/*
* 用于参数传递：比如向RestServerVerticle传递endpoint地址。
*/
public static final String ENDPOINT_KEY = "cse.endpoint";
+ private static final long REQUEST_TIMEOUT_CFG_FAIL = -1;
+
private static final long DEFAULT_TIMEOUT_MILLIS = 30000;
// 所有transport使用同一个vertx实例，避免创建太多的线程
@@ -163,81 +155,33 @@ public abstract class AbstractTransport implements
Transport {
}
/**
- * Handles the request timeout configurations.
- *
- * @param invocation
- * invocation of request
- * @return configuration value
+ * Handles the request timeout configurations.
+ * @param operationName operation name
+ * @param schema schema id
+ * @param microservice micro service name
+ * @return configured value
*/
- public static long getReqTimeout(Invocation invocation) {
- long value = 0;
- String config;
-
- // get the config base on priority.
operationName-->schema-->service-->global
- String operationName = invocation.getOperationName();
- String schema = invocation.getSchemaId();
- String serviceName = invocation.getMicroserviceName();
-
- config = CONSUMER_REQUEST_TIMEOUT + "." + serviceName + "." + schema + "."
+ operationName;
- value = getConfigValue(config);
- if ((value != REQUEST_TIMEOUT_CFG_FAIL)) {
- return value;
- }
-
- config = CONSUMER_REQUEST_TIMEOUT + "." + serviceName + "." + schema;
- value = getConfigValue(config);
- if ((value != REQUEST_TIMEOUT_CFG_FAIL)) {
- return value;
- }
-
- config = CONSUMER_REQUEST_TIMEOUT + "." + serviceName;
- value = getConfigValue(config);
- if ((value != REQUEST_TIMEOUT_CFG_FAIL)) {
- return value;
- }
-
- value = getConfigValue(CONSUMER_REQUEST_TIMEOUT);
- if ((value != REQUEST_TIMEOUT_CFG_FAIL)) {
- return value;
- }
- return DEFAULT_TIMEOUT_MILLIS;
- }
-
- /**
- * Get the configuration value
- * @param config config parameter
- * @return long value
- */
- private static long getConfigParam(String config, long defaultValue) {
- DynamicLongProperty dynamicLongProperty =
DynamicPropertyFactory.getInstance().getLongProperty(config,
- defaultValue);
-
- cfgCallback.computeIfAbsent(config, key -> {
- dynamicLongProperty.addCallback(() -> {
- long newValue = dynamicLongProperty.get();
- String cfgName = dynamicLongProperty.getName();
-
- //store the value in config center map and check for next requests.
- configCenterValue.put(cfgName, new AtomicLong(newValue));
- LOGGER.info("{} changed to {}", cfgName, newValue);
- });
- return config;
- });
-
- return dynamicLongProperty.get();
+ public static long getReqTimeout(String operationName, String schema, String
microservice) {
+ return getLongProperty(DEFAULT_TIMEOUT_MILLIS,
+ PROP_ROOT + "." + microservice + "." + schema + "." + operationName +
PROP_TIMEOUT,
+ PROP_ROOT + "." + microservice + "." + schema + PROP_TIMEOUT,
+ PROP_ROOT + "." + microservice + PROP_TIMEOUT,
+ PROP_ROOT + PROP_TIMEOUT);
}
/**
- * Get the configuration value
- * @param config config parameter
- * @return long value
+ * Handles the request timeout configurations.
+ * @param defaultValue
+ * @param keys list of keys
+ * @return configured value
*/
- private static long getConfigValue(String config) {
- //first need to check in config center map which has high priority.
- if (configCenterValue.containsKey(config)) {
- return configCenterValue.get(config).get();
+ private static long getLongProperty(long defaultValue, String... keys) {
+ for (String key : keys) {
+ long property =
DynamicPropertyFactory.getInstance().getLongProperty(key,
REQUEST_TIMEOUT_CFG_FAIL).get();
+ if (property != REQUEST_TIMEOUT_CFG_FAIL) {
+ return property;
+ }
}
-
- return getConfigParam(config, REQUEST_TIMEOUT_CFG_FAIL);
+ return defaultValue;
}
}
diff --git
a/core/src/test/java/org/apache/servicecomb/core/transport/TestAbstractTransport.java
b/core/src/test/java/org/apache/servicecomb/core/transport/TestAbstractTransport.java
index baf752b..f4e6726 100644
---
a/core/src/test/java/org/apache/servicecomb/core/transport/TestAbstractTransport.java
+++
b/core/src/test/java/org/apache/servicecomb/core/transport/TestAbstractTransport.java
@@ -139,7 +139,7 @@ public class TestAbstractTransport {
transport.setListenAddressWithoutSchema(null);
Assert.assertNull(transport.getEndpoint().getEndpoint());
Assert.assertNull(transport.parseAddress(null));
- Assert.assertEquals(30000,
AbstractTransport.getReqTimeout(Mockito.mock(Invocation.class)));
+ Assert.assertEquals(30000, AbstractTransport.getReqTimeout("sayHi",
"hello", "test"));
}
@Test(expected = NumberFormatException.class)
@@ -154,14 +154,10 @@ public class TestAbstractTransport {
*/
@Test
public void testRequestCfgService() throws Exception {
- System.setProperty("cse.request.timeout.hello1", "3000");
- Invocation invocation = Mockito.mock(Invocation.class);
- Mockito.when(invocation.getOperationName()).thenReturn("sayHello1");
- Mockito.when(invocation.getSchemaId()).thenReturn("sayHelloSchema1");
- Mockito.when(invocation.getMicroserviceName()).thenReturn("hello1");
+ System.setProperty("cse.request.hello1.timeout", "3000");
//check for service level timeout value
- Assert.assertEquals(3000, AbstractTransport.getReqTimeout(invocation));
- System.getProperties().remove("cse.request.timeout.hello1");
+ Assert.assertEquals(3000, AbstractTransport.getReqTimeout("sayHello1",
"sayHelloSchema1", "hello1"));
+ System.getProperties().remove("cse.request.hello1.timeout");
}
/**
@@ -169,14 +165,10 @@ public class TestAbstractTransport {
*/
@Test
public void testRequestCfgSchema() throws Exception {
- System.setProperty("cse.request.timeout.hello2.sayHelloSchema2", "2000");
- Invocation invocation = Mockito.mock(Invocation.class);
- Mockito.when(invocation.getOperationName()).thenReturn("sayHello2");
- Mockito.when(invocation.getSchemaId()).thenReturn("sayHelloSchema2");
- Mockito.when(invocation.getMicroserviceName()).thenReturn("hello2");
+ System.setProperty("cse.request.hello2.sayHelloSchema2.timeout", "2000");
- Assert.assertEquals(2000, AbstractTransport.getReqTimeout(invocation));
-
System.getProperties().remove("cse.request.timeout.hello2.sayHelloSchema2");
+ Assert.assertEquals(2000, AbstractTransport.getReqTimeout("sayHello2",
"sayHelloSchema2", "hello2"));
+
System.getProperties().remove("cse.request.hello2.sayHelloSchema2.timeout");
}
/**
@@ -184,14 +176,10 @@ public class TestAbstractTransport {
*/
@Test
public void testRequestCfgOperation() throws Exception {
- System.setProperty("cse.request.timeout.hello3.sayHelloSchema3.sayHello3",
"1000");
- Invocation invocation = Mockito.mock(Invocation.class);
- Mockito.when(invocation.getOperationName()).thenReturn("sayHello3");
- Mockito.when(invocation.getSchemaId()).thenReturn("sayHelloSchema3");
- Mockito.when(invocation.getMicroserviceName()).thenReturn("hello3");
+ System.setProperty("cse.request.hello3.sayHelloSchema3.sayHello3.timeout",
"1000");
- Assert.assertEquals(1000, AbstractTransport.getReqTimeout(invocation));
-
System.getProperties().remove("cse.request.timeout.hello3.sayHelloSchema3.sayHello3");
+ Assert.assertEquals(1000, AbstractTransport.getReqTimeout("sayHello3",
"sayHelloSchema3", "hello3"));
+
System.getProperties().remove("cse.request.hello3.sayHelloSchema3.sayHello3.timeout");
}
/**
@@ -199,16 +187,16 @@ public class TestAbstractTransport {
*/
@Test
public void testRequestTimeoutCfgEvent() {
- System.setProperty("cse.request.timeout.hello4.sayHelloSchema4.sayHello4",
"1000");
+ System.setProperty("cse.request.hello4.sayHelloSchema4.sayHello4.timeout",
"1000");
Invocation invocation = Mockito.mock(Invocation.class);
Mockito.when(invocation.getOperationName()).thenReturn("sayHello4");
Mockito.when(invocation.getSchemaId()).thenReturn("sayHelloSchema4");
Mockito.when(invocation.getMicroserviceName()).thenReturn("hello4");
- Assert.assertEquals(1000, AbstractTransport.getReqTimeout(invocation));
+ Assert.assertEquals(1000, AbstractTransport.getReqTimeout("sayHello4",
"sayHelloSchema4", "hello4"));
- updateProperty("cse.request.timeout.hello4.sayHelloSchema4.sayHello4",
2000);
+ updateProperty("cse.request.hello4.sayHelloSchema4.sayHello4.timeout",
2000);
- Assert.assertEquals(2000, AbstractTransport.getReqTimeout(invocation));
-
System.getProperties().remove("cse.request.timeout.hello4.sayHelloSchema4.sayHello4");
+ Assert.assertEquals(2000, AbstractTransport.getReqTimeout("sayHello4",
"sayHelloSchema4", "hello4"));
+
System.getProperties().remove("cse.request.hello4.sayHelloSchema4.sayHello4.timeout");
}
}
diff --git a/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml
b/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml
index 1fbcc79..35049a8 100644
--- a/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml
+++ b/demo/demo-jaxrs/jaxrs-client/src/main/resources/microservice.yaml
@@ -34,8 +34,12 @@ servicecomb:
retryHandler: mycustomhandler
cse:
request:
- timeout:
- jaxrs:
- clientreqtimeout:
- sayHello: 100
- add: 100
+ timeout: 5000
+ jaxrs:
+ timeout: 1000
+ clientreqtimeout:
+ timeout: 150
+ sayHello:
+ timeout: 100
+ add:
+ timeout: 100
diff --git
a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
index 4ce6d2c..5106f55 100644
---
a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
+++
b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayClient.java
@@ -88,7 +88,9 @@ public class HighwayClient {
//set the timeout based on priority. the priority is follows.
//high priotiry: 1) operational level 2)schema level 3) service level 4)
global level : low priotiry.
TcpClientConfig tcpClientConfig = tcpClient.getClientConfig();
-
tcpClientConfig.setRequestTimeoutMillis(AbstractTransport.getReqTimeout(invocation));
+
tcpClientConfig.setRequestTimeoutMillis(AbstractTransport.getReqTimeout(invocation.getOperationName(),
+ invocation.getSchemaId(),
+ invocation.getMicroserviceName()));
HighwayClientPackage clientPackage = new HighwayClientPackage(invocation,
operationProtobuf, tcpClient);
LOGGER.debug("Sending request by highway, qualifiedName={}, endpoint={}.",
diff --git
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java
b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java
index 16d06cc..a47b9a6 100644
---
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java
+++
b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayClient.java
@@ -83,7 +83,7 @@ public class TestHighwayClient {
@Test
public void testRequestTimeout() {
- Assert.assertEquals(AbstractTransport.getReqTimeout(invocation), 2000);
+ Assert.assertEquals(AbstractTransport.getReqTimeout("sayHi", "hello",
"test"), 2000);
}
@Test
diff --git
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
index dcfdb08..6d26979 100644
---
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
+++
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/RestClientInvocation.java
@@ -118,7 +118,9 @@ public class RestClientInvocation {
this.setCseContext();
//set the timeout based on priority. the priority is follows.
//high priotiry: 1) operational level 2)schema level 3) service level 4)
global level : low priotiry.
- clientRequest.setTimeout(AbstractTransport.getReqTimeout(invocation));
+
clientRequest.setTimeout(AbstractTransport.getReqTimeout(invocation.getOperationName(),
+ invocation.getSchemaId(),
+ invocation.getMicroserviceName()));
try {
restClientRequest.end();
} catch (Throwable e) {
--
To stop receiving notification emails like this one, please contact
liu...@apache.org.
