taklwu commented on code in PR #6950:
URL: https://github.com/apache/hbase/pull/6950#discussion_r2070634979
##########
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++ %>
Review Comment:
so, will this page result in a very large page or unlimited length if the
region server for e.g. 7 days? or is the paginated feature enabled
automatically ? if not , can we implement it ?
##########
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:
in line 236, i saw the `getPeriodTimeInMinutes` has a check on > 0, do we
need to check here as well ?
##########
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>
Review Comment:
nit: could we add a space in between `/` ?
```suggestion
<td><% String.format("%,.2f",
((double)bc.getStats().getHitCounts()[i] /
(double)bc.getStats().getRequestCounts()[i]) * 100.0) %><% "%" %></td>
```
--
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]
