Skip to content
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

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 60 additions & 1 deletion src/java/org/apache/cassandra/tools/NodeProbe.java
Original file line number Diff line number Diff line change
Expand Up @@ -1892,7 +1892,66 @@ public Object getThreadPoolMetric(String pathName, String poolName, String metri
}
}

/**
public Object getSAIIndexMetrics(String ks, String cf, String metricName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: { on newline

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public Object getSAIIndexMetrics(String ks, String cf, String metricName) {
public Object getSaiMetric(String ks, String cf, String metricName) {

try {
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might as well just avoid the braces for this if


return getMetricValue(metricName, oName);
} catch (MalformedObjectNameException e) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: { on newline

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private Object getMetricValue(String metricName, ObjectName oName) throws IOException {
private Object getSaiMetricValue(String metricName, ObjectName oName) throws IOException {

switch (metricName) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: { on newline

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private String getScopeForMetric(String metricName) {
private String getSaiMetricScope(String metricName) {

switch (metricName) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have TableQueryMetrics.TABLE_QUERY_METRIC_TYPE and TableStateMetrics.TABLE_STATE_METRIC_TYPE that we can reuse here, but I feel like it would be a good time to do something similar for "IndexGroupMetrics" (in IndexGroupMetrics) and "PerQuery" (in PerQueryMetrics).

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
*/
Expand Down
11 changes: 11 additions & 0 deletions src/java/org/apache/cassandra/tools/nodetool/stats/StatsTable.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...or we could just prefix them all with sai, which I was going to suggest anyway, given some of them are pretty general, like disk used bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Up @@ -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"))
Copy link
Contributor

Choose a reason for hiding this comment

The 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use compareFileSizes() here?

}
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For TotalIndexCount and TotalQueryableIndexCount, are we actually comparing anything here?

}
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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We should probably prefix these two with sai_ to be safe.

mpTable.put("sai_queryable_total_indexes", table.TotalQueryableIndexRatio);

return mpTable;
}

Expand Down Expand Up @@ -393,12 +404,53 @@ 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 totalQueryTimeouts = probe.getSAIIndexMetrics(keyspaceName, tableName, "TotalQueryTimeouts");
statsTable.TotalQueryTimeouts = (totalQueryTimeouts != null) ? (Long) totalQueryTimeouts : 0L;

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ protected void printStatsTable(StatsTable table, String tableDisplayName, String
for (Map.Entry<String, Long> tombstonecnt : table.topTombstonePartitions.entrySet())
out.printf(indent + " %-" + maxWidth + "s %s%n", tombstonecnt.getKey(), tombstonecnt.getValue());
}

out.println(indent + "SAI local query latency (mean): " + FBUtilities.prettyPrintLatency(table.QueryLatencyMs));
out.println(indent + "SAI post-filtering latency (mean): " + FBUtilities.prettyPrintLatency(table.PostFilteringReadLatencyms));
out.println(indent + "SAI space used (bytes): " + table.DiskUsedBytes);
out.println(indent + "SAI SSTable indexes hit per query (mean): " + table.SSTableIndexesHit);
out.println(indent + "SAI index segments hit per query (mean): " + table.IndexSegmentsHit);
out.println(indent + "SAI rows filtered per query (mean): " + table.RowsFiltered);
out.println(indent + "SAI local query timeouts: " + table.TotalQueryTimeouts);
out.println(indent + "SAI queryable/total indexes: " + table.TotalQueryableIndexRatio);

out.println("");
}

Expand Down
Loading