HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient #9166

Gargi-jais11 · 2025-10-17T11:12:24Z

What changes were proposed in this pull request?

Currently RpcClient has a ElasticByteBufferPool to reuse buffers during EC data read and write. ElasticByteBufferPool can save the time of buffer allocation. While this Pool doesn't have a upper limit, so in s3g case, a long lived RpcClient will accumulate all buffers allocated through this pool, which lead to high memory pressure of s3g.

Solution:
Create a new class implementing ByteBufferPool which will be a bounded version of ElasticByteBufferPool that limits the total size of buffers that can be cached in the pool.
To control the size of this pool added a new configuration :

public static final String OZONE_CLIENT_ELASTIC_BYTE_BUFFER_POOL_MAX_SIZE_GB =
      "ozone.client.elastic.byte.buffer.pool.max.size.gb";
public static final String OZONE_CLIENT_ELASTIC_BYTE_BUFFER_POOL_MAX_SIZE_GB_DEFAULT = "16GB";

In RpcClient use BoundedElasticByteBufferPool instead of ElasticByteBufferPool.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13400

How was this patch tested?

Passed Existing Tests and green CI.

Gargi-jais11 · 2025-10-21T04:18:01Z

@ChenSammi could you please review this patch.

...dds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java

peterxcli

LGTM!

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java

peterxcli · 2025-10-23T05:23:56Z

@ChenSammi Would you like to take a look? Thanks!

Gargi-jais11 · 2025-10-23T05:56:18Z

@peterxcli Please take a look, I have updated the patch.

peterxcli

Other looks good.

peterxcli · 2025-10-23T06:22:41Z

hadoop-hdds/common/src/main/resources/ozone-default.xml

+      like the S3 Gateway. Once this limit is reached, used buffers are not
+      put back to the pool and will be garbage collected.


used buffers are not put back to the pool and will be garbage collected.

can we help them to deallocate the buffer immediately? so we can reduce the GC pressure.

not quite understand how GC in java works

In Java, we can't deallocate memory manually (like free() in C/C++). The only way to free memory is to remove all references to an object and let the Garbage Collector (GC) reclaim it.
When our pool is full, by returning without storing the buffer, we are doing exactly that. The buffer becomes "unreachable," and the GC will handle its deallocation.

So, I believe while we are still relying on the GC (which is unavoidable in Java), it's for a much smaller fraction of objects, which is exactly the fix we want to reduce overall s3g memory pressure.

We can also call System.gc() to suggest that the garbage collector run immediately. However, the Java Runtime makes the final decision.
According to the Java documentation.
So immediately deallocating buffer is not allowed by java. However if needed we can use System.gc() to run garbage collector immediately.

Got it, Thanks for the detail explanation. learned a lot!

Gargi-jais11 · 2025-10-23T06:45:20Z

@peterxcli Could please re-trigger these failed checks as my CI has passed these checks.
https://github.com/Gargi-jais11/ozone/actions/runs/18738774685

peterxcli · 2025-10-23T06:51:54Z

@peterxcli Could please re-trigger these failed checks as my CI has passed these checks. Gargi-jais11/ozone/actions/runs/18738774685

#9130 (comment)
#9130 (comment)

Gargi-jais11 · 2025-10-23T06:58:31Z

@peterxcli Could please re-trigger these failed checks as my CI has passed these checks. Gargi-jais11/ozone/actions/runs/18738774685

#9130 (comment) #9130 (comment)

I think I need to rebase my branch as the above Commit has been reverted.

adoroszlai · 2025-10-23T08:21:22Z

need to rebase my branch

Please use git merge, not git rebase, to avoid force-push.

Gargi-jais11 · 2025-10-23T09:15:32Z

need to rebase my branch

Please use git merge, not git rebase, to avoid force-push.

Okay sure.

peterxcli · 2025-10-25T03:05:00Z

...dds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java

+  @Override
+  public synchronized ByteBuffer getBuffer(boolean direct, int length) {
+    TreeMap<Key, ByteBuffer> tree = this.getBufferTree(direct);
+    Map.Entry<Key, ByteBuffer> entry = tree.ceilingEntry(new Key(length, 0L));
+    if (entry == null) {
+      // Pool is empty or has no suitable buffer. Allocate a new one.
+      return direct ? ByteBuffer.allocateDirect(length) : ByteBuffer.allocate(length);
+    }
+    tree.remove(entry.getKey());
+    ByteBuffer buffer = entry.getValue();
+
+    // Decrement the size because we are taking a buffer OUT of the pool.
+    currentPoolSize.addAndGet(-buffer.capacity());
+    buffer.clear();
+    return buffer;
+  }


Should we also count those "allocated but not released" buffer into the buffer size limit?

Just like BufferPool does:

