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 <junfan.zh...@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