From 34dee6d42b6a95789008ae167bfefb82ae9b5103 Mon Sep 17 00:00:00 2001 From: EvgeniiR Date: Tue, 23 Jun 2026 17:12:50 +0200 Subject: [PATCH] Fix unreliable networking cache size metrics in nodetool info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nodetool info reported "Network Cache" size using BufferPool.sizeInBytes() which includes memoryAllocated — a monotonically non-decreasing counter that never drops below the maximum ever allocated. Switch to usedSizeInBytes() which tracks only currently checked-out buffers. Additionally fix BufferPool.LocalPool.putUnusedPortion for non-pooled (overflow) buffers: the chunk==null branch was decrementing overflowMemoryUsage by the unused portion, but the subsequent put() decremented by the full original capacity, causing overflowMemoryUsage to drift negative over time. patch by EvgeniiR; reviewed by TBD for CASSANDRA-21117 --- .../apache/cassandra/tools/nodetool/Info.java | 2 +- .../cassandra/utils/memory/BufferPool.java | 6 +-- .../metrics/BufferPoolMetricsTest.java | 47 +++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/java/org/apache/cassandra/tools/nodetool/Info.java b/src/java/org/apache/cassandra/tools/nodetool/Info.java index b0b2dff019ab..60da9ec6cdfa 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Info.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Info.java @@ -145,7 +145,7 @@ public void execute(NodeProbe probe) try { out.printf("%-23s: size %s, overflow size: %s, capacity %s%n", "Network Cache", - FileUtils.stringifyFileSize((long) probe.getBufferPoolMetric("networking", "Size")), + FileUtils.stringifyFileSize((long) probe.getBufferPoolMetric("networking", "UsedSize")), FileUtils.stringifyFileSize((long) probe.getBufferPoolMetric("networking", "OverflowSize")), FileUtils.stringifyFileSize((long) probe.getBufferPoolMetric("networking", "Capacity"))); } diff --git a/src/java/org/apache/cassandra/utils/memory/BufferPool.java b/src/java/org/apache/cassandra/utils/memory/BufferPool.java index 1cd6fef35932..bf0a5561c087 100644 --- a/src/java/org/apache/cassandra/utils/memory/BufferPool.java +++ b/src/java/org/apache/cassandra/utils/memory/BufferPool.java @@ -882,16 +882,12 @@ public void putUnusedPortion(ByteBuffer buffer) { Chunk chunk = Chunk.getParentChunk(buffer); int originalCapacity = buffer.capacity(); - int size = originalCapacity - buffer.limit(); if (chunk == null) - { - updateOverflowMemoryUsage(-size); return; - } chunk.freeUnusedPortion(buffer); - // Calculate the actual freed bytes which may be different from `size` when pooling is involved + // The actual freed bytes may differ from (originalCapacity - limit) when pooling is involved memoryInUse.inc(buffer.capacity() - originalCapacity); } diff --git a/test/unit/org/apache/cassandra/metrics/BufferPoolMetricsTest.java b/test/unit/org/apache/cassandra/metrics/BufferPoolMetricsTest.java index 8db86d89cdfd..b9d5b6ab251a 100644 --- a/test/unit/org/apache/cassandra/metrics/BufferPoolMetricsTest.java +++ b/test/unit/org/apache/cassandra/metrics/BufferPoolMetricsTest.java @@ -18,6 +18,9 @@ package org.apache.cassandra.metrics; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; import java.util.Random; import org.junit.Before; @@ -242,6 +245,50 @@ public void testFailedRequestsDontChangeMetrics() .isGreaterThanOrEqualTo(65536); } + @Test + public void testOverflowAccountingBalancesAfterPutUnusedPortionAndPut() + { + assertEquals(0, metrics.overflowSize.getValue().longValue()); + assertEquals(0, metrics.usedSize.getValue().longValue()); + + // Buffer larger than NORMAL_CHUNK_SIZE bypasses the pool and goes to overflow + int bufferSize = BufferPool.NORMAL_CHUNK_SIZE + 1; + ByteBuffer buffer = bufferPool.get(bufferSize, BufferType.OFF_HEAP); + assertEquals(bufferSize, metrics.overflowSize.getValue().longValue()); + + // Simulate partial use: caller only fills half the buffer + buffer.limit(bufferSize / 2); + bufferPool.putUnusedPortion(buffer); + + // Full return — overflowSize must return to zero, not go negative + bufferPool.put(buffer); + assertEquals(0, metrics.overflowSize.getValue().longValue()); + assertEquals(0, metrics.usedSize.getValue().longValue()); + } + + @Test + public void testUsedSizeIsZeroWhenPoolIsIdle() + { + assertEquals(0, metrics.usedSize.getValue().longValue()); + + // Allocate pooled buffers until the pool is fully expanded + int bufferSize = BufferPool.NORMAL_CHUNK_SIZE - 1; + List buffers = new ArrayList<>(); + while (bufferPool.sizeInBytes() < bufferPool.memoryUsageThreshold()) + buffers.add(bufferPool.get(bufferSize, BufferType.OFF_HEAP)); + + assertThat(metrics.usedSize.getValue().longValue()).isGreaterThan(0); + + // Return all buffers — pool stays expanded but usedSize must drop to zero + for (ByteBuffer buf : buffers) + bufferPool.put(buf); + + assertEquals(0, metrics.usedSize.getValue().longValue()); + // sizeInBytes remains at capacity (memoryAllocated is never decremented), + // confirming that usedSize and sizeInBytes measure different things + assertThat(bufferPool.sizeInBytes()).isGreaterThan(0); + } + private void tryRequestNegativeBufferSize() { assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(