advancedxy commented on code in PR #494:
URL: https://github.com/apache/incubator-uniffle/pull/494#discussion_r1072115369
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -322,16 +322,22 @@ public void updateCachedBlockIds(String appId, int
shuffleId, int partitionId, S
size += spb.getSize();
}
}
- shuffleTaskInfo.addPartitionDataSize(
+ long partitionSize = shuffleTaskInfo.addPartitionDataSize(
shuffleId,
partitionId,
size
);
+ if (shuffleBufferManager.isHugePartition(partitionSize)) {
Review Comment:
Did a overview of the current code. I think this's the better place to
update related metrics here.
Once shuffleBufferManager detects a huge partition, it cloud increase the
app_num_with_huge_partition metrics if the appId is not already added to
`hasSeenAppIdSet`.
`public void markHugePartition(int shuffleId, int partitionId) {`
could be change its signature to
`public boolean markHugePartition(int shuffleId, int partitionId) {`
which returns whether the current partition has already been marked or not.
The logging and metrics inc should be updated here.
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskInfo.java:
##########
@@ -124,4 +136,32 @@ public long getPartitionDataSize(int shuffleId, int
partitionId) {
return size;
}
+ public boolean hasHugePartition() {
+ return !hugePartitionTags.isEmpty();
+ }
+
+ public int getHugePartitionSize() {
+ return hugePartitionTags.values().stream().map(x -> x.size()).reduce((x,
y) -> x + y).orElse(0);
+ }
+
+ public void markHugePartition(int shuffleId, int partitionId) {
+ if (!existHugePartition) {
Review Comment:
To be honest, this is quite ugly..😂😇
##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -322,16 +322,22 @@ public void updateCachedBlockIds(String appId, int
shuffleId, int partitionId, S
size += spb.getSize();
}
}
- shuffleTaskInfo.addPartitionDataSize(
+ long partitionSize = shuffleTaskInfo.addPartitionDataSize(
shuffleId,
partitionId,
size
);
+ if (shuffleBufferManager.isHugePartition(partitionSize)) {
Review Comment:
Therefore, the appId in `ShuffleTaskInfo` could be removed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
