You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Gargi-jais11 (via GitHub)" <gi...@apache.org> on 2025/10/17 11:12:27 UTC
[PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Gargi-jais11 opened a new pull request, #9166:
URL: https://github.com/apache/ozone/pull/9166
## 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.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435422932
> @peterxcli Could please re-trigger these failed checks as my CI has passed these checks. [Gargi-jais11/ozone/actions/runs/18738774685](https://github.com/Gargi-jais11/ozone/actions/runs/18738774685)
https://github.com/apache/ozone/pull/9130#issuecomment-3435328316
https://github.com/apache/ozone/pull/9130#issuecomment-3435422475
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2450383125
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
Review Comment:
Okay, could you help me to figure out what all test cases we need to verify for this?
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2454095134
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -465,6 +465,18 @@
<description>Socket timeout for Ozone client. Unit could be defined with
postfix (ns,ms,s,m,h,d)</description>
</property>
+ <property>
+ <name>ozone.client.elastic.byte.buffer.pool.max.size</name>
+ <value>16GB</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ The maximum total size of buffers that can be cached in the client-side
+ ByteBufferPool. This pool is used heavily during EC read and write operations.
+ Setting a limit prevents unbounded memory growth in long-lived rpc clients
+ like the S3 Gateway. Once this limit is reached, used buffers are not
+ put back to the pool and will be garbage collected.
Review Comment:
Got it, Thanks for the detail explanation. learned a lot!
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2462380250
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ /**
+ * A logical timestamp counter used for creating unique Keys in the TreeMap.
+ * This is used as the insertionTime for the Key instead of System.nanoTime()
+ * to guarantee uniqueness and avoid a potential spin-wait in putBuffer
+ * if two buffers of the same capacity are added at the same nanosecond.
+ */
+ private long logicalTimestamp = 0;
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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;
+ }
Review Comment:
Should we also count those "allocated but not released" buffer into the buffer size limit?
Just like BufferPool does:
https://github.com/apache/ozone/blob/3f6ba7e267eabfa2f418d12df8d37422ab16acf1/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BufferPool.java#L52-L53
https://github.com/apache/ozone/blob/3f6ba7e267eabfa2f418d12df8d37422ab16acf1/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BufferPool.java#L92-L95
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.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli merged PR #9166:
URL: https://github.com/apache/ozone/pull/9166
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435142553
@ChenSammi Would you like to take a look? Thanks!
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2471736109
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ /**
+ * A logical timestamp counter used for creating unique Keys in the TreeMap.
+ * This is used as the insertionTime for the Key instead of System.nanoTime()
+ * to guarantee uniqueness and avoid a potential spin-wait in putBuffer
+ * if two buffers of the same capacity are added at the same nanosecond.
+ */
+ private long logicalTimestamp = 0;
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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;
+ }
Review Comment:
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.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2471737352
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ /**
+ * A logical timestamp counter used for creating unique Keys in the TreeMap.
+ * This is used as the insertionTime for the Key instead of System.nanoTime()
+ * to guarantee uniqueness and avoid a potential spin-wait in putBuffer
+ * if two buffers of the same capacity are added at the same nanosecond.
+ */
+ private long logicalTimestamp = 0;
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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;
+ }
Review Comment:
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.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2453952699
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -690,6 +690,10 @@ public final class OzoneConfigKeys {
"ozone.security.crypto.compliance.mode";
public static final String OZONE_SECURITY_CRYPTO_COMPLIANCE_MODE_UNRESTRICTED = "unrestricted";
+ 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";
+
Review Comment:
nit:
```suggestion
public static final String OZONE_CLIENT_ELASTIC_BYTE_BUFFER_POOL_MAX_SIZE =
"ozone.client.elastic.byte.buffer.pool.max.size";
public static final String OZONE_CLIENT_ELASTIC_BYTE_BUFFER_POOL_MAX_SIZE_DEFAULT = "16GB";
```
dont need size unit in the config key.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2454075630
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -465,6 +465,18 @@
<description>Socket timeout for Ozone client. Unit could be defined with
postfix (ns,ms,s,m,h,d)</description>
</property>
+ <property>
+ <name>ozone.client.elastic.byte.buffer.pool.max.size</name>
+ <value>16GB</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ The maximum total size of buffers that can be cached in the client-side
+ ByteBufferPool. This pool is used heavily during EC read and write operations.
+ Setting a limit prevents unbounded memory growth in long-lived rpc clients
+ like the S3 Gateway. Once this limit is reached, used buffers are not
+ put back to the pool and will be garbage collected.
Review Comment:
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.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2446852631
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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);
+ } else {
+ 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;
+ }
+ }
+
+ @Override
+ public synchronized void putBuffer(ByteBuffer buffer) {
+ if (buffer == null) {
+ return;
+ }
+
+ if (currentPoolSize.get() + buffer.capacity() > maxPoolSize) {
+ // Pool is full, do not add the buffer back.
+ // It will be garbage collected by JVM.
+ return;
+ }
+
+ buffer.clear();
+ TreeMap<Key, ByteBuffer> tree = getBufferTree(buffer.isDirect());
+ Key key;
+ do {
+ key = new Key(buffer.capacity(), System.nanoTime());
+ } while (tree.containsKey(key));
Review Comment:
How about using logic time instead of system time, so we dont have to wait here.
```suggestion
key = new Key(buffer.capacity(), ts++);
```
ts is init to 0, and only increase itself here.
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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);
+ } else {
+ 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;
+ }
Review Comment:
nit:
```suggestion
}
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;
```
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435438789
> > @peterxcli Could please re-trigger these failed checks as my CI has passed these checks. [Gargi-jais11/ozone/actions/runs/18738774685](https://github.com/Gargi-jais11/ozone/actions/runs/18738774685)
>
> [#9130 (comment)](https://github.com/apache/ozone/pull/9130#issuecomment-3435328316) [#9130 (comment)](https://github.com/apache/ozone/pull/9130#issuecomment-3435422475)
I think I need to rebase my branch as the above Commit has been reverted.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3424606180
@ChenSammi could you please review this patch.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2471737352
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ /**
+ * A logical timestamp counter used for creating unique Keys in the TreeMap.
+ * This is used as the insertionTime for the Key instead of System.nanoTime()
+ * to guarantee uniqueness and avoid a potential spin-wait in putBuffer
+ * if two buffers of the same capacity are added at the same nanosecond.
+ */
+ private long logicalTimestamp = 0;
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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;
+ }
Review Comment:
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.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2450340540
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
Review Comment:
wait, we might need to add a unit test class for this
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2454083998
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -465,6 +465,18 @@
<description>Socket timeout for Ozone client. Unit could be defined with
postfix (ns,ms,s,m,h,d)</description>
</property>
+ <property>
+ <name>ozone.client.elastic.byte.buffer.pool.max.size</name>
+ <value>16GB</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ The maximum total size of buffers that can be cached in the client-side
+ ByteBufferPool. This pool is used heavily during EC read and write operations.
+ Setting a limit prevents unbounded memory growth in long-lived rpc clients
+ like the S3 Gateway. Once this limit is reached, used buffers are not
+ put back to the pool and will be garbage collected.
Review Comment:
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](http://docs.oracle.com/javase/7/docs/api/java/lang/System.html#gc()).
So immediately deallocating buffer is not allowed by java. However if needed we can use **System.gc()** to run garbage collector immediately.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435699940
> need to rebase my branch
Please use `git merge`, not `git rebase`, to avoid force-push.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2447680461
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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);
+ } else {
+ 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;
+ }
+ }
+
+ @Override
+ public synchronized void putBuffer(ByteBuffer buffer) {
+ if (buffer == null) {
+ return;
+ }
+
+ if (currentPoolSize.get() + buffer.capacity() > maxPoolSize) {
+ // Pool is full, do not add the buffer back.
+ // It will be garbage collected by JVM.
+ return;
+ }
+
+ buffer.clear();
+ TreeMap<Key, ByteBuffer> tree = getBufferTree(buffer.isDirect());
+ Key key;
+ do {
+ key = new Key(buffer.capacity(), System.nanoTime());
+ } while (tree.containsKey(key));
Review Comment:
Thanks @peterxcli for feedback. I agree with this change.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2450400276
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
Review Comment:
I think the most important thing is to make sure the tree map timestamp ordering is right, especially since we change from wall time to logical timestamp
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2454043902
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -465,6 +465,18 @@
<description>Socket timeout for Ozone client. Unit could be defined with
postfix (ns,ms,s,m,h,d)</description>
</property>
+ <property>
+ <name>ozone.client.elastic.byte.buffer.pool.max.size</name>
+ <value>16GB</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ The maximum total size of buffers that can be cached in the client-side
+ ByteBufferPool. This pool is used heavily during EC read and write operations.
+ Setting a limit prevents unbounded memory growth in long-lived rpc clients
+ like the S3 Gateway. Once this limit is reached, used buffers are not
+ put back to the pool and will be garbage collected.
Review Comment:
> 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
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2454075630
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -465,6 +465,18 @@
<description>Socket timeout for Ozone client. Unit could be defined with
postfix (ns,ms,s,m,h,d)</description>
</property>
+ <property>
+ <name>ozone.client.elastic.byte.buffer.pool.max.size</name>
+ <value>16GB</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ The maximum total size of buffers that can be cached in the client-side
+ ByteBufferPool. This pool is used heavily during EC read and write operations.
+ Setting a limit prevents unbounded memory growth in long-lived rpc clients
+ like the S3 Gateway. Once this limit is reached, used buffers are not
+ put back to the pool and will be garbage collected.
Review Comment:
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.
The key point is that this whole patch is designed to massively reduce that GC pressure.
**Before this patch:** 100% of buffers were created and then garbage collected.
**After this patch:** We cache and reuse most buffers. Only the excess buffers (that don't fit in the pool) are left for the GC.
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 memory and GC pressure.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2471971651
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.client.io;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ComparisonChain;
+import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
+import org.apache.hadoop.io.ByteBufferPool;
+
+/**
+ * A bounded version of ElasticByteBufferPool that limits the total size
+ * of buffers that can be cached in the pool. This prevents unbounded memory
+ * growth in long-lived rpc clients like S3 Gateway.
+ *
+ * When the pool reaches its maximum size, newly returned buffers are not
+ * added back to the pool and will be garbage collected instead.
+ */
+public class BoundedElasticByteBufferPool implements ByteBufferPool {
+ private final TreeMap<Key, ByteBuffer> buffers = new TreeMap<>();
+ private final TreeMap<Key, ByteBuffer> directBuffers = new TreeMap<>();
+ private final long maxPoolSize;
+ private final AtomicLong currentPoolSize = new AtomicLong(0);
+
+ /**
+ * A logical timestamp counter used for creating unique Keys in the TreeMap.
+ * This is used as the insertionTime for the Key instead of System.nanoTime()
+ * to guarantee uniqueness and avoid a potential spin-wait in putBuffer
+ * if two buffers of the same capacity are added at the same nanosecond.
+ */
+ private long logicalTimestamp = 0;
+
+ public BoundedElasticByteBufferPool(long maxPoolSize) {
+ super();
+ this.maxPoolSize = maxPoolSize;
+ }
+
+ private TreeMap<Key, ByteBuffer> getBufferTree(boolean direct) {
+ return direct ? this.directBuffers : this.buffers;
+ }
+
+ @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;
+ }
Review Comment:
Sounds good—let’s get this merged.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2450383125
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
Review Comment:
Okay, could you please suggest me some of the test cases we need to verify for this?
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435277539
@peterxcli Please take a look, I have updated the patch.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435407654
@peterxcli Could please re-trigger these failed checks as my CI has passed these checks.
https://github.com/Gargi-jais11/ozone/actions/runs/18738774685
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3460166937
Thanks @Gargi-jais11 for the patch, @adoroszlai for reviewing!
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9166:
URL: https://github.com/apache/ozone/pull/9166#issuecomment-3435897598
> > need to rebase my branch
>
> Please use `git merge`, not `git rebase`, to avoid force-push.
Okay sure.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2454075630
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -465,6 +465,18 @@
<description>Socket timeout for Ozone client. Unit could be defined with
postfix (ns,ms,s,m,h,d)</description>
</property>
+ <property>
+ <name>ozone.client.elastic.byte.buffer.pool.max.size</name>
+ <value>16GB</value>
+ <tag>OZONE, CLIENT</tag>
+ <description>
+ The maximum total size of buffers that can be cached in the client-side
+ ByteBufferPool. This pool is used heavily during EC read and write operations.
+ Setting a limit prevents unbounded memory growth in long-lived rpc clients
+ like the S3 Gateway. Once this limit is reached, used buffers are not
+ put back to the pool and will be garbage collected.
Review Comment:
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 memory and GC pressure.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13400. S3g has accumulated memory pressure due to unlimited ElasticByteBufferPool in RpcClient [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9166:
URL: https://github.com/apache/ozone/pull/9166#discussion_r2450414073
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BoundedElasticByteBufferPool.java:
##########
@@ -0,0 +1,148 @@
+/*
Review Comment:
Okay sure
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org