This is an automated email from the ASF dual-hosted git repository. jensdeppe pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push: new e69590e GEODE-9669: Improve reporting of Radish mem_fragmentation_ratio (#7127) e69590e is described below commit e69590e1061c0184385c1b2305f7593b1ec3f01a Author: Jens Deppe <jde...@vmware.com> AuthorDate: Tue Nov 30 05:13:35 2021 -0800 GEODE-9669: Improve reporting of Radish mem_fragmentation_ratio (#7127) - For this implementation, this value is the ratio of total amount of memory available to the JVM (Java heap) vs. the amount of memory used by the JVM. --- .../AbstractRedisInfoStatsIntegrationTest.java | 2 -- .../AbstractAppendMemoryIntegrationTest.java | 16 +++++---- .../commands/executor/server/InfoExecutor.java | 38 +++++++++++++++++----- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractRedisInfoStatsIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractRedisInfoStatsIntegrationTest.java index 480619f..30c1edc 100644 --- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractRedisInfoStatsIntegrationTest.java +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractRedisInfoStatsIntegrationTest.java @@ -28,7 +28,6 @@ import org.assertj.core.data.Offset; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import redis.clients.jedis.Jedis; @@ -142,7 +141,6 @@ public abstract class AbstractRedisInfoStatsIntegrationTest implements RedisInte assertThat(Long.valueOf(getInfo(jedis).get(USED_MEMORY))).isGreaterThan(0); } - @Ignore("tracked by GEODE-9669") // currently we return 1.0 @Test public void memFragmentation_shouldBeGreaterThanOne() { for (int i = 0; i < 10000; i++) { diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractAppendMemoryIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractAppendMemoryIntegrationTest.java index 05bea0b..e1e44e9 100644 --- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractAppendMemoryIntegrationTest.java +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractAppendMemoryIntegrationTest.java @@ -16,6 +16,7 @@ package org.apache.geode.redis.internal.commands.executor.string; import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; import java.util.Map; import org.junit.After; @@ -45,21 +46,24 @@ public abstract class AbstractAppendMemoryIntegrationTest implements RedisIntegr @Test public void testAppend_actuallyIncreasesBucketSize() { - int listSize = 1000; + int listSize = 100_000; String key = "key"; - Map<String, String> info = RedisTestHelper.getInfo(jedis); - Long previousMemValue = Long.valueOf(info.get("used_memory")); + System.gc(); + Long startingMemValue = getUsedMemory(jedis); jedis.set(key, "initial"); for (int i = 0; i < listSize; i++) { jedis.append(key, "morestuff"); } - info = RedisTestHelper.getInfo(jedis); - Long finalMemValue = Long.valueOf(info.get("used_memory")); + GeodeAwaitility.await().atMost(Duration.ofSeconds(20)).pollInterval(Duration.ofSeconds(1)) + .untilAsserted(() -> assertThat(getUsedMemory(jedis)).isGreaterThan(startingMemValue)); + } - assertThat(finalMemValue).isGreaterThan(previousMemValue); + private Long getUsedMemory(Jedis jedis) { + Map<String, String> info = RedisTestHelper.getInfo(jedis); + return Long.valueOf(info.get("used_memory")); } } diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/InfoExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/InfoExecutor.java index 2a3c420..08192df 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/InfoExecutor.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/InfoExecutor.java @@ -31,7 +31,6 @@ import java.text.DecimalFormat; import java.util.Arrays; import java.util.List; -import org.apache.geode.internal.cache.PartitionedRegion; import org.apache.geode.redis.internal.commands.Command; import org.apache.geode.redis.internal.commands.executor.CommandExecutor; import org.apache.geode.redis.internal.commands.executor.RedisResponse; @@ -77,7 +76,7 @@ public class InfoExecutor implements CommandExecutor { } else if (Arrays.equals(bytes, CLIENTS)) { return getClientsSection(context); } else if (Arrays.equals(bytes, MEMORY)) { - return getMemorySection(context); + return getMemorySection(); } else if (Arrays.equals(bytes, KEYSPACE)) { return getKeyspaceSection(context); } else if (Arrays.equals(bytes, DEFAULT) || Arrays.equals(bytes, ALL)) { @@ -127,13 +126,36 @@ public class InfoExecutor implements CommandExecutor { "blocked_clients:0\r\n"; } - private String getMemorySection(ExecutionHandlerContext context) { - PartitionedRegion pr = (PartitionedRegion) context.getRegionProvider().getDataRegion(); - long usedMemory = pr.getDataStore().currentAllocatedMemory(); + /** + * Redis' fragmentation ratio is calculated from process_rss / used_memory. Essentially this is + * the ratio of physical memory being used vs. the amount of virtual memory the process + * requires. So, effectively an indication of memory pressure. If this number goes below 1.0 it + * would indicate that the process has started to swap memory which is bad. + * <p/> + * In a similar sense, the calculation for fragmentation here is a ratio of the maximum amount + * of memory available to the JVM (Java heap) vs. the amount of memory used by the JVM. This + * ratio can only approach 1.0 and cannot go lower. However, the closer to 1.0, the greater the + * likelihood of incurring GC pauses. This is analogous to swapping and will have a very + * similar effect in that it will adversely impact performance. + * <p/> + * Used memory is derived from {@link Runtime} memory and is calculated as + * {@code totalMemory() - freeMemory()}. + */ + private String getMemorySection() { + long usedMemory = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + + String fragmentationRatio; + if (usedMemory != 0) { + fragmentationRatio = String.format("%.2f", + Runtime.getRuntime().maxMemory() / (float) usedMemory); + } else { + fragmentationRatio = "1.0"; + } + return "# Memory\r\n" + - "maxmemory:" + pr.getLocalMaxMemory() * ONE_MEGABYTE + "\r\n" + + "maxmemory:" + Runtime.getRuntime().maxMemory() + "\r\n" + "used_memory:" + usedMemory + "\r\n" + - "mem_fragmentation_ratio:1.00\r\n"; + "mem_fragmentation_ratio:" + fragmentationRatio + "\r\n"; } private String getKeyspaceSection(ExecutionHandlerContext context) { @@ -169,7 +191,7 @@ public class InfoExecutor implements CommandExecutor { final String SECTION_SEPARATOR = "\r\n"; return getServerSection(context) + SECTION_SEPARATOR + getClientsSection(context) + SECTION_SEPARATOR + - getMemorySection(context) + SECTION_SEPARATOR + + getMemorySection() + SECTION_SEPARATOR + getPersistenceSection() + SECTION_SEPARATOR + getStatsSection(context) + SECTION_SEPARATOR + getKeyspaceSection(context) + SECTION_SEPARATOR +