[ISSUE-507] Fix Flaky Test: ShuffleBufferManagerTest#cacheShuffleDataTest

[xianjingfeng]

What changes were proposed in this pull request?

Fix Flaky Test: ShuffleBufferManagerTest#cacheShuffleDataTest.

Why are the changes needed?

It is flaky Test. Fix #507

I found logs as follows in org.apache.uniffle.server.buffer.ShuffleBufferManagerTest-output.txt.

2023-01-28 09:24:40,791 INFO  [expiredAppCleaner] server.ShuffleTaskManager (ShuffleTaskManager.java:checkResourceStatus(511)) - Detect expired appId[removeShuffleDataTest1] according to rss.server.app.expired.withoutHeartbeat

So i think the reason is that some threads have not exit after running ShuffleTaskManagerTest#removeShuffleDataWithHdfsTest.

Does this PR introduce any user-facing change?

No

How was this patch tested?

No need

[kaijchen]

LGTM, thanks @xianjingfeng for the fix.

[zuston]

It seems this bug is not fixed yet. https://github.com/apache/incubator-uniffle/actions/runs/4031027397/jobs/6930075442

[codecov-commenter]

Codecov Report

Merging #511 (51f248b) into master (0a6605b) will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #511      +/-   ##
============================================
+ Coverage     59.73%   59.75%   +0.01%     
- Complexity     1764     1765       +1     
============================================
  Files           205      205              
  Lines         11527    11527              
  Branches       1033     1033              
============================================
+ Hits           6886     6888       +2     
+ Misses         4234     4233       -1     
+ Partials        407      406       -1     

Impacted Files	Coverage Δ
...pache/hadoop/mapreduce/task/reduce/RssFetcher.java	92.30% <0.00%> (+1.53%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[xianjingfeng]

It seems this bug is not fixed yet. https://github.com/apache/incubator-uniffle/actions/runs/4031027397/jobs/6930075442

Updated

[advancedxy]

LGTM.

[advancedxy]

So i think the reason is that some threads have not exit after running ShuffleTaskManagerTest#removeShuffleDataWithHdfsTest.

This pr should fix the flaky test. However, I think this statement indicates that there might be other resource leaking problem in the test code as many test create a shuffle server but never stop the server. We should create a new issue to track this problem: close/stop shuffle server after use in each test case. @xianjingfeng or @zuston(since you wrote most of the related test code) could you help track and resolve this issue?

[zuston]

So i think the reason is that some threads have not exit after running ShuffleTaskManagerTest#removeShuffleDataWithHdfsTest.

This pr should fix the flaky test. However, I think this statement indicates that there might be other resource leaking problem in the test code as many test create a shuffle server but never stop the server. We should create a new issue to track this problem: close/stop shuffle server after use in each test case. @xianjingfeng or @zuston(since you wrote most of the related test code) could you help track and resolve this issue?

Make sense. By the way, let's raise a test improvement issue? Except above problems, the speed of CI could be improved, including removing the unused mini HDFS cluster setup for some test basic class and sharing the mini HDFS for all class to avoid being created multiple times.

[advancedxy]

So i think the reason is that some threads have not exit after running ShuffleTaskManagerTest#removeShuffleDataWithHdfsTest.

This pr should fix the flaky test. However, I think this statement indicates that there might be other resource leaking problem in the test code as many test create a shuffle server but never stop the server. We should create a new issue to track this problem: close/stop shuffle server after use in each test case. @xianjingfeng or @zuston(since you wrote most of the related test code) could you help track and resolve this issue?

Make sense. By the way, let's raise a test improvement issue? Except above problems, the speed of CI could be improved, including removing the unused mini HDFS cluster setup for some test basic class and sharing the mini HDFS for all class to avoid being created multiple times.

Sounds good to me. The average UT time is around 20mins, this is quite some time. Let's speed that up.

[advancedxy]

Merged this, thanks @xianjingfeng, @kaijchen and @zuston