DL1231 commented on code in PR #20847:
URL: https://github.com/apache/kafka/pull/20847#discussion_r2508606995
##########
clients/src/main/java/org/apache/kafka/common/utils/BufferSupplier.java:
##########
@@ -58,6 +63,11 @@ public static BufferSupplier create() {
*/
public abstract void release(ByteBuffer buffer);
+ /**
+ * Return total size in bytes of cached buffers.
+ */
+ public abstract long size();
Review Comment:
Thank you for pointing out this issue. As `BufferSupplier` is designed to be
a "simple non-threadsafe interface for caching byte buffers," I've made the
following targeted changes to address the specific concerns with the `size()`
method when called from metrics threads:
- Changed the `bufferMap` in `DefaultSupplier` to a `ConcurrentHashMap` to
prevent potential `ConcurrentModificationException`.
- Added the `volatile` keyword to `cachedBuffer` in `GrowableBufferSupplier`
to resolve visibility issues.
I intentionally avoided adding synchronization to the `get()`, `release()`,
and `size()` methods to maintain performance, as comprehensive thread safety
isn't a design goal for this interface. These changes specifically address the
metrics collection use case while preserving the lightweight
--
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]
