wchevreuil commented on code in PR #6950:
URL: https://github.com/apache/hbase/pull/6950#discussion_r2071409186


##########
hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/BlockCacheTmpl.jamon:
##########
@@ -216,11 +216,30 @@ 
org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix;
         <td>Block requests that were cache misses but only requests set to use 
block cache</td>
     </tr>
     <tr>
-        <td>Hit Ratio</td>
+        <td>All Time Hit Ratio</td>
         <td><% String.format("%,.2f", bc.getStats().getHitRatio() * 100) %><% 
"%" %></td>
         <td>Hit Count divided by total requests count</td>
     </tr>
-
+    <%for int i=0; i<bc.getStats().getNumPeriodsInWindow(); i++ %>
+      <%if bc.getStats().getWindowPeriods()[i] != null %>
+          <tr>
+              <td>Hit Ratio for period starting at <% 
bc.getStats().getWindowPeriods()[i] %></td>
+              <%if bc.getStats().getRequestCounts()[i] > 0 %>
+                <td><% String.format("%,.2f", 
((double)bc.getStats().getHitCounts()[i]/(double)bc.getStats().getRequestCounts()[i])
 * 100.0) %><% "%" %></td>
+              <%else>
+                <td>No requests</td>
+              </%if>
+              <td>Hit Count divided by total requests count over the <% i %>th 
period of <% bc.getStats().getPeriodTimeInMinutes() %> minutes</td>

Review Comment:
   Here it's not needed, because if it's 0, we won't schedule the periods 
roller thread in CacheStats that is necessary to produce the periods windows we 
are iterating over in this for loop. That said, I think it may be ok to add a 
check just for the sake of clarity.



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