You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "xianjingfeng (via GitHub)" <gi...@apache.org> on 2023/03/01 13:50:16 UTC
[GitHub] [incubator-uniffle] xianjingfeng opened a new pull request, #673: [#667] test: fix flaky test QuorumTest#case7
xianjingfeng opened a new pull request, #673:
URL: https://github.com/apache/incubator-uniffle/pull/673
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#233] fix: check null before access result in xxx"
- "[MINOR] refactor: fix typo in variable name"
- "[MINOR] docs: fix typo in README"
- "[#255] test: fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. Contributor guidelines:
https://github.com/apache/incubator-uniffle/blob/master/CONTRIBUTING.md
3. If the PR is unfinished, please mark this PR as draft.
-->
### What changes were proposed in this pull request?
Fix flaky test QuorumTest#case7
### Why are the changes needed?
200ms is too short and it is easy to timeout.
Fix: #667
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
No need.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1454353598
> 100x QuorumTest#case7 before this PR: https://github.com/kaijchen/incubator-uniffle/actions/runs/4329005416
You can search for `[INFO] T E S T S` in the log.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1461386553
> Is this ready for merging?
Not yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1478858097
> I'm not sure about this fix, looking at the CI of last 10 commits on master. There are only 2 failures of "Unexpected flush process" in ShuffleFlushManagerTest.
>
> ```
> in org.apache.uniffle.server.ShuffleFlushManagerTest
> Error: writeTest Time elapsed: 51.105 s <<< FAILURE!
> org.opentest4j.AssertionFailedError: Unexpected flush process
> ```
I think this problem has nothing to do with this PR. Because `ShuffleFlushManagerTest` is not in `integration-test` module, so i think they are independent of each other.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1475205419
## [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/673?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#673](https://codecov.io/gh/apache/incubator-uniffle/pull/673?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2402c0) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/c70d1543e7c9ca774e986026890763ce949a10e5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c70d154) will **increase** coverage by `2.13%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #673 +/- ##
============================================
+ Coverage 60.67% 62.80% +2.13%
- Complexity 1802 1894 +92
============================================
Files 216 224 +8
Lines 12458 11035 -1423
Branches 1052 1090 +38
============================================
- Hits 7559 6931 -628
+ Misses 4494 3727 -767
+ Partials 405 377 -28
```
[see 76 files with indirect coverage changes](https://codecov.io/gh/apache/incubator-uniffle/pull/673/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on a diff in pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1141371218
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
Review Comment:
It maybe the root cause of #483. @jerqi @zuston
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1454353134
> 100x QuorumTest#case7 before this PR: https://github.com/kaijchen/incubator-uniffle/actions/runs/4329005416
Seems it was good, maybe you should consider `mvn test -Dtest=QuorumTest`, to see if the tests are interferencing with each other.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on a diff in pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1141371462
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
// When the timeout of one server is recovered, the block sending should success
disableTimeout((MockedShuffleServer)shuffleServers.get(2));
report = shuffleWriteClientImpl.getShuffleResult("GRPC",
Sets.newHashSet(shuffleServerInfo0, shuffleServerInfo1, shuffleServerInfo2),
testAppId, 0, 0);
assertEquals(report, blockIdBitmap);
+
}
@Test
public void case4() throws Exception {
String testAppId = "case4";
+ /** We must wait until server1 finish start, because {@link #case3} will restart server1. */
+ await().timeout(10, TimeUnit.SECONDS).until(
+ () -> isPortAvailable(SHUFFLE_SERVER_PORT + 1));
+ Thread.sleep(500);
Review Comment:
If i remove this line, this ut case will fail sometimes. i don't find the root cause.
https://github.com/xianjingfeng/incubator-uniffle/actions/runs/4460910585/jobs/7834366795
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1475553833
Thanks @xianjingfeng for working on this issue.
Seems success rate of QuorumTest is good without any changes:
https://github.com/kaijchen/incubator-uniffle/actions/runs/4464172169
I'm not sure why, but maybe it's interfering with other tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1454346670
> > ### How was this patch tested?
> > No need.
>
> Can you test it in CI (repeat it for multiple times) to prove it's fixed? Example: [apache/ratis#827](https://github.com/apache/ratis/pull/827)
>
> Command for running a single test: `mvn test -Dtest=QuorumTest#case7`
I don't know how to test it repeatedly. The env `ITERATIONS` doesn't work. What is its principle?
https://github.com/xianjingfeng/incubator-uniffle/blob/cc62f9a7721bd56debe9266079656b5693fc12dd/.github/workflows/test.yml#L30
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1477197262
I'm not sure about this fix, looking at the CI of last 10 commits on master.
There are only 2 failures of "Unexpected flush process" in ShuffleFlushManagerTest.
```
in org.apache.uniffle.server.ShuffleFlushManagerTest
Error: writeTest Time elapsed: 51.105 s <<< FAILURE!
org.opentest4j.AssertionFailedError: Unexpected flush process
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on a diff in pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1144182340
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
// When the timeout of one server is recovered, the block sending should success
disableTimeout((MockedShuffleServer)shuffleServers.get(2));
report = shuffleWriteClientImpl.getShuffleResult("GRPC",
Sets.newHashSet(shuffleServerInfo0, shuffleServerInfo1, shuffleServerInfo2),
testAppId, 0, 0);
assertEquals(report, blockIdBitmap);
+
}
@Test
public void case4() throws Exception {
String testAppId = "case4";
+ /** We must wait until server1 finish start, because {@link #case3} will restart server1. */
+ await().timeout(10, TimeUnit.SECONDS).until(
+ () -> isPortAvailable(SHUFFLE_SERVER_PORT + 1));
+ Thread.sleep(500);
Review Comment:
I think I have solved this problem. https://github.com/apache/incubator-uniffle/pull/673/commits/3020eb05d91c71736272029f521d102777520183
https://github.com/xianjingfeng/incubator-uniffle/actions/runs/4485510017
By the way, i found that this method was added by you, I think you should be able to explain the underlying principle to me. @zuston
https://github.com/apache/incubator-uniffle/blob/5fd68764d6fc9e085c89f9ddabd62d99eea6163e/integration-test/common/src/test/java/org/apache/uniffle/test/MultiStorageFaultToleranceBase.java#L56
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1461369247
Is this ready for merging?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1450460743
> ### How was this patch tested?
> No need.
Can you test it in CI (repeat it for multiple times) to prove it's fixed?
Example: https://github.com/apache/ratis/pull/827
Command for repeating a test: `mvn test -Dtest=QuorumTest#case7`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1475270955
> > 100x QuorumTest#case7 before this PR: https://github.com/kaijchen/incubator-uniffle/actions/runs/4329005416
>
> Seems it was good, maybe you should consider `mvn test -Dtest=QuorumTest`, to see if the tests are interferencing with each other.
Done. https://github.com/xianjingfeng/incubator-uniffle/actions/runs/4460964902
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1478871740
> > I think this problem has nothing to do with this PR. Because `ShuffleFlushManagerTest` is not in `integration-test` module, so i think they are independent of each other.
>
> Yes, I was trying to say, this QuorumTest is not very flaky. So I'm not sure increasing timeout will help. Glad to see you have found the root cause. Cc @zuston PTAL.
There is another question that I can't understand. In the original logic, why does server1 stop in case3, and case4 can still run normally?
And i found other probloms in `QuorumTest`
1. Sometimes `getShuffleResult` fail is because of `DEADLINE_EXCEEDED`, and not because server1 stop
https://github.com/apache/incubator-uniffle/blob/7cb0b000cdf7260db3ac77f9d1742b8496c24cb2/integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java#L421-L429
2. `shuffleServers` is a static variable and elements inside will be modified. And this will affect other tests.
https://github.com/apache/incubator-uniffle/blob/d60d675d38c833b99b012a1f4c726a012ce93463/integration-test/common/src/test/java/org/apache/uniffle/test/IntegrationTestBase.java#L60-L61
In general, modifying the timeout period will help expose the problems in advance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#667][#502] test: fix flaky test QuorumTest [incubator-uniffle]
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng closed pull request #673: [#667][#502] test: fix flaky test QuorumTest
URL: https://github.com/apache/incubator-uniffle/pull/673
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1454351083
> I don't know how to test it repeatedly. The env `ITERATIONS` doesn't work. What is its principle?
`ITERATIONS` is for its `unit.sh`. Currently Uniffle doesn't have it.
Try this: https://github.com/kaijchen/incubator-uniffle/blob/repeat/.github/workflows/build.yml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1478860497
> I think this problem has nothing to do with this PR. Because `ShuffleFlushManagerTest` is not in `integration-test` module, so i think they are independent of each other.
Yes, I was trying to say, this QuorumTest is not very flaky. So I'm not sure increasing timeout will help.
Glad to see you have found the root cause. Cc @zuston PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on a diff in pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1141988602
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
// When the timeout of one server is recovered, the block sending should success
disableTimeout((MockedShuffleServer)shuffleServers.get(2));
report = shuffleWriteClientImpl.getShuffleResult("GRPC",
Sets.newHashSet(shuffleServerInfo0, shuffleServerInfo1, shuffleServerInfo2),
testAppId, 0, 0);
assertEquals(report, blockIdBitmap);
+
}
@Test
public void case4() throws Exception {
String testAppId = "case4";
+ /** We must wait until server1 finish start, because {@link #case3} will restart server1. */
+ await().timeout(10, TimeUnit.SECONDS).until(
+ () -> isPortAvailable(SHUFFLE_SERVER_PORT + 1));
+ Thread.sleep(500);
Review Comment:
I change it to sleep 1 second and run for 5 times. And it looks normal.
https://github.com/xianjingfeng/incubator-uniffle/actions/runs/4465348796
PS: some errors are caused by download dependency failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1475562948
> Thanks @xianjingfeng for working on this issue.
>
> Seems success rate of QuorumTest is good without any changes: https://github.com/kaijchen/incubator-uniffle/actions/runs/4464172169 I'm not sure why, but maybe it's interfering with other tests?
I guess it's caused by concurrent execution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on a diff in pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1141590460
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
Review Comment:
I think we should not use `static` here.
https://github.com/apache/incubator-uniffle/blob/d60d675d38c833b99b012a1f4c726a012ce93463/integration-test/common/src/test/java/org/apache/uniffle/test/IntegrationTestBase.java#L60-L61
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#issuecomment-1478872830
Cc @frankliee who is the original author.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on a diff in pull request #673: [#667][#502] test: fix flaky test QuorumTest
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1141581713
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
// When the timeout of one server is recovered, the block sending should success
disableTimeout((MockedShuffleServer)shuffleServers.get(2));
report = shuffleWriteClientImpl.getShuffleResult("GRPC",
Sets.newHashSet(shuffleServerInfo0, shuffleServerInfo1, shuffleServerInfo2),
testAppId, 0, 0);
assertEquals(report, blockIdBitmap);
+
}
@Test
public void case4() throws Exception {
String testAppId = "case4";
+ /** We must wait until server1 finish start, because {@link #case3} will restart server1. */
+ await().timeout(10, TimeUnit.SECONDS).until(
+ () -> isPortAvailable(SHUFFLE_SERVER_PORT + 1));
+ Thread.sleep(500);
Review Comment:
> If i remove this line, this ut case will fail sometimes. i don't find the root cause.
> https://github.com/xianjingfeng/incubator-uniffle/actions/runs/4460910585/jobs/7834366795
Seems it still fail sometimes.
https://github.com/kaijchen/incubator-uniffle/actions/runs/4464176186/jobs/7840089348
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on a diff in pull request #673: [#667] test: fix flaky test QuorumTest#case7
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on code in PR #673:
URL: https://github.com/apache/incubator-uniffle/pull/673#discussion_r1141371462
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -427,18 +431,25 @@ public void case3() throws Exception {
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Get shuffle result is failed"));
}
+ shuffleServers.set(1, createServer(1, tmpDir));
+ shuffleServers.get(1).start();
// When the timeout of one server is recovered, the block sending should success
disableTimeout((MockedShuffleServer)shuffleServers.get(2));
report = shuffleWriteClientImpl.getShuffleResult("GRPC",
Sets.newHashSet(shuffleServerInfo0, shuffleServerInfo1, shuffleServerInfo2),
testAppId, 0, 0);
assertEquals(report, blockIdBitmap);
+
}
@Test
public void case4() throws Exception {
String testAppId = "case4";
+ /** We must wait until server1 finish start, because {@link #case3} will restart server1. */
+ await().timeout(10, TimeUnit.SECONDS).until(
+ () -> isPortAvailable(SHUFFLE_SERVER_PORT + 1));
+ Thread.sleep(500);
Review Comment:
If i remove this line, this ut case will fail sometimes. i don't find the root cause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org