[#720][FOLLOW-UP] Correct the shuffle server id

[jerqi]

What changes were proposed in this pull request?

Assign the correct shuffle server id

Why are the changes needed?

#720 follow-up pr

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing UTs

[xumanbu]

I have an idea for ShuffleServerId, and new ShuffleFlushManager with Future<String> shuffleServerId

public class ShuffleServerIdCreator {

  private String ip;
  private Integer grpcPortConf;
  private Integer nettyPortConf;

  private final ExecutorService executor = Executors.newSingleThreadExecutor();
  private final SynchronousQueue<Integer> nettyPortFuture = new SynchronousQueue();

  public ShuffleServerIdCreator(String ip, Integer grpcPortConf, Integer nettyPortConf) {
    this.ip = ip;
    this.grpcPortConf = grpcPortConf;
    this.nettyPortConf = nettyPortConf;
  }

  public Future<String> create() {
    return executor.submit(() -> {
      if (nettyPortConf > 0) {
        return ip + "-" + grpcPortConf + "-" + nettyPortConf;
      } else if (nettyPortConf == 0) {
        return ip + "-" + grpcPortConf + "-" + nettyPortFuture.take();
      } else {
        return ip + "-" + grpcPortConf;
      }
    });
  }

  public void setNettyActualPort(int actualNettyPort) {
    nettyPortFuture.offer(actualNettyPort);
  }
}

[codecov-commenter]

Codecov Report

Merging #792 (c0992e1) into master (7476cea) will increase coverage by 1.46%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #792      +/-   ##
============================================
+ Coverage     58.08%   59.54%   +1.46%     
- Complexity     2068     2072       +4     
============================================
  Files           303      289      -14     
  Lines         14809    12845    -1964     
  Branches       1212     1212              
============================================
- Hits           8602     7649     -953     
+ Misses         5715     4768     -947     
+ Partials        492      428      -64     

Impacted Files	Coverage Δ
...a/org/apache/uniffle/server/ShuffleServerConf.java	99.45% <ø> (ø)
...org/apache/uniffle/server/ShuffleFlushManager.java	84.53% <100.00%> (+0.08%)	⬆️
.../java/org/apache/uniffle/server/ShuffleServer.java	69.40% <100.00%> (+3.35%)	⬆️

... and 16 files with indirect coverage changes

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

[jerqi]

I have an idea for ShuffleServerId, and new ShuffleFlushManager with Future<String> shuffleServerId

public class ShuffleServerIdCreator {

  private String ip;
  private Integer grpcPortConf;
  private Integer nettyPortConf;

  private final ExecutorService executor = Executors.newSingleThreadExecutor();
  private final SynchronousQueue<Integer> nettyPortFuture = new SynchronousQueue();

  public ShuffleServerIdCreator(String ip, Integer grpcPortConf, Integer nettyPortConf) {
    this.ip = ip;
    this.grpcPortConf = grpcPortConf;
    this.nettyPortConf = nettyPortConf;
  }

  public Future<String> create() {
    return executor.submit(() -> {
      if (nettyPortConf > 0) {
        return ip + "-" + grpcPortConf + "-" + nettyPortConf;
      } else if (nettyPortConf == 0) {
        return ip + "-" + grpcPortConf + "-" + nettyPortFuture.take();
      } else {
        return ip + "-" + grpcPortConf;
      }
    });
  }

  public void setNettyActualPort(int actualNettyPort) {
    nettyPortFuture.offer(actualNettyPort);
  }
}

A little complex. I fix this issue in another way. You can take a look.

[advancedxy]

LGTM, except one minor comment