You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@uniffle.apache.org by ro...@apache.org on 2022/10/11 06:24:09 UTC
[incubator-uniffle] branch master updated: Fix flaky test of ClientConfManagerTest (#260)
This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new fc6d49b1 Fix flaky test of ClientConfManagerTest (#260)
fc6d49b1 is described below
commit fc6d49b163c408ab9d6f659bbea7b932d6ad524b
Author: jokercurry <84...@users.noreply.github.com>
AuthorDate: Tue Oct 11 14:24:05 2022 +0800
Fix flaky test of ClientConfManagerTest (#260)
### What changes were proposed in this pull request?
For [https://github.com/apache/incubator-uniffle/actions/runs/3213433147/jobs/5253100348](https://github.com/apache/incubator-uniffle/actions/runs/3213433147/jobs/5253100348)
```Error: dynamicRemoteByAppNumStrategyStorageTest Time elapsed: 11.065 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <hdfs://host2/path2> but was: <hdfs://host3/path3>
at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1141)
at org.apache.uniffle.coordinator.ClientConfManagerTest.dynamicRemoteByAppNumStrategyStorageTest(ClientConfManagerTest.java:200)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
at
```
This is because scheduling thread didn't update the remote path timely. For this test, we will trigger update logic manually.
### Why are the changes needed?
Fix flaky test.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
UT passed.
---
.../uniffle/coordinator/ClientConfManagerTest.java | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/coordinator/src/test/java/org/apache/uniffle/coordinator/ClientConfManagerTest.java b/coordinator/src/test/java/org/apache/uniffle/coordinator/ClientConfManagerTest.java
index 3dd7bf62..11952d59 100644
--- a/coordinator/src/test/java/org/apache/uniffle/coordinator/ClientConfManagerTest.java
+++ b/coordinator/src/test/java/org/apache/uniffle/coordinator/ClientConfManagerTest.java
@@ -170,14 +170,13 @@ public class ClientConfManagerTest {
CoordinatorConf conf = new CoordinatorConf();
conf.set(CoordinatorConf.COORDINATOR_DYNAMIC_CLIENT_CONF_UPDATE_INTERVAL_SEC, updateIntervalSec);
conf.set(CoordinatorConf.COORDINATOR_DYNAMIC_CLIENT_CONF_PATH, cfgFile.toURI().toString());
- conf.setLong(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_TIME, 200);
+ conf.setLong(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_TIME, 60000);
conf.setInteger(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_ACCESS_TIMES, 1);
conf.set(CoordinatorConf.COORDINATOR_DYNAMIC_CLIENT_CONF_ENABLED, true);
ApplicationManager applicationManager = new ApplicationManager(conf);
final ClientConfManager clientConfManager = new ClientConfManager(conf, new Configuration(), applicationManager);
- // the reason for sleep here is to ensure that threads can be scheduled normally, the same below
- Thread.sleep(1000);
+ applicationManager.getSelectStorageStrategy().detectStorage();
Set<String> expectedAvailablePath = Sets.newHashSet(remotePath1);
assertEquals(expectedAvailablePath, applicationManager.getAvailableRemoteStorageInfo().keySet());
RemoteStorageInfo remoteStorageInfo = applicationManager.pickRemoteStorage("testAppId1");
@@ -187,7 +186,7 @@ public class ClientConfManagerTest {
writeRemoteStorageConf(cfgFile, remotePath3);
expectedAvailablePath = Sets.newHashSet(remotePath3);
waitForUpdate(expectedAvailablePath, applicationManager);
- Thread.sleep(1000);
+ applicationManager.getSelectStorageStrategy().detectStorage();
remoteStorageInfo = applicationManager.pickRemoteStorage("testAppId2");
assertEquals(remotePath3, remoteStorageInfo.getPath());
@@ -195,7 +194,7 @@ public class ClientConfManagerTest {
writeRemoteStorageConf(cfgFile, remotePath2 + Constants.COMMA_SPLIT_CHAR + remotePath3, confItems);
expectedAvailablePath = Sets.newHashSet(remotePath2, remotePath3);
waitForUpdate(expectedAvailablePath, applicationManager);
- Thread.sleep(1000);
+ applicationManager.getSelectStorageStrategy().detectStorage();
remoteStorageInfo = applicationManager.pickRemoteStorage("testAppId3");
assertEquals(remotePath2, remoteStorageInfo.getPath());
assertEquals(2, remoteStorageInfo.getConfItems().size());
@@ -235,7 +234,7 @@ public class ClientConfManagerTest {
conf.set(CoordinatorConf.COORDINATOR_DYNAMIC_CLIENT_CONF_UPDATE_INTERVAL_SEC, updateIntervalSec);
conf.set(CoordinatorConf.COORDINATOR_DYNAMIC_CLIENT_CONF_PATH, cfgFile.toURI().toString());
conf.set(CoordinatorConf.COORDINATOR_DYNAMIC_CLIENT_CONF_ENABLED, true);
- conf.setLong(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_TIME, 200);
+ conf.setLong(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_TIME, 60000);
conf.setInteger(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SCHEDULE_ACCESS_TIMES, 1);
conf.set(CoordinatorConf.COORDINATOR_REMOTE_STORAGE_SELECT_STRATEGY, IO_SAMPLE);
@@ -247,7 +246,7 @@ public class ClientConfManagerTest {
final ClientConfManager clientConfManager = new ClientConfManager(conf, new Configuration(), applicationManager);
// the reason for sleep here is to ensure that threads can be scheduled normally, the same below
- Thread.sleep(1000);
+ applicationManager.getSelectStorageStrategy().detectStorage();
Set<String> expectedAvailablePath = Sets.newHashSet(remotePath1);
assertEquals(expectedAvailablePath, applicationManager.getAvailableRemoteStorageInfo().keySet());
selectStorageStrategy.sortPathByRankValue(remotePath1, testPath, System.currentTimeMillis());
@@ -260,7 +259,7 @@ public class ClientConfManagerTest {
waitForUpdate(expectedAvailablePath, applicationManager);
selectStorageStrategy.sortPathByRankValue(remotePath3, testPath, System.currentTimeMillis());
- Thread.sleep(1000);
+ applicationManager.getSelectStorageStrategy().detectStorage();
remoteStorageInfo = applicationManager.pickRemoteStorage("testAppId2");
assertEquals(remotePath3, remoteStorageInfo.getPath());
@@ -271,7 +270,7 @@ public class ClientConfManagerTest {
waitForUpdate(expectedAvailablePath, applicationManager);
selectStorageStrategy.sortPathByRankValue(remotePath2, testPath, current);
selectStorageStrategy.sortPathByRankValue(remotePath3, testPath, current);
- Thread.sleep(1000);
+ applicationManager.getSelectStorageStrategy().detectStorage();
remoteStorageInfo = applicationManager.pickRemoteStorage("testAppId3");
assertEquals(remotePath2, remoteStorageInfo.getPath());
assertEquals(2, remoteStorageInfo.getConfItems().size());