Maxwell-Guo commented on code in PR #2333:
URL: https://github.com/apache/cassandra/pull/2333#discussion_r1217364611
##########
src/java/org/apache/cassandra/tools/nodetool/CompactionStats.java:
##########
@@ -53,29 +58,82 @@ public class CompactionStats extends NodeToolCmd
public void execute(NodeProbe probe)
{
PrintStream out = probe.output().out;
+ out.print(pendingTasksAndConcurrentCompactorsStats(probe));
+ out.print(compactionsCompletedStats(probe));
+ out.print(compactionThroughPutStats(probe));
+ out.println();
CompactionManagerMBean cm = probe.getCompactionManagerProxy();
+ reportCompactionTable(cm.getCompactions(),
probe.getCompactionThroughputBytes(), humanReadable, vtableOutput, out);
+ }
+
+ private static String pendingTasksAndConcurrentCompactorsStats(NodeProbe
probe)
+ {
Map<String, Map<String, Integer>> pendingTaskNumberByTable =
(Map<String, Map<String, Integer>>)
probe.getCompactionMetric("PendingTasksByTableName");
- int numTotalPendingTask = 0;
+ StringBuffer toPrint = new StringBuffer();
+ toPrint.append(String.format("%s concurrent compactors, %s pending
tasks", probe.getConcurrentCompactors()
+ , numPendingTasks(pendingTaskNumberByTable)));
+ toPrint.append(LINE_SEPARATOR);
for (Entry<String, Map<String, Integer>> ksEntry :
pendingTaskNumberByTable.entrySet())
{
+ String ksName = ksEntry.getKey();
for (Entry<String, Integer> tableEntry :
ksEntry.getValue().entrySet())
- numTotalPendingTask += tableEntry.getValue();
+ {
+ toPrint.append("- " + ksName + '.' + tableEntry.getKey() + ":
" + tableEntry.getValue());
Review Comment:
Can we use String.format uniformly.
Although its performance is not very good, and actually I prefer to use
string concatenation.
But Cassandra seems to be all String.formt,So it seems that unity looks
better
##########
src/java/org/apache/cassandra/tools/nodetool/CompactionStats.java:
##########
@@ -53,29 +58,82 @@ public class CompactionStats extends NodeToolCmd
public void execute(NodeProbe probe)
{
PrintStream out = probe.output().out;
+ out.print(pendingTasksAndConcurrentCompactorsStats(probe));
+ out.print(compactionsCompletedStats(probe));
+ out.print(compactionThroughPutStats(probe));
+ out.println();
CompactionManagerMBean cm = probe.getCompactionManagerProxy();
+ reportCompactionTable(cm.getCompactions(),
probe.getCompactionThroughputBytes(), humanReadable, vtableOutput, out);
+ }
+
+ private static String pendingTasksAndConcurrentCompactorsStats(NodeProbe
probe)
+ {
Map<String, Map<String, Integer>> pendingTaskNumberByTable =
(Map<String, Map<String, Integer>>)
probe.getCompactionMetric("PendingTasksByTableName");
- int numTotalPendingTask = 0;
+ StringBuffer toPrint = new StringBuffer();
+ toPrint.append(String.format("%s concurrent compactors, %s pending
tasks", probe.getConcurrentCompactors()
+ , numPendingTasks(pendingTaskNumberByTable)));
+ toPrint.append(LINE_SEPARATOR);
for (Entry<String, Map<String, Integer>> ksEntry :
pendingTaskNumberByTable.entrySet())
{
+ String ksName = ksEntry.getKey();
for (Entry<String, Integer> tableEntry :
ksEntry.getValue().entrySet())
- numTotalPendingTask += tableEntry.getValue();
+ {
+ toPrint.append("- " + ksName + '.' + tableEntry.getKey() + ":
" + tableEntry.getValue());
+ toPrint.append(LINE_SEPARATOR);
+ }
}
- out.println("pending tasks: " + numTotalPendingTask);
+
+ return toPrint.toString();
+ }
+
+ private static int numPendingTasks(Map<String, Map<String, Integer>>
pendingTaskNumberByTable)
+ {
+ int numTotalPendingTasks = 0;
for (Entry<String, Map<String, Integer>> ksEntry :
pendingTaskNumberByTable.entrySet())
{
- String ksName = ksEntry.getKey();
for (Entry<String, Integer> tableEntry :
ksEntry.getValue().entrySet())
- {
- String tableName = tableEntry.getKey();
- int pendingTaskCount = tableEntry.getValue();
+ numTotalPendingTasks += tableEntry.getValue();
+ }
- out.println("- " + ksName + '.' + tableName + ": " +
pendingTaskCount);
- }
+ return numTotalPendingTasks;
+ }
+
+ private static String compactionsCompletedStats(NodeProbe probe)
+ {
+ Long completedTasks =
(Long)probe.getCompactionMetric("CompletedTasks");
+ CassandraMetricsRegistry.JmxMeterMBean
totalCompactionsCompletedMetrics =
+
(CassandraMetricsRegistry.JmxMeterMBean)probe.getCompactionMetric("TotalCompactionsCompleted");
+ NumberFormat formatter = new DecimalFormat("##.00");
+ StringBuffer toPrint = new StringBuffer();
+ toPrint.append(String.format("compactions completed: %s",
completedTasks));
+ toPrint.append(LINE_SEPARATOR);
+ toPrint.append(String.format("\tminute rate: %s/second",
formatter.format(totalCompactionsCompletedMetrics.getOneMinuteRate())));
+ toPrint.append(LINE_SEPARATOR);
+ toPrint.append(String.format("\t5 minute rate: %s/second",
formatter.format(totalCompactionsCompletedMetrics.getFiveMinuteRate())));
+ toPrint.append(LINE_SEPARATOR);
+ toPrint.append(String.format("\t15 minute rate: %s/second",
formatter.format(totalCompactionsCompletedMetrics.getFifteenMinuteRate())));
+ toPrint.append(LINE_SEPARATOR);
+ toPrint.append(String.format("\tMean rate: %s/second",
formatter.format(totalCompactionsCompletedMetrics.getMeanRate())));
+ toPrint.append(LINE_SEPARATOR);
+
+ return toPrint.toString();
+ }
+
+ private static String compactionThroughPutStats(NodeProbe probe)
+ {
+ double configured = probe.getCompactionThroughputMebibytesAsDouble();
+ double actual = probe.getCompactionRate() / (1024 * 1024);
+ if(configured == 0)
+ {
+ return String.format("compaction throughput absolute: %s MBps",
actual);
}
- out.println();
- reportCompactionTable(cm.getCompactions(),
probe.getCompactionThroughputBytes(), humanReadable, vtableOutput, out);
+ else
+ {
+ double percentage = (actual / configured) * 100;
Review Comment:
I think I would change the code to :` double configured =
probe.getCompactionThroughputMebibytesAsDouble();
if(configured == 0)
{
return String.format("compaction throughput absolute: %s MBps",
actual);
}
else
{
double actual = probe.getCompactionRate() / (1024 * 1024);
double percentage = (actual / configured) * 100;
return String.format("compaction throughput ratio: %s MBps / %s
MBps (%s%s)", actual, configured, percentage, "%");
}`
if configured is 0, there is no need to calculate the value of actual.
##########
test/unit/org/apache/cassandra/tools/nodetool/CompactionStatsTest.java:
##########
@@ -130,6 +130,14 @@ compactionId, OperationType.COMPACTION,
CQLTester.KEYSPACE, currentTable(), byte
CompactionInfo.Unit.BYTES,
(double) bytesCompacted / bytesTotal * 100);
Assertions.assertThat(stdout).containsPattern(expectedStatsPattern);
+ Assertions.assertThat(stdout).containsPattern("[0-9]* concurrent
compactors, [0-9]* pending tasks");
Review Comment:
I think to be on the safe side, you can add a test case that uses the
invokeNodetool method for full-link testing.
Though when using invokeNodetool from a client side , It is impossible to
specifically determine the value of the test, but we can test and output some
expected results, such as whether the output information has the print out
field we need, and when there is no data, and when we write a certain amount of
data then do a major, These print out data should met our expectations。
@driftx WDYT?
##########
src/java/org/apache/cassandra/tools/nodetool/CompactionStats.java:
##########
@@ -53,29 +58,82 @@ public class CompactionStats extends NodeToolCmd
public void execute(NodeProbe probe)
{
PrintStream out = probe.output().out;
+ out.print(pendingTasksAndConcurrentCompactorsStats(probe));
+ out.print(compactionsCompletedStats(probe));
+ out.print(compactionThroughPutStats(probe));
+ out.println();
CompactionManagerMBean cm = probe.getCompactionManagerProxy();
+ reportCompactionTable(cm.getCompactions(),
probe.getCompactionThroughputBytes(), humanReadable, vtableOutput, out);
+ }
+
+ private static String pendingTasksAndConcurrentCompactorsStats(NodeProbe
probe)
+ {
Map<String, Map<String, Integer>> pendingTaskNumberByTable =
(Map<String, Map<String, Integer>>)
probe.getCompactionMetric("PendingTasksByTableName");
- int numTotalPendingTask = 0;
+ StringBuffer toPrint = new StringBuffer();
+ toPrint.append(String.format("%s concurrent compactors, %s pending
tasks", probe.getConcurrentCompactors()
+ , numPendingTasks(pendingTaskNumberByTable)));
Review Comment:
I think there is a problem of code format
line 75 should have some space before ',' , so that In this way “，” can be
aligned with "%$"
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
