[
https://issues.apache.org/jira/browse/CASSANDRA-17725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17563982#comment-17563982
]
Caleb Rackliffe commented on CASSANDRA-17725:
---------------------------------------------
bq. The only thing I am wondering here is how to handle that nodetool and the
StorageService JMX methods are rounding to int and 0 very small values in
megabits to MiB/s (the getters in MiB, when we provide too small megabits) I
added in nodetool a check that will say instead of unlimited - 1 MiB/s. I plan
to document that when you use one or the other unit you should use the
respective setter/getter as otherwise you might see that side effect. Seeing 0
in MiB - someone might decide that they are unthrottling when in reality they
have super low value in megabits internally. What do others think about this?
Why don't we just simplify things and leave MiB/s values as doubles everywhere?
If we do that for JMX and {{nodetool}}, can't we just sidestep the rounding
ambiguity almost entirely? (I mean, we still need to decide on how many
decimals we'll report, but we can probably do 2 or 3 and call it a day.) I
don't see any reason to force a rounded/integer output when it would only
materialize in conjunction with our new flag.
Other minor notes from my pass at review:
- In the {{MEGABITS_TO_MEBIBYTES_PER_SECOND_DATA_RATE}} JavaDoc, "megatibs" ->
"megabits"?
- There's enough in common that it might be nice to combine
{{SetGetInterDCStreamThroughputMiBTest}} and
{{SetGetInterDCStreamThroughputTest}} into a single
{{InterDCStreamThroughputConfigTest}}. (Same goes for the non-inter-DC
versions.)
- In {{StorageService}}, we have {{int oldValue = (int)
DatabaseDescriptor.getStreamThroughputOutboundMebibytesPerSec();}}. Why not
just get the existing value as a double and log it that way? (Happens in
{{setStreamThroughputMebibytesPerSec}} and
{{setInterDCStreamThroughputMebibytesPerSec}}.)
- Given we're going to a new major version, did we really have to change
{{MiB}} back to {{MB}} in [this
commit|https://github.com/ekaterinadimitrova2/cassandra/commit/92e0c3948fc65b29fbe289814f84aaf603188152]?
Even if someone was parsing tool output, they still would only break if they
explicitly validated "MB" rather than just making sure to take the numeric
value, right?
- Two things on {{"-i", "--stream_throughput_mib"}}. First, is the normal
pattern to use hyphens instead of underscores for the long form? Second, given
this is a modifier on the default argument, why not just use something like
{{"-m", "--mib"}}?
> Add a flag for throughput in MiB/s for nodetool setstreamthroughput,
> getstreamthroughput, setinterdcstreamthroughput and
> getinterdcstreamthroughput
> ----------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-17725
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17725
> Project: Cassandra
> Issue Type: Bug
> Components: Tool/nodetool
> Reporter: Ekaterina Dimitrova
> Assignee: Ekaterina Dimitrova
> Priority: Normal
> Fix For: 4.1-beta, 4.x
>
>
> As we agreed not to add new JMX methods for the new config on the mailing
> list, we need at least new flags for setstreamthroughput and
> interdcstreamthroughput for the two 4.0 parameters to be set/get also in MiB,
> not only in megabits.
> Thus we will have the option either to use the old version for those 2, or to
> be able to set/get in MiB all 4 streaming parameters. As of 4.1 supported
> units for DataRateSpec are MiB/s, B/s, KiB/s, megabit is only for legacy from
> 4.0 - backward compatibility.
> To be sure we satisfy the requirements around the latest discussions about
> backward compatibility in tools, I will use this ticket also to make a final
> pass on the unit changes done, to ensure the probe output is not affected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
