-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding new metrics for SAI in nodetool tablestats command. #3742
base: trunk
Are you sure you want to change the base?
Changes from 1 commit
f5e31ee
8a322bc
e12e844
c383226
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1892,7 +1892,66 @@ public Object getThreadPoolMetric(String pathName, String poolName, String metri | |||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
public Object getSAIIndexMetrics(String ks, String cf, String metricName) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
try { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: { on newline |
||||||
String scope = getScopeForMetric(metricName); | ||||||
String objectNameStr = String.format("org.apache.cassandra.metrics:type=StorageAttachedIndex,keyspace=%s,table=%s,scope=%s,name=%s",ks, cf, scope, metricName); | ||||||
ObjectName oName = new ObjectName(objectNameStr); | ||||||
|
||||||
Set<ObjectName> matchingMBeans = mbeanServerConn.queryNames(oName, null); | ||||||
if (matchingMBeans.isEmpty()) { | ||||||
return null; | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: might as well just avoid the braces for this |
||||||
|
||||||
return getMetricValue(metricName, oName); | ||||||
} catch (MalformedObjectNameException e) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: { on newline |
||||||
throw new RuntimeException("Invalid ObjectName format: " + e.getMessage(), e); | ||||||
} catch (IOException e) { | ||||||
throw new RuntimeException("Error accessing MBean server: " + e.getMessage(), e); | ||||||
} | ||||||
} | ||||||
|
||||||
private Object getMetricValue(String metricName, ObjectName oName) throws IOException { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
switch (metricName) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: { on newline |
||||||
case "QueryLatency": | ||||||
return JMX.newMBeanProxy(mbeanServerConn, oName, CassandraMetricsRegistry.JmxTimerMBean.class); | ||||||
case "PostFilteringReadLatency": | ||||||
case "SSTableIndexesHit": | ||||||
case "IndexSegmentsHit": | ||||||
case "RowsFiltered": | ||||||
return JMX.newMBeanProxy(mbeanServerConn, oName, CassandraMetricsRegistry.JmxHistogramMBean.class); | ||||||
case "DiskUsedBytes": | ||||||
case "TotalIndexCount": | ||||||
case "TotalQueryableIndexCount": | ||||||
return JMX.newMBeanProxy(mbeanServerConn, oName, CassandraMetricsRegistry.JmxGaugeMBean.class).getValue(); | ||||||
case "TotalQueryTimeouts": | ||||||
return JMX.newMBeanProxy(mbeanServerConn, oName, CassandraMetricsRegistry.JmxCounterMBean.class).getCount(); | ||||||
default: | ||||||
throw new IllegalArgumentException("Unknown metric name: " + metricName); | ||||||
} | ||||||
} | ||||||
|
||||||
private String getScopeForMetric(String metricName) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
switch (metricName) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: { on newline |
||||||
case "QueryLatency": | ||||||
case "SSTableIndexesHit": | ||||||
case "IndexSegmentsHit": | ||||||
case "RowsFiltered": | ||||||
return "PerQuery"; | ||||||
case "PostFilteringReadLatency": | ||||||
case "TotalQueryTimeouts": | ||||||
return "TableQueryMetrics"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have I wouldn't say we have to extend this to the individual metric names, as we shouldn't be changing those anyway (with them being a user-facing thing). |
||||||
case "DiskUsedBytes": | ||||||
return "IndexGroupMetrics"; | ||||||
case "TotalIndexCount": | ||||||
case "TotalQueryableIndexCount": | ||||||
return "TableStateMetrics"; | ||||||
default: | ||||||
throw new IllegalArgumentException("Unknown metric name: " + metricName); | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* Retrieve threadpool paths and names for threadpools with metrics. | ||||||
* @return Multimap from path (internal, request, etc.) to name | ||||||
*/ | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,4 +82,15 @@ public class StatsTable | |
public double localReadWriteRatio; | ||
public Long twcsDurationInMillis; | ||
public String twcs; | ||
|
||
public double QueryLatencyMs; | ||
public double PostFilteringReadLatencyms; | ||
public String DiskUsedBytes; | ||
public double SSTableIndexesHit; | ||
public double IndexSegmentsHit; | ||
public double RowsFiltered; | ||
public long TotalQueryTimeouts; | ||
public int TotalIndexCount; | ||
public int TotalQueryableIndexCount; | ||
public String TotalQueryableIndexRatio; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit/style: For the above, make sure to start field names w/ lower-case letter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...or we could just prefix them all with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added prefix as sai. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -338,6 +338,47 @@ else if (sortKey.equals("table_name")) | |
{ | ||
return sign * stx.tableName.compareTo(sty.tableName); | ||
} | ||
else if(sortKey.equals("QueryLatency")) | ||
{ | ||
result = compareDoubles(stx.QueryLatencyMs, sty.QueryLatencyMs); | ||
} | ||
else if(sortKey.equals("PostFilteringReadLatencyms")) | ||
{ | ||
result = compareDoubles(stx.PostFilteringReadLatencyms, sty.PostFilteringReadLatencyms); | ||
} | ||
else if(sortKey.equals("disk_used_bytes")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be "sai_disk_used_bytes"? |
||
{ | ||
return sign * ((String) stx.DiskUsedBytes).compareTo((String) sty.DiskUsedBytes); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use |
||
} | ||
else if(sortKey.equals("SSTableIndexesHit")) | ||
{ | ||
return compareDoubles(stx.SSTableIndexesHit, sty.SSTableIndexesHit); | ||
} | ||
else if(sortKey.equals("IndexSegmentsHit")) | ||
{ | ||
return compareDoubles(stx.IndexSegmentsHit, sty.IndexSegmentsHit); | ||
} | ||
else if(sortKey.equals("RowsFiltered")) | ||
{ | ||
return compareDoubles(stx.RowsFiltered, sty.RowsFiltered); | ||
} | ||
else if(sortKey.equals("TotalQueryTimeouts")) | ||
{ | ||
return sign * ((Long) stx.TotalQueryTimeouts).compareTo((Long) sty.TotalQueryTimeouts); | ||
} | ||
else if(sortKey.equals("TotalIndexCount")) | ||
{ | ||
return sign * ((int) stx.TotalIndexCount); | ||
} | ||
else if(sortKey.equals("TotalQueryableIndexCount")) | ||
{ | ||
return sign * ((int) stx.TotalQueryableIndexCount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For |
||
} | ||
else if(sortKey.equals("TotalQueryableIndexRatio")) | ||
{ | ||
return sign * ((String) stx.TotalQueryableIndexRatio).compareTo((String) sty.TotalQueryableIndexRatio); | ||
} | ||
|
||
else | ||
{ | ||
throw new IllegalStateException(String.format("Unsupported sort key: %s", sortKey)); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -175,6 +175,17 @@ private Map<String, Object> convertStatsTableToMap(StatsTable table) | |
mpTable.put("top_tombstone_partitions", table.topTombstonePartitions); | ||
if (locationCheck) | ||
mpTable.put("sstables_in_correct_location", table.isInCorrectLocation); | ||
mpTable.put("sai_local_query_latency",String.format("%01.3f", table.QueryLatencyMs)); | ||
mpTable.put("sai_post_filtering_read_latency",String.format("%01.3f", table.PostFilteringReadLatencyms)); | ||
mpTable.put("sai_disk_used_bytes",table.DiskUsedBytes); | ||
mpTable.put("sai_SSTable_Indexes_Hit",table.SSTableIndexesHit); | ||
mpTable.put("sai_Index_Segments_Hit",table.IndexSegmentsHit); | ||
mpTable.put("sai_Rows_Filtered",table.RowsFiltered); | ||
mpTable.put("sai_total_query_timeouts",table.TotalQueryTimeouts); | ||
mpTable.put("total_index_count",table.TotalIndexCount); | ||
mpTable.put("queryable_index_count",table.TotalQueryableIndexCount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: We should probably prefix these two with |
||
mpTable.put("sai_queryable_total_indexes", table.TotalQueryableIndexRatio); | ||
|
||
return mpTable; | ||
} | ||
|
||
|
@@ -393,12 +404,50 @@ private void initializeKeyspaces(NodeProbe probe, boolean ignore, List<String> t | |
if (table.getTopTombstonePartitionsLastUpdate() != null) | ||
statsTable.topTombstonePartitionsLastUpdate = millisToDateString(table.getTopTombstonePartitionsLastUpdate()); | ||
|
||
Object queryLatencyMetric = probe.getSAIIndexMetrics(keyspaceName, tableName, "QueryLatency"); | ||
double QueryLatency = getMetricMean(queryLatencyMetric); | ||
statsTable.QueryLatencyMs = QueryLatency > 0 ? QueryLatency : Double.NaN; | ||
|
||
Object PostFilteringReadLatency = probe.getSAIIndexMetrics(keyspaceName, tableName, "PostFilteringReadLatency"); | ||
double postfilteringreadlatency = getMetricMean(PostFilteringReadLatency); | ||
statsTable.PostFilteringReadLatencyms = postfilteringreadlatency > 0 ? postfilteringreadlatency : Double.NaN; | ||
|
||
Object diskUsedBytes = probe.getSAIIndexMetrics(keyspaceName, tableName, "DiskUsedBytes"); | ||
statsTable.DiskUsedBytes = (diskUsedBytes != null) ? format(Map.of("DiskUsedBytes", (long) diskUsedBytes), humanReadable).get("DiskUsedBytes") : "NaN"; | ||
|
||
Object SSTableIndexesHit = probe.getSAIIndexMetrics(keyspaceName, tableName, "SSTableIndexesHit"); | ||
statsTable.SSTableIndexesHit = getMetricMean(SSTableIndexesHit); | ||
|
||
Object IndexSegmentsHit = probe.getSAIIndexMetrics(keyspaceName, tableName, "IndexSegmentsHit"); | ||
statsTable.IndexSegmentsHit = getMetricMean(IndexSegmentsHit); | ||
|
||
Object RowsFiltered = probe.getSAIIndexMetrics(keyspaceName, tableName, "RowsFiltered"); | ||
statsTable.RowsFiltered = getMetricMean(RowsFiltered); | ||
|
||
Object totalIndexCount = probe.getSAIIndexMetrics(keyspaceName, tableName, "TotalIndexCount"); | ||
statsTable.TotalIndexCount = (totalIndexCount != null) ? (int) totalIndexCount : 0; | ||
|
||
Object totalQueryableIndexCount = probe.getSAIIndexMetrics(keyspaceName, tableName, "TotalQueryableIndexCount"); | ||
statsTable.TotalQueryableIndexCount = (totalQueryableIndexCount != null) ? (int) totalQueryableIndexCount : 0; | ||
|
||
statsTable.TotalQueryableIndexRatio = String.format("%d/%d", statsTable.TotalIndexCount, statsTable.TotalQueryableIndexCount); | ||
|
||
statsKeyspace.tables.add(statsTable); | ||
} | ||
keyspaces.add(statsKeyspace); | ||
} | ||
} | ||
|
||
private double getMetricMean(Object metricObject) { | ||
if (metricObject instanceof CassandraMetricsRegistry.JmxTimerMBean) { | ||
return ((CassandraMetricsRegistry.JmxTimerMBean) metricObject).getMean() / 1000; | ||
} | ||
if (metricObject instanceof CassandraMetricsRegistry.JmxHistogramMBean) { | ||
return Math.round(((CassandraMetricsRegistry.JmxHistogramMBean) metricObject).getMean() * 100.0) / 100.0; | ||
} | ||
return Double.NaN; | ||
} | ||
|
||
private void maybeAddTWCSWindowWithMaxDuration(StatsTable statsTable, NodeProbe probe, String keyspaceName, String tableName) | ||
{ | ||
Map<String, String> compactionParameters = probe.getCfsProxy(statsTable.keyspaceName, statsTable.tableName) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
{
on newline