[#1608] refactor: Reuse ShuffleManagerClient in ShuffleReader and ShuffleWriter

[xumanbu]

What changes were proposed in this pull request?

(Please outline the changes and how this PR fixes the issue.)

Why are the changes needed?

1. The createShuffleManagerClient function is defined in both RssShuffleWriter and RssFetchFailedIterator.
2. The ShuffleManagerClient is created in RssShuffleManager when shuffleManagerRpcServiceEnabled is true. We can reuse the client in RssShuffleReader and RssShuffleWriter.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing UTs.

[github-actions[bot]]

Test Results

 2 747 files  + 15   2 747 suites  +15   9h 48m 23s :stopwatch: + 4h 11m 44s    978 tests +  9     977 :white_check_mark: +  9   1 :zzz: ±0  0 :x: ±0  12 279 runs  +135  12 264 :white_check_mark: +135  15 :zzz: ±0  0 :x: ±0 

Results for commit 02ca92ce. ± Comparison against base commit 48d23ce4.

:recycle: This comment has been updated with latest results.

[xumanbu]

@zuston @advancedxy PTAL

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 41.66667% with 49 lines in your changes missing coverage. Please review.

Project coverage is 52.62%. Comparing base (f8e4329) to head (0bb986c). Report is 41 commits behind head on master.

Files	Patch %	Lines
...e/spark/shuffle/reader/RssFetchFailedIterator.java	0.00%	14 Missing :warning:
...uniffle/shuffle/manager/RssShuffleManagerBase.java	0.00%	11 Missing :warning:
...org/apache/spark/shuffle/RssSparkShuffleUtils.java	0.00%	9 Missing :warning:
.../shuffle/BlockIdSelfManagedShuffleWriteClient.java	0.00%	5 Missing :warning:
...le/client/factory/ShuffleManagerClientFactory.java	0.00%	3 Missing :warning:
...fle/client/impl/grpc/ShuffleManagerGrpcClient.java	50.00%	3 Missing :warning:
...pache/uniffle/shuffle/RssShuffleClientFactory.java	0.00%	2 Missing :warning:
...uniffle/common/util/ExpiringCloseableSupplier.java	94.11%	0 Missing and 2 partials :warning:

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #1838       +/-   ##
=============================================
+ Coverage          0   52.62%   +52.62%     
- Complexity        0     2561     +2561     
=============================================
  Files             0      415      +415     
  Lines             0    18784    +18784     
  Branches          0     1703     +1703     
=============================================
+ Hits              0     9885     +9885     
- Misses            0     8288     +8288     
- Partials          0      611      +611     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[rickyma]

If we are trying to refactor ShuffleManagerClient, could you also consider to fix https://github.com/apache/incubator-uniffle/issues/1774 as well? @xumanbu

rssConf in ShuffleManagerGrpcClient is hard-coded.

[xumanbu]

If we are trying to refactor ShuffleManagerClient, could you also consider to fix #1774 as well? @xumanbu

rssConf in ShuffleManagerGrpcClient is hard-coded.

ok.

[xumanbu]

@xumanbu thanks for your hard work. I think it's in good shape and the right direction.

The biggest issue is how to handle close from external caller for ExpiringCloseableSupplier.

@advancedxy Very thank you for helping me review the code and for your valuable suggestions. I'm still a bit confused by some comments, I need think about more and optimize the code.

[advancedxy]

@xumanbu thanks for your hard work. I think it's in good shape and the right direction. The biggest issue is how to handle close from external caller for ExpiringCloseableSupplier.

@advancedxy Very thank you for helping me review the code and for your valuable suggestions. I'm still a bit confused by some comments, I need think about more and optimize the code.

You are welcome. And if you have any further questions or problems, feel free to ask here.

[advancedxy]

@xumanbu other parts lgtm now. The ExpiringCloseableSupplier may still needs some refinements. Please take a look at the PR I raised in your repo.

also cc @zuston to take a look if you are interested.

[advancedxy]

Merging this since there's no objections. Others are welcome to file additional comments if any and can be addressed in a follow-up PR.

[advancedxy]

BTW, thanks your contribution @xumanbu.