dschneider-pivotal commented on a change in pull request #7127:
URL: https://github.com/apache/geode/pull/7127#discussion_r753365448



##########
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/executor/server/InfoExecutor.java
##########
@@ -127,13 +127,34 @@ private String getClientsSection(ExecutionHandlerContext 
context) {
         "blocked_clients:0\r\n";
   }
 
+  /**
+   * 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 amount of memory
+   * available to the partitioned region (Java heap) vs. the amount of memory 
consumed by data in
+   * the region. This ratio can only approach 1.0 and cannot go lower. 
However, the closer to 1.0

Review comment:
       Actually, with the way you have implemented it, it can go below 1.0. 
This is because pr.getLocalMaxMemory is by default something like 90% of the 
jvm's max heap. But geode does not enforce this limit in any way. So we can add 
more data than that limit to the pr. 
   Also, because we use heap memory for things that are not stored in the pr 
(think pubsub), I think it would be better for our "usedmemory" not to be 
limited to the getDataStore().currentAllocatedMemory(). 
   For this fragmentation ratio I think we should ask the jvm for the max heap 
(Runtime.maxMemory), and then divide it by how much memory the jvm is using 
(Runtime.maxMemory()-Runtime.freeMemory()). I would not use Runtime.totalMemory 
because I have seen jvms (Azul) that set max and total the same but do have a 
meaningful freeMemory value. 
   I wrote up another ticket discussing all this a couple of weeks ago (check 
the misc section). You don't need to do all that for this ticket. You could 
just focus on the fragmentation ratio.




-- 
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]


Reply via email to