[
https://issues.apache.org/jira/browse/CASSANDRA-17725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17561476#comment-17561476
]
Ekaterina Dimitrova commented on CASSANDRA-17725:
-------------------------------------------------
interdcstreamthroughput flags added in this
[commit|https://github.com/ekaterinadimitrova2/cassandra/commit/d5e7134c99e5e938b74c2ad74fbf2c916f0785ce]
and streamthroughput flag added in this
[commit|https://github.com/ekaterinadimitrova2/cassandra/commit/9c6e1452a939ad5420daa5eefce33ac5b6e050ec]
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?
I had a few findings which I addressed in separate commits:
* [several changes in
output|https://github.com/ekaterinadimitrova2/cassandra/commit/92e0c3948fc65b29fbe289814f84aaf603188152]
- I tried instead to clarify the unit in the description
*
[cassandra.yaml|https://github.com/ekaterinadimitrova2/cassandra/commit/153ccb6dfff630b2229bbcc5c92d201e5e5d2616]
- there were a few parameters which were commented for testing and that was
not reverted. It doesn't break anything as then we are using the same default
value which is in DatabaseDescriptor but it is not needed change and makes it
confusing for the users
* I believe this
[withBufferSizeInMB|https://github.com/ekaterinadimitrova2/cassandra/commit/d16170fd6a15f2ec20acd057e8f44ec5ded39094]
method had to be deprecated and not just removed
And a few topics I am adding for discussion:
-
[https://github.com/apache/cassandra/commit/5bb4bab12f8edfef95ed13cbabf8c0f377986065#diff-f7dd0237c343649f70b7ec9fefd7f6941a40b5164fd6063dce00fc09d2c234a7L163-R163]
- method which is not exposed by the mbean but it is public; is this a
breaking change?
*
[https://github.com/apache/cassandra/commit/c51a7c66fc21ca2da08b89ae5f9b4817ee4d8c23#diff-2314788f556b14ab8cd9c4cf4eba04f4b292f0b9c93d74919eed33a0af42ababL279-L280]
—> breaking change?? - I doubt it but double-checking. I can revert it just to
be on the safe side
And a bug I found that I want to ask [~jolynch] for advise as he was involved
into adding that property:
[https://github.com/apache/cassandra/commit/9f56bf4ca7fdb61ad09e5f2ad09b87cd01e0716b#diff-77707d0908c31940828b6425dcb09a7409827db99b48c371f71c63294dfe1562L444-R444]
—> please check, I believe this change is a bug. The Converter used for that
property with the pre-4.0 format does not allow negatives and we will always
have 0 in AutoSavingCache where we check for negatives and not 0. The test
where this was set to -1 is testKeyCacheLoadZeroCacheLoadTime and it works
without an issue with 0. . For [~jolynch] - long story short we prohibited the
usage of negatives as it was considered a bug in 4.1. But it seems different
here with this property. If negatives and 0 will do the same (I am not sure,
didn’t dive too much into that patch), maybe we can use the
_NEGATIVE_SECONDS_DURATION_ converter from the Converters class we added for
validation_preview_purge_head_start which will convert a negative number to 0
in 4.1 and tell people from now on will use 0 to disable but at the same time
if they upgrade with negative and the old value format and property name, they
will be able to set negative number which will be migrated internally to 0. I
am really not completely sure, what do others think?
CI running here for the 4.1 branch:
[j8|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1767/workflows/5f62f5d2-0930-4084-9031-61e1087b22fb],
[j11|https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1767/workflows/781c87d6-9ea1-45ad-838e-53e0d9ee50ee]
[~maedhroz] , [~dcapwell] , [~mck] , can I ask you for review, please?
> 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: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
