You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@uniffle.apache.org by js...@apache.org on 2022/07/01 06:57:54 UTC
[incubator-uniffle] 10/17: [MINOR] Close clusterManager resources (#202)
This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
commit 8b5f363fa296312042130b73c8dd8f5a15b5e0ae
Author: Junfan Zhang <ju...@outlook.com>
AuthorDate: Mon Jun 27 17:34:13 2022 +0800
[MINOR] Close clusterManager resources (#202)
### What changes were proposed in this pull request?
1. Change the method of shutdown to close
2. Close resources of clustermanager in test cases
### Why are the changes needed?
Close resources to reduce the resource occupying in test cases.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Test cases
---
.../java/com/tencent/rss/coordinator/ClusterManager.java | 5 ++---
.../java/com/tencent/rss/coordinator/CoordinatorServer.java | 2 +-
.../com/tencent/rss/coordinator/SimpleClusterManager.java | 10 ++++++++--
.../rss/coordinator/BasicAssignmentStrategyTest.java | 5 ++++-
.../coordinator/PartitionBalanceAssignmentStrategyTest.java | 4 +++-
.../tencent/rss/coordinator/SimpleClusterManagerTest.java | 13 +++++++++++--
.../test/java/com/tencent/rss/test/CoordinatorGrpcTest.java | 1 +
7 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/coordinator/src/main/java/com/tencent/rss/coordinator/ClusterManager.java b/coordinator/src/main/java/com/tencent/rss/coordinator/ClusterManager.java
index 4249a03..9f5915e 100644
--- a/coordinator/src/main/java/com/tencent/rss/coordinator/ClusterManager.java
+++ b/coordinator/src/main/java/com/tencent/rss/coordinator/ClusterManager.java
@@ -18,10 +18,11 @@
package com.tencent.rss.coordinator;
+import java.io.Closeable;
import java.util.List;
import java.util.Set;
-public interface ClusterManager {
+public interface ClusterManager extends Closeable {
/**
* Add a server to the cluster.
@@ -49,6 +50,4 @@ public interface ClusterManager {
List<ServerNode> list();
int getShuffleNodesMax();
-
- void shutdown();
}
diff --git a/coordinator/src/main/java/com/tencent/rss/coordinator/CoordinatorServer.java b/coordinator/src/main/java/com/tencent/rss/coordinator/CoordinatorServer.java
index 7ba7e1c..3b79221 100644
--- a/coordinator/src/main/java/com/tencent/rss/coordinator/CoordinatorServer.java
+++ b/coordinator/src/main/java/com/tencent/rss/coordinator/CoordinatorServer.java
@@ -94,7 +94,7 @@ public class CoordinatorServer {
jettyServer.stop();
}
if (clusterManager != null) {
- clusterManager.shutdown();
+ clusterManager.close();
}
if (accessManager != null) {
accessManager.close();
diff --git a/coordinator/src/main/java/com/tencent/rss/coordinator/SimpleClusterManager.java b/coordinator/src/main/java/com/tencent/rss/coordinator/SimpleClusterManager.java
index 10af74d..fcfd1dc 100644
--- a/coordinator/src/main/java/com/tencent/rss/coordinator/SimpleClusterManager.java
+++ b/coordinator/src/main/java/com/tencent/rss/coordinator/SimpleClusterManager.java
@@ -21,6 +21,7 @@ package com.tencent.rss.coordinator;
import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
+import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -186,8 +187,13 @@ public class SimpleClusterManager implements ClusterManager {
}
@Override
- public void shutdown() {
- scheduledExecutorService.shutdown();
+ public void close() throws IOException {
+ if (scheduledExecutorService != null) {
+ scheduledExecutorService.shutdown();
+ }
+ if (checkNodesExecutorService != null) {
+ checkNodesExecutorService.shutdown();
+ }
}
@Override
diff --git a/coordinator/src/test/java/com/tencent/rss/coordinator/BasicAssignmentStrategyTest.java b/coordinator/src/test/java/com/tencent/rss/coordinator/BasicAssignmentStrategyTest.java
index 97afabf..7a95d76 100644
--- a/coordinator/src/test/java/com/tencent/rss/coordinator/BasicAssignmentStrategyTest.java
+++ b/coordinator/src/test/java/com/tencent/rss/coordinator/BasicAssignmentStrategyTest.java
@@ -24,6 +24,8 @@ import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import com.google.common.collect.Sets;
import com.tencent.rss.common.PartitionRange;
+
+import java.io.IOException;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -49,8 +51,9 @@ public class BasicAssignmentStrategyTest {
}
@AfterEach
- public void tearDown() {
+ public void tearDown() throws IOException {
clusterManager.clear();
+ clusterManager.close();
}
@Test
diff --git a/coordinator/src/test/java/com/tencent/rss/coordinator/PartitionBalanceAssignmentStrategyTest.java b/coordinator/src/test/java/com/tencent/rss/coordinator/PartitionBalanceAssignmentStrategyTest.java
index 018aa62..9ca4146 100644
--- a/coordinator/src/test/java/com/tencent/rss/coordinator/PartitionBalanceAssignmentStrategyTest.java
+++ b/coordinator/src/test/java/com/tencent/rss/coordinator/PartitionBalanceAssignmentStrategyTest.java
@@ -18,6 +18,7 @@
package com.tencent.rss.coordinator;
+import java.io.IOException;
import java.util.Comparator;
import java.util.List;
import java.util.Set;
@@ -169,8 +170,9 @@ public class PartitionBalanceAssignmentStrategyTest {
}
@AfterEach
- public void tearDown() {
+ public void tearDown() throws IOException {
clusterManager.clear();
+ clusterManager.close();
}
void updateServerResource(List<Long> resources) {
diff --git a/coordinator/src/test/java/com/tencent/rss/coordinator/SimpleClusterManagerTest.java b/coordinator/src/test/java/com/tencent/rss/coordinator/SimpleClusterManagerTest.java
index bed9081..fc90d9e 100644
--- a/coordinator/src/test/java/com/tencent/rss/coordinator/SimpleClusterManagerTest.java
+++ b/coordinator/src/test/java/com/tencent/rss/coordinator/SimpleClusterManagerTest.java
@@ -20,6 +20,7 @@ package com.tencent.rss.coordinator;
import java.io.File;
import java.io.FileWriter;
+import java.io.IOException;
import java.io.PrintWriter;
import java.util.List;
import java.util.Map;
@@ -49,7 +50,7 @@ public class SimpleClusterManagerTest {
}
@Test
- public void getServerListTest() {
+ public void getServerListTest() throws IOException {
CoordinatorConf ssc = new CoordinatorConf();
ssc.setLong(CoordinatorConf.COORDINATOR_HEARTBEAT_TIMEOUT, 30 * 1000L);
SimpleClusterManager clusterManager = new SimpleClusterManager(ssc);
@@ -99,6 +100,8 @@ public class SimpleClusterManagerTest {
assertTrue(testTagNodes.contains(sn2));
assertTrue(testTagNodes.contains(sn3));
assertTrue(testTagNodes.contains(sn4));
+
+ clusterManager.close();
}
@Test
@@ -141,10 +144,12 @@ public class SimpleClusterManagerTest {
Thread.sleep(500);
serverNodes = clusterManager.getServerList(testTags);
assertEquals(0, serverNodes.size());
+
+ clusterManager.close();
}
@Test
- public void testGetCorrectServerNodesWhenOneNodeRemoved() {
+ public void testGetCorrectServerNodesWhenOneNodeRemoved() throws IOException {
CoordinatorConf ssc = new CoordinatorConf();
ssc.setLong(CoordinatorConf.COORDINATOR_HEARTBEAT_TIMEOUT, 30 * 1000L);
SimpleClusterManager clusterManager = new SimpleClusterManager(ssc);
@@ -167,6 +172,8 @@ public class SimpleClusterManagerTest {
List<ServerNode> serverList = clusterManager.getServerList(testTags);
Assertions.assertEquals(2, tagToNodes.get(testTags.iterator().next()).size());
Assertions.assertEquals(2, serverList.size());
+
+ clusterManager.close();
}
@Test
@@ -231,6 +238,8 @@ public class SimpleClusterManagerTest {
remainNodes.remove(node.getId());
}
assertEquals(0, remainNodes.size());
+
+ scm.close();
}
private void writeExcludeHosts(String path, Set<String> values) throws Exception {
diff --git a/integration-test/common/src/test/java/com/tencent/rss/test/CoordinatorGrpcTest.java b/integration-test/common/src/test/java/com/tencent/rss/test/CoordinatorGrpcTest.java
index cc11218..b713c00 100644
--- a/integration-test/common/src/test/java/com/tencent/rss/test/CoordinatorGrpcTest.java
+++ b/integration-test/common/src/test/java/com/tencent/rss/test/CoordinatorGrpcTest.java
@@ -237,6 +237,7 @@ public class CoordinatorGrpcTest extends CoordinatorTestBase {
shuffleServers.set(0, ss);
Thread.sleep(3000);
assertEquals(2, coordinators.get(0).getClusterManager().getNodesNum());
+ scm.close();
}
@Test