leixm commented on code in PR #3756:
URL: https://github.com/apache/celeborn/pull/3756#discussion_r3620725728


##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/congestcontrol/CongestionController.java:
##########
@@ -265,20 +285,22 @@ protected void checkCongestion() {
     try {
       long pendingConsume = getTotalPendingBytes();
       long workerProduceSpeed = producedBufferStatusHub.avgBytesPerSec();
-      if (pendingConsume < workerTrafficQuota.diskBufferLowWatermark()
-          && workerProduceSpeed < 
workerTrafficQuota.workerProduceSpeedLowWatermark()) {
-        if (overHighWatermark.compareAndSet(true, false)) {
-          logger.info(
-              "Pending consume and produce speed is lower than low watermark, 
exit congestion control");
+      if (overHighWatermark.get()) {
+        long activePendingBytes = getActivePendingBytes();

Review Comment:
   Suppose that direct memory spikes and exceeds the high watermark within a 
very short period of time. Due to the latency of pinned memory, the current 
value might still be the value before the spike. In this case, we might 
mistakenly set overHighWatermark to false. Recommendation: We should refresh 
cachedActivePendingBytes when setting overHighWatermark to true.



-- 
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]

Reply via email to