You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "TheodoreLx (via GitHub)" <gi...@apache.org> on 2025/11/14 03:40:32 UTC
[PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
TheodoreLx opened a new pull request, #3479:
URL: https://github.com/apache/celeborn/pull/3479
### What changes were proposed in this pull request?
Provides a configuration item that can copy the body buffer in pushdata to a newly requested buffer before writing on the worker, achieving 100% buffer internal space utilization, and ultimately significantly improving the overall utilization of NettyMemory.
### Why are the changes needed?
In the worker, Netty uses AdaptiveRecvByteBufAllocator to determine the buffer size to allocate in advance when reading data from the socket. However, in certain network environments, there can be a significant discrepancy between the buffer size predicted and allocated by AdaptiveRecvByteBufAllocator and the actual data size read from the socket. This can result in a large buffer being allocated but only a small amount of data being read, ultimately leading to very low overall memory utilization in the worker. A clear metric is that NettyMemory is large but DiskBuffer is very small. This means that the worker may receive a small amount of data but quickly enter the Pause state due to excessive NettyMemory usage.
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
cluster test
### Performance Test
<img width="1697" height="700" alt="image" src="https://github.com/user-attachments/assets/56495d08-6da7-4d43-8e8a-da87a33ccf90" />
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#issuecomment-3405223759
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "TheodoreLx (via GitHub)" <gi...@apache.org>.
TheodoreLx commented on code in PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#discussion_r2357450794
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala:
##########
@@ -1514,6 +1515,25 @@ class PushDataHandler(val workerSource: WorkerSource) extends BaseMessageHandler
hardSplitIndexes: Array[Int] = Array.empty[Int]): Unit = {
val length = fileWriters.length
val result = new Array[StatusCode](length)
+
+ var finalBody: ByteBuf = body
Review Comment:
In the worker, the buffer read from the socket by Netty is moved to the FileWriter's `flushBuffer`. When the `flushBuffer` reaches a certain size, a FlushTask is generated and queued for flushing in the Flusher's `workingQueues`. The current PR approach is to copy the buffer before moving it to the FileWriter to improve internal buffer space utilization. This has several advantages over copying before generating the FlushTask:
1. Generating a FlushTask after the flushBuffer reaches a certain size can take a long time, meaning that during this time, the buffer in the flushBuffer is still underutilized. If there are thousands of FileWriters on a worker, the data size of the flushBuffers across all FileWriters could reach over 1GB, but the memory used could be several times that amount. The earlier the copy is performed, the earlier the underutilized buffer can be released, improving memory utilization.
2. In my initial solution, I did copy before generating the FlushTask, but I observed high CPU consumption and sometimes very high `PrimaryPushDataTime`. Investigation revealed that if the received data is mostly PushMergedData, because PushMergedData may be composed of multiple batches, the buffer corresponding to each batch must be copied. Netty needs to iterate through the chunk queue to request a buffer. The more batches in a single PushMergedData, the greater the buffer traversal overhead. If the copy is performed before splitting the PushMergedData, only a single buffer request is required.
3. In the current PR, after copying the buffer, `FileWriter.write` is immediately called to add it to the `flushBuffer`. After the write method completes, the old buffer is released. Because the write method executes very quickly, there is almost no chance of the old and new buffers existing simultaneously and consuming memory.
I agree with your suggestion to only copy when memory usage exceeds a certain threshold. If memory usage is not high, there is indeed no need to copy the buffer. I will develop this feature later.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3479: [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization
URL: https://github.com/apache/celeborn/pull/3479
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas closed pull request #3479: [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization
URL: https://github.com/apache/celeborn/pull/3479
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#discussion_r2536139970
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala:
##########
@@ -1514,6 +1515,25 @@ class PushDataHandler(val workerSource: WorkerSource) extends BaseMessageHandler
hardSplitIndexes: Array[Int] = Array.empty[Int]): Unit = {
val length = fileWriters.length
val result = new Array[StatusCode](length)
+
+ var finalBody: ByteBuf = body
Review Comment:
LGTM
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#discussion_r2536131559
##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -4228,6 +4230,23 @@ object CelebornConf extends Logging {
.doubleConf
.createWithDefault(0.5)
+ val WORKER_PUSH_DATA_MERGE_BUFFER_ENABLED: ConfigEntry[Boolean] =
+ buildConf("celeborn.worker.pushdata.mergeBuffer.enabled")
+ .categories("worker")
+ .version("0.5.1")
+ .doc("enable merge low utilization push data's body buffer before write")
+ .booleanConf
+ .createWithDefault(true)
+
+ val WORKER_DIRECT_MEMORY_RATIO_TO_MERGE_BUFFER: ConfigEntry[Double] = {
+ buildConf("celeborn.worker.directMemoryRatioToMergeBuffer")
+ .categories("worker")
+ .version("0.5.1")
Review Comment:
ditto
##########
common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala:
##########
@@ -4228,6 +4230,23 @@ object CelebornConf extends Logging {
.doubleConf
.createWithDefault(0.5)
+ val WORKER_PUSH_DATA_MERGE_BUFFER_ENABLED: ConfigEntry[Boolean] =
+ buildConf("celeborn.worker.pushdata.mergeBuffer.enabled")
+ .categories("worker")
+ .version("0.5.1")
Review Comment:
Minor: change the version to `0.6.2` and disable merge Buffer by default.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on code in PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#discussion_r2357378252
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala:
##########
@@ -1514,6 +1515,25 @@ class PushDataHandler(val workerSource: WorkerSource) extends BaseMessageHandler
hardSplitIndexes: Array[Int] = Array.empty[Int]): Unit = {
val length = fileWriters.length
val result = new Array[StatusCode](length)
+
+ var finalBody: ByteBuf = body
Review Comment:
Copying and quickly releasing buffers can indeed reduce direct memory usage. However, I prefer to perform this in the PartitionWriter before generating the FlushTask, because after the FlushTask is completed, the chunk memory for that part can be completely released. With the current PR, the memory cannot be released until all related tasks are finished. In addition, I recommend copying the buffer only after the direct memory exceeds a certain threshold, so that when the system is not under heavy pressure, we can reduce unnecessary CPU consumption.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#issuecomment-3448202525
This issue was closed because it has been staled for 10 days with no activity.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2152] Support merge buffers on the worker side to improve memory utilization [celeborn]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #3479:
URL: https://github.com/apache/celeborn/pull/3479#issuecomment-3555631049
Merged to main(v0.7.0) and branch-0.6(v0.6.2).
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org