ozone/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BufferPool.java

Lines 52 to 53 in 3f6ba7e

private final LinkedList<ChunkBuffer> allocated = new LinkedList<>();

private final LinkedList<ChunkBuffer> released = new LinkedList<>();

ozone/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BufferPool.java

Lines 92 to 95 in 3f6ba7e

while (allocated.size() == capacity) {

LOG.debug("Allocation needs to wait the pool is at capacity (allocated = capacity = {}).", capacity);

notFull.await();

}

I know that the original ElasticByteBufferPool doesn't do this.
Just want to make sure if we need to managed the allocated buffer, and why or why not.

I appreciate your suggestion but I think If we did count "allocated but not released" buffers toward the limit, we would be forced to change our getBuffer method to be blocking (i.e., to wait() when the limit is hit).
This would be a major, high-risk change from the original ElasticByteBufferPool's behavior, which always allocates a new buffer immediately. It could introduce performance bottlenecks or even deadlocks.

The BufferPool is linked to is a blocking, fixed-size pool. Its purpose is to strictly limit the total number of buffers ever created (e.g., "this system will only ever use 100 buffers, total"). If you ask for buffer 101, getBuffer will wait until one is returned.

Our BoundedElasticByteBufferPool is a non-blocking, caching pool. Its purpose is to fix a memory leak from the original ElasticByteBufferPool (which grew forever) while preserving its "elastic" (non-blocking) nature.

Sounds good—let’s get this merged.

peterxcli · 2025-10-29T07:24:55Z

Thanks @Gargi-jais11 for the patch, @adoroszlai for reviewing!

ivandika3 added s3 S3 Gateway EC labels Oct 19, 2025

Gargi-jais11 marked this pull request as ready for review October 21, 2025 04:17

peterxcli requested a review from ChenSammi October 21, 2025 04:38

peterxcli reviewed Oct 21, 2025

View reviewed changes

...dds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java Outdated Show resolved Hide resolved

...dds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

This comment was marked as duplicate.

Sign in to view

peterxcli reviewed Oct 22, 2025

View reviewed changes

...dds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java Show resolved Hide resolved

Gargi-jais11 requested a review from peterxcli October 22, 2025 08:28

peterxcli approved these changes Oct 22, 2025

View reviewed changes

peterxcli reviewed Oct 23, 2025

View reviewed changes

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java Outdated Show resolved Hide resolved

ivandika3 requested review from guohao-rosicky and xichen01 October 23, 2025 06:04

peterxcli approved these changes Oct 23, 2025

View reviewed changes

Gargi Jaiswal added 5 commits October 23, 2025 12:23

Created BoundedElasticByteBufferPool and added pool limit

821c3b5

refactor key creation using a logical timestamp

5eea0dd

added unit test for boundedElasticByteBufferPool

1850804

fixed findbugs failure

4b854bc

refactor configuration

9d3183e

Gargi-jais11 force-pushed the HDDS-13400 branch from 4f9aeeb to 9d3183e Compare October 23, 2025 07:00

peterxcli reviewed Oct 25, 2025

View reviewed changes

peterxcli merged commit 388f3d2 into apache:master Oct 29, 2025
43 checks passed

		like the S3 Gateway. Once this limit is reached, used buffers are not
		put back to the pool and will be garbage collected.

	private final LinkedList<ChunkBuffer> allocated = new LinkedList<>();
	private final LinkedList<ChunkBuffer> released = new LinkedList<>();

	while (allocated.size() == capacity) {
	LOG.debug("Allocation needs to wait the pool is at capacity (allocated = capacity = {}).", capacity);
	notFull.await();
	}

HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient #9166

HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient #9166

Uh oh!

Conversation

Gargi-jais11 commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Gargi-jais11 commented Oct 21, 2025

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

peterxcli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peterxcli commented Oct 23, 2025

Uh oh!

Gargi-jais11 commented Oct 23, 2025

Uh oh!

peterxcli left a comment

Choose a reason for hiding this comment

Uh oh!

peterxcli Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Gargi-jais11 Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gargi-jais11 Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

peterxcli Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Gargi-jais11 commented Oct 23, 2025

Uh oh!

peterxcli commented Oct 23, 2025

Uh oh!

Gargi-jais11 commented Oct 23, 2025

Uh oh!

adoroszlai commented Oct 23, 2025

Uh oh!

Gargi-jais11 commented Oct 23, 2025

Uh oh!

peterxcli Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

Gargi-jais11 Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Gargi-jais11 Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterxcli Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peterxcli commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Gargi-jais11 commented Oct 17, 2025 •

edited

Loading

Gargi-jais11 Oct 23, 2025 •

edited

Loading

Gargi-jais11 Oct 29, 2025 •

edited

Loading