saurabhd336 opened a new pull request, #3173: URL: https://github.com/apache/celeborn/pull/3173
### What changes were proposed in this pull request? There is an unnecessary explicit `buffer[i] = null` that would cause certain set of buffers to not be reused from the pool, leading to higher memory usage and gc pressure ### Why are the changes needed? https://github.com/apache/celeborn/pull/131 introduced a feature to reuse buffers across mapper tasks. However it seems like when closing the writer, we explicitly set certain (i.e. buffers which are non empty and need to be flushed when closing) buffer to null, leading to these buffers not being returned to the pool. As a result, such buffers are permanently lost (GC'ed), and we'd end up creating more buffers that really necessary [here](https://github.com/apache/celeborn/blob/main/client-spark/spark-2/src/main/java/org/apache/spark/shuffle/celeborn/HashBasedShuffleWriter.java#L270), which contributes to the overall total memory usage. I checked the flow and the buffer being merged, is compressed [here](https://github.com/apache/celeborn/blob/main/client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java#L1019) and then copied into a new buffer [here](https://github.com/apache/celeborn/blob/main/client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java#L1031), and all of these are sync operations i.e. the overwrite to the buffer by a newly started writer task, should not cause any data corruption, hence it's safe to do so. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? TODO (add UTs), but manually tested once -- 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]
