RexXiong commented on code in PR #2456:
URL: https://github.com/apache/celeborn/pull/2456#discussion_r1579223390
##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -606,14 +606,29 @@ private ConcurrentHashMap<Integer, PartitionLocation>
registerShuffleInternal(
StatusCode respStatus = Utils.toStatusCode(response.getStatus());
if (StatusCode.SUCCESS.equals(respStatus)) {
ConcurrentHashMap<Integer, PartitionLocation> result =
JavaUtils.newConcurrentHashMap();
- for (int i = 0; i < response.getPartitionLocationsList().size();
i++) {
- PartitionLocation partitionLoc =
-
PbSerDeUtils.fromPbPartitionLocation(response.getPartitionLocationsList().get(i));
- pushExcludedWorkers.remove(partitionLoc.hostAndPushPort());
- if (partitionLoc.hasPeer()) {
-
pushExcludedWorkers.remove(partitionLoc.getPeer().hostAndPushPort());
+ List<PbPartitionLocation> partitionLocationList =
response.getPartitionLocationsList();
Review Comment:
Newer client only need to support packed PartitionLocation.
##########
master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala:
##########
@@ -888,7 +892,10 @@ private[celeborn] class Master(
if (authEnabled) {
pushApplicationMetaToWorkers(requestSlots, slots)
}
- context.reply(RequestSlotsResponse(StatusCode.SUCCESS,
slots.asInstanceOf[WorkerResource]))
+ context.reply(RequestSlotsResponse(
Review Comment:
We can introduce a flag to indicate whether the client requires a packed
partition. This allows the master to determine the appropriate response instead
of returning both options.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
