You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "maoling (via GitHub)" <gi...@apache.org> on 2024/06/12 13:43:57 UTC
[PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
maoling opened a new pull request, #3373:
URL: https://github.com/apache/cassandra/pull/3373
- more details in the [CASSANDRA-13890](https://issues.apache.org/jira/browse/CASSANDRA-13890)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "maoling (via GitHub)" <gi...@apache.org>.
maoling closed pull request #3373: CASSANDRA-13890: Expose current compaction throughput
URL: https://github.com/apache/cassandra/pull/3373
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #3373:
URL: https://github.com/apache/cassandra/pull/3373#discussion_r1637797924
##########
src/java/org/apache/cassandra/tools/nodetool/GetCompactionThroughput.java:
##########
@@ -38,13 +40,18 @@ public void execute(NodeProbe probe)
double throughput = probe.getCompactionThroughputMebibytesAsDouble();
if (compactionThroughputAsDouble)
- probe.output().out.println("Current compaction throughput: " + throughput + " MiB/s");
+ probe.output().out.println("Current compaction throughput limit: " + throughput + " MiB/s");
else
{
if (!DoubleMath.isMathematicalInteger(throughput))
throw new RuntimeException("Use the -d flag to quiet this error and get the exact throughput in MiB/s");
- probe.output().out.println("Current compaction throughput: " + probe.getCompactionThroughput() + " MB/s");
+ probe.output().out.println("Current compaction throughput limit: " + probe.getCompactionThroughput() + " MB/s");
}
+
+ Map<String, String> currentCompactionThroughputMetricsMap = probe.getCurrentCompactionThroughput();
Review Comment:
`MiB/s`
##########
test/unit/org/apache/cassandra/tools/nodetool/CompactionStatsTest.java:
##########
@@ -140,6 +140,9 @@ compactionId, OperationType.COMPACTION, CQLTester.KEYSPACE, currentTable(), byte
assertThat(stdout).containsPattern("15 minute rate\\s+[0-9]*.[0-9]*[0-9]*/minute");
assertThat(stdout).containsPattern("mean rate\\s+[0-9]*.[0-9]*[0-9]*/hour");
assertThat(stdout).containsPattern("compaction throughput \\(MiB/s\\)\\s+throttling disabled \\(0\\)");
+ assertThat(stdout).containsPattern("current compaction throughput \\(1 minute\\)\\s+[0-9]*.[0-9]*[0-9]* MB/s");
Review Comment:
`MiB/s`
##########
test/unit/org/apache/cassandra/tools/nodetool/SetGetCompactionThroughputTest.java:
##########
@@ -81,6 +81,17 @@ public void testUnparseable()
assertPreciseMibFlagNeeded();
}
+ @Test
+ public void testCurrentCompactionThroughput()
+ {
+ ToolResult tool = invokeNodetool("getcompactionthroughput");
+ tool.assertOnCleanExit();
+
+ assertThat(tool.getStdout()).containsPattern("Current compaction throughput \\(1 minute\\): \\d+\\.\\d+ MB/s");
Review Comment:
`MiB/s`
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #3373:
URL: https://github.com/apache/cassandra/pull/3373#discussion_r1637764082
##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -1415,6 +1416,17 @@ public void setCompactionThroughputMbPerSec(int value)
value, oldValue);
}
+ public Map<String, String> getCurrentCompactionThroughput()
+ {
+ HashMap<String, String> result = new LinkedHashMap<>();
+ Meter rate = CompactionManager.instance.getCompactionThroughput();
+ double mb = 1024.0 * 1024.0;
Review Comment:
we have `org.apache.cassandra.io.util.FileUtils.ONE_MIB` to use. You would just make double from it if necessary.
##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -1415,6 +1416,17 @@ public void setCompactionThroughputMbPerSec(int value)
value, oldValue);
}
+ public Map<String, String> getCurrentCompactionThroughput()
+ {
+ HashMap<String, String> result = new LinkedHashMap<>();
+ Meter rate = CompactionManager.instance.getCompactionThroughput();
+ double mb = 1024.0 * 1024.0;
Review Comment:
@maoling we have `org.apache.cassandra.io.util.FileUtils.ONE_MIB` to use. You would just make double from it if necessary.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #3373:
URL: https://github.com/apache/cassandra/pull/3373#discussion_r1637797659
##########
src/java/org/apache/cassandra/tools/nodetool/CompactionStats.java:
##########
@@ -111,6 +111,10 @@ private void compactionsStats(NodeProbe probe, TableBuilder tableBuilder)
double configured = probe.getStorageService().getCompactionThroughtputMibPerSecAsDouble();
tableBuilder.add("compaction throughput (MiB/s)", configured == 0 ? "throttling disabled (0)" : Double.toString(configured));
+ Map<String, String> currentCompactionThroughputMetricsMap = probe.getCurrentCompactionThroughput();
+ tableBuilder.add("current compaction throughput (1 minute)", currentCompactionThroughputMetricsMap.get("1minute") + " MB/s");
Review Comment:
`MiB/s`
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #3373:
URL: https://github.com/apache/cassandra/pull/3373#discussion_r1637757464
##########
src/java/org/apache/cassandra/db/compaction/CompactionManager.java:
##########
@@ -1520,6 +1527,16 @@ static void compactionRateLimiterAcquire(RateLimiter limiter, long bytesScanned,
}
}
+ private long getLengthRead(long bytesScanned, long lastBytesScanned, double compressionRatio)
Review Comment:
@maoling do we need these two methods? they are just oneliners and not used anywhere else. Just inline it into `compactionRateLimiterAcquire` I guess?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #3373:
URL: https://github.com/apache/cassandra/pull/3373#discussion_r1637797371
##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -1415,6 +1416,17 @@ public void setCompactionThroughputMbPerSec(int value)
value, oldValue);
}
+ public Map<String, String> getCurrentCompactionThroughput()
+ {
+ HashMap<String, String> result = new LinkedHashMap<>();
+ Meter rate = CompactionManager.instance.getCompactionThroughput();
+ double mb = 1024.0 * 1024.0;
+ result.put("1minute", String.format("%.3f", rate.getOneMinuteRate() / mb));
+ result.put("5minute", String.format("%.3f", rate.getFiveMinuteRate() / mb));
+ result.put("15minute", String.format("%.3f", rate.getFifteenMinuteRate() / mb));
Review Comment:
arent these technically mebibytes, not megabytes? 1024*1024 bytes is one mebibyte, not megabyte, so the unit should be "mib"
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-13890: Expose current compaction throughput [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #3373:
URL: https://github.com/apache/cassandra/pull/3373#discussion_r1637794106
##########
src/java/org/apache/cassandra/service/StorageService.java:
##########
@@ -1415,6 +1416,17 @@ public void setCompactionThroughputMbPerSec(int value)
value, oldValue);
}
+ public Map<String, String> getCurrentCompactionThroughput()
Review Comment:
please javadoc this method, what will a user get in the map? Name of the method might reflect that units are MBs per second.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org