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/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new cfecef012 [SCB-2664]fix findByContext may cause out of memory if not
used in a … (#3268)
cfecef012 is described below
commit cfecef0129d28796907e750a903493bd01235af8
Author: liubao68 <[email protected]>
AuthorDate: Mon Aug 8 20:53:06 2022 +0800
[SCB-2664]fix findByContext may cause out of memory if not used in a …
(#3268)
---
.../jaxrs/client/TestCodeFirstJaxrsReactive.java | 56 ++++++++++++++++++++++
.../foundation/vertx/client/ClientPoolManager.java | 8 ++--
.../foundation/vertx/client/http/HttpClients.java | 29 ++---------
.../vertx/server/TcpServerConnection.java | 4 +-
.../vertx/client/TestClientPoolManager.java | 16 +++----
5 files changed, 73 insertions(+), 40 deletions(-)
diff --git
a/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/TestCodeFirstJaxrsReactive.java
b/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/TestCodeFirstJaxrsReactive.java
new file mode 100644
index 000000000..2fdaffe3e
--- /dev/null
+++
b/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/TestCodeFirstJaxrsReactive.java
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.demo.jaxrs.client;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.servicecomb.demo.CategorizedTestCase;
+import org.apache.servicecomb.demo.TestMgr;
+import org.apache.servicecomb.provider.pojo.RpcReference;
+import org.springframework.stereotype.Component;
+
+@Component
+public class TestCodeFirstJaxrsReactive implements CategorizedTestCase {
+ interface AddOperation {
+ CompletableFuture<Integer> add(int a, int b);
+ }
+
+ @RpcReference(microserviceName = "jaxrs", schemaId = "codeFirst")
+ AddOperation addOperation;
+
+ @Override
+ public void testAllTransport() throws Exception {
+ final int count = 10;
+ CountDownLatch latch = new CountDownLatch(count);
+ AtomicInteger result = new AtomicInteger(0);
+
+ for (int i = 0; i < count; i++) {
+ new Thread(() -> addOperation.add(1, 2)
+ .whenComplete((r, e) -> addOperation.add(r, r).whenComplete((r1, e1)
-> {
+ result.addAndGet(r1);
+ latch.countDown();
+ }))).start();
+ }
+
+ latch.await(3, TimeUnit.SECONDS);
+ TestMgr.check(count * 6, result.get());
+ }
+}
diff --git
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/ClientPoolManager.java
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/ClientPoolManager.java
index dd99b006a..900f994c1 100644
---
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/ClientPoolManager.java
+++
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/ClientPoolManager.java
@@ -95,9 +95,9 @@ public class ClientPoolManager<CLIENT_POOL> {
return clientPool;
}
- // this will make "client.thread-count" bigger than which in
microservice.yaml
- // maybe it's better to remove "client.thread-count", just use
"rest/highway.thread-count"
- return createClientPool(currentContext);
+ // Maybe executed in a call back of a reactive call.
+ // The Context is created in a non-event thread and passed to the event
loop
+ // thread by vert.x.
}
// not in correct context:
@@ -120,7 +120,7 @@ public class ClientPoolManager<CLIENT_POOL> {
}
private void assertPoolsInitialized() {
- if (pools.size() == 0) {
+ if (pools.isEmpty()) {
throw new IllegalStateException("client pool not initialized
successfully when making calls."
+ "Please check if system boot up is ready or some errors happened
when startup.");
}
diff --git
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClients.java
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClients.java
index d763b4db0..06085446c 100644
---
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClients.java
+++
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClients.java
@@ -30,8 +30,6 @@ import
org.apache.servicecomb.foundation.vertx.client.ClientVerticle;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.annotations.VisibleForTesting;
-
import io.vertx.core.Context;
import io.vertx.core.DeploymentOptions;
import io.vertx.core.Vertx;
@@ -60,17 +58,6 @@ public class HttpClients {
});
}
- @VisibleForTesting
- public static void mockClientPoolManager(String name,
ClientPoolManager<HttpClientWithContext> clientPool) {
- httpClients.put(name, clientPool);
- }
-
- /* used for configurations module: these module must be boot before other
HttpClients is initialized. so can
- * not load by SPI, must add manually */
- public static void addNewClientPoolManager(HttpClientOptionsSPI option) {
- httpClients.put(option.clientName(), createClientPoolManager(option));
- }
-
/* destroy at shutdown. */
public static void destroy() {
httpClients.clear();
@@ -84,7 +71,7 @@ public class HttpClients {
new
HttpClientPoolFactory(HttpClientOptionsSPI.createHttpClientOptions(option)));
DeploymentOptions deployOptions =
VertxUtils.createClientDeployOptions(clientPoolManager,
- option.getInstanceCount())
+ option.getInstanceCount())
.setWorker(option.isWorker())
.setWorkerPoolName(option.getWorkerPoolName())
.setWorkerPoolSize(option.getWorkerPoolSize());
@@ -117,12 +104,7 @@ public class HttpClients {
* @return the deployed instance name
*/
public static HttpClientWithContext getClient(String clientName) {
- ClientPoolManager<HttpClientWithContext> poolManager =
httpClients.get(clientName);
- if (poolManager == null) {
- LOGGER.error("client name [{}] not exists, should only happen in
tests.", clientName);
- return null;
- }
- return poolManager.findThreadBindClientPool();
+ return getClient(clientName, true);
}
/**
@@ -132,12 +114,7 @@ public class HttpClients {
* @return the deployed instance name
*/
public static HttpClientWithContext getClient(String clientName, boolean
sync) {
- ClientPoolManager<HttpClientWithContext> poolManager =
httpClients.get(clientName);
- if (poolManager == null) {
- LOGGER.error("client name [{}] not exists, should only happen in
tests.", clientName);
- return null;
- }
- return poolManager.findClientPool(sync);
+ return getClient(clientName, sync, null);
}
/**
diff --git
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/server/TcpServerConnection.java
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/server/TcpServerConnection.java
index 409b9eea3..dae3d4221 100644
---
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/server/TcpServerConnection.java
+++
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/server/TcpServerConnection.java
@@ -36,11 +36,11 @@ public class TcpServerConnection extends TcpConnection {
LOGGER.info("connect from {}, in thread {}",
remoteAddress,
Thread.currentThread().getName());
- netSocket.exceptionHandler(e -> LOGGER.error("disconected from {}, in
thread {}, cause {}",
+ netSocket.exceptionHandler(e -> LOGGER.error("disconnected from {}, in
thread {}, cause {}",
remoteAddress,
Thread.currentThread().getName(),
e.getMessage()));
- netSocket.closeHandler(Void -> LOGGER.error("disconected from {}, in
thread {}",
+ netSocket.closeHandler(Void -> LOGGER.info("disconnected from {}, in
thread {}",
remoteAddress,
Thread.currentThread().getName()));
diff --git
a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/client/TestClientPoolManager.java
b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/client/TestClientPoolManager.java
index fea328340..c42c3f336 100644
---
a/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/client/TestClientPoolManager.java
+++
b/foundations/foundation-vertx/src/test/java/org/apache/servicecomb/foundation/vertx/client/TestClientPoolManager.java
@@ -26,6 +26,7 @@ import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
import io.vertx.core.Context;
import io.vertx.core.Vertx;
@@ -35,7 +36,6 @@ import mockit.Expectations;
import mockit.Mock;
import mockit.MockUp;
import mockit.Mocked;
-import org.junit.jupiter.api.Assertions;
public class TestClientPoolManager {
@Mocked
@@ -143,22 +143,22 @@ public class TestClientPoolManager {
@Test
public void findByContext_reactive() {
- HttpClientWithContext notMatchPool = new HttpClientWithContext(null, null);
- pools.add(notMatchPool);
+ HttpClientWithContext notMatchPool1 = new HttpClientWithContext(null,
null);
+ HttpClientWithContext notMatchPool2 = new HttpClientWithContext(null,
null);
+ pools.add(notMatchPool1);
+ pools.add(notMatchPool2);
new Expectations() {
{
- factory.createClientPool(context);
- result = new HttpClientWithContext(null, null);
Vertx.currentContext();
result = context;
}
};
+ context.put(id, notMatchPool2);
- HttpClientWithContext result = poolMgr.findByContext();
- Assertions.assertNotSame(notMatchPool, result);
+ Assertions.assertSame(notMatchPool2, poolMgr.findByContext());
// find again, get the same result
- Assertions.assertSame(result, poolMgr.findByContext());
+ Assertions.assertSame(notMatchPool2, poolMgr.findByContext());
}
@Test
