Flaky Test: GrpcServerTest#testGrpcExecutorPool

[zuston]

Code of Conduct

• I agree to follow this project's Code of Conduct

Search before asking

• I have searched in the issues and found no similar issues.

Describe the flaky test

Error:  Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 4.137 s <<< FAILURE! - in org.apache.uniffle.common.GrpcServerTest
Error:  testGrpcExecutorPool  Time elapsed: 4.054 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <0.0> but was: <1.0>
	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:70)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:65)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:885)
	at org.apache.uniffle.common.GrpcServerTest.testGrpcExecutorPool(GrpcServerTest.java:83)
	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 org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)

Actions URL

https://github.com/apache/incubator-uniffle/actions/runs/3625931180/jobs/6114463345

Parent issue

#1733

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[jerqi]

Is Flaky test a good first issue?

[zuston]

Emm... Good question.

[jerqi]

Emm... Good question.

I feel it's difficult to fix flaky test. WDYT?

[jerqi]

If it's easy to fix, we should fix the flaky test immediately.

[zuston]

Emm... Good question.

I feel it's difficult to fix flaky test. WDYT?

Yes. Maybe if we have the clear fix solution or small easy features, it could be mark it as good first issue.

[zhengchenyu]

This problem still exists (https://github.com/apache/incubator-uniffle/actions/runs/5518411958/jobs/10062325314?pr=982).

The testGrpcExecutorPool has two state, then verify the metrics.
First state : assert the metrics when two thread has already run, but the third thread is not running.
Second state: assert the metrics when all thread is done.

I think the problem is that we can't make sure to enter first state. Because the three thread only sleep 200ms, in this patch sleep 120ms to enter the first state, the mistake is only 80ms, it is too short.
In my laptop, I change 120ms to 190ms, the probability of reproduce this problem will be increased.

So I think two way to solve this problem:
(a) increased thread sleep time from 200ms to 1000ms
(b) do not verify the first state.

@jerqi @zuston how about you?

[jerqi]

This problem still exists (https://github.com/apache/incubator-uniffle/actions/runs/5518411958/jobs/10062325314?pr=982).

The testGrpcExecutorPool has two state, then verify the metrics. First state : assert the metrics when two thread has already run, but the third thread is not running. Second state: assert the metrics when all thread is done.

I think the problem is that we can't make sure to enter first state. Because the three thread only sleep 200ms, in this patch sleep 120ms to enter the first state, the mistake is only 80ms, it is too short. In my laptop, I change 120ms to 190ms, the probability of reproduce this problem will be increased.

So I think two way to solve this problem: (a) increased thread sleep time from 200ms to 1000ms (b) do not verify the first state.

@jerqi @zuston how about you?

I prefer solution a.

[jerqi]

This problem still exists (https://github.com/apache/incubator-uniffle/actions/runs/5518411958/jobs/10062325314?pr=982).

The testGrpcExecutorPool has two state, then verify the metrics. First state : assert the metrics when two thread has already run, but the third thread is not running. Second state: assert the metrics when all thread is done.

I think the problem is that we can't make sure to enter first state. Because the three thread only sleep 200ms, in this patch sleep 120ms to enter the first state, the mistake is only 80ms, it is too short. In my laptop, I change 120ms to 190ms, the probability of reproduce this problem will be increased.

So I think two way to solve this problem: (a) increased thread sleep time from 200ms to 1000ms (b) do not verify the first state.

@jerqi @zuston how about you?

@zhengchenyu There is a fix. #1023

[zhengchenyu]

@jerqi OK, #1023 is a great work indeed. Wait the condition is more reliable than reset the sleep time.