Flaky Test: ShuffleBufferManagerTest#bufferSizeTest

[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

[INFO] Running org.apache.uniffle.server.buffer.ShuffleBufferManagerTest
Error:  Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 3.473 s <<< FAILURE! - in org.apache.uniffle.server.buffer.ShuffleBufferManagerTest
Error:  bufferSizeTest  Time elapsed: 0.535 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <0> but was: <288>
	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:166)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:161)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:628)
	at org.apache.uniffle.server.buffer.ShuffleBufferManagerTest.bufferSizeTest(ShuffleBufferManagerTest.java:428)

Actions URL

https://github.com/apache/incubator-uniffle/actions/runs/3681854053/jobs/6228951081

Parent issue

#1733

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[xianjingfeng]

@zuston ShuffleBufferManagerTest#waitForFlush seems not applicable in this UT. because updateCommittedBlockIds is invoked before event.doCleanup(). I have no good idea to fix it. Do you have any good ideas?

incubator-uniffle/server/src/main/java/org/apache/uniffle/server/ShuffleFlushManager.java

Lines 211 to 233 in 33c4fe4

	if (writeSuccess) {
	updateCommittedBlockIds(event.getAppId(), event.getShuffleId(), blocks);
	ShuffleServerMetrics.incStorageSuccessCounter(storage.getStorageHost());
	break;
	} else {
	event.increaseRetryTimes();
	ShuffleServerMetrics.incStorageRetryCounter(storage.getStorageHost());
	}
	} catch (Throwable throwable) {
	// just log the error, don't throw the exception and stop the flush thread
	LOG.error("Exception happened when process flush shuffle data for {}", event, throwable);
	event.increaseRetryTimes();
	}
	}
	if (event.getRetryTimes() > retryMax) {
	LOG.error("Failed to write data for {} in {} times, shuffle data will be lost", event, retryMax);
	if (event.getUnderStorage() != null) {
	ShuffleServerMetrics.incStorageFailedCounter(event.getUnderStorage().getStorageHost());
	}
	}
	event.doCleanup();

[zuston]

Another failed tests: https://github.com/apache/incubator-uniffle/actions/runs/4242048090/jobs/7373054242

[jerqi]

@zuston ShuffleBufferManagerTest#waitForFlush seems not applicable in this UT. because updateCommittedBlockIds is invoked before event.doCleanup(). I have no good idea to fix it. Do you have any good ideas?

incubator-uniffle/server/src/main/java/org/apache/uniffle/server/ShuffleFlushManager.java

Lines 211 to 233 in 33c4fe4

	if (writeSuccess) {
	updateCommittedBlockIds(event.getAppId(), event.getShuffleId(), blocks);
	ShuffleServerMetrics.incStorageSuccessCounter(storage.getStorageHost());
	break;
	} else {
	event.increaseRetryTimes();
	ShuffleServerMetrics.incStorageRetryCounter(storage.getStorageHost());
	}
	} catch (Throwable throwable) {
	// just log the error, don't throw the exception and stop the flush thread
	LOG.error("Exception happened when process flush shuffle data for {}", event, throwable);
	event.increaseRetryTimes();
	}
	}
	if (event.getRetryTimes() > retryMax) {
	LOG.error("Failed to write data for {} in {} times, shuffle data will be lost", event, retryMax);
	if (event.getUnderStorage() != null) {
	ShuffleServerMetrics.incStorageFailedCounter(event.getUnderStorage().getStorageHost());
	}
	}
	event.doCleanup();

If this test don't have good solution, we can disable this test first to reduce the failure of CI. Or we can rewrite this test.

[zuston]

How about sleep a while to wait event.doCleanUp being invoked?