[
https://issues.apache.org/jira/browse/CASSANDRA-16959?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17422220#comment-17422220
]
Andres de la Peña commented on CASSANDRA-16959:
-----------------------------------------------
Thanks for the feedback, I have tried to address all the review comments.
{quote}I do think we should improve both log messages, as that not only
provides needed context, but these messages appear to be outliers for not
displaying a unit where applicable. If some tooling out there depends on this
output, adding a unit will be an easy fix, or it can grow up and use JMX
directly so that's not necessary.
{quote}
Makes sense, I have updated both messages to say {{throttle set to {} Mb/s (was
{} Mb/s)}}.
{quote}A quick question: what versions are you going to apply the fix to? I can
see many versions mentioned in "fix version" field, however, the patch is for
trunk only.
{quote}
Here are all the PRs for the other branches:
||PR||CI||
|[3.0|https://github.com/apache/cassandra/pull/1236]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/925/workflows/d968e026-24a9-49f0-8fa7-69632ed5293d]|
|[3.11|https://github.com/apache/cassandra/pull/1237]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/923/workflows/7908af36-0777-4964-a34c-b2e15d270046]|
|[4.0|https://github.com/apache/cassandra/pull/1238]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/924/workflows/0568f57e-8824-42be-9648-586f7da8c78d]
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/924/workflows/0d2f7db6-ea04-48bf-8aee-b05841623bac]|
|[trunk|https://github.com/apache/cassandra/pull/1202]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/922/workflows/99aa78a6-e631-40df-af64-09b8ceda6f69]
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/922/workflows/c2aa41b5-c15c-484a-a30f-d7f2185c7dd9]|
Unfortunately, in the case of 3.0 and 3.11 we don't have
[{{ToolRunner}}|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/tools/ToolRunner.java]
available so the tests for the nodetool commands can't be backported as they
are. In those branches we still have {{StreamManagerTest}} doing something
quite similar, but I can add some dtests with a {{@since}} annotation if you
think it is worth it.
> nodetool setstreamthroughput accepts invalid arguments that are not
> immediately applied
> ---------------------------------------------------------------------------------------
>
> Key: CASSANDRA-16959
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16959
> Project: Cassandra
> Issue Type: Bug
> Components: Tool/nodetool
> Reporter: Andres de la Peña
> Assignee: Andres de la Peña
> Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Both {{nodetool setstreamthroughput}} and {{nodetool
> setinterdcstreamthroughput}} accept a negative throughput. The throughput
> value is not immediately applied to the corresponding rate limiters. Instead,
> the value is [set in the
> {{Config}}|https://github.com/apache/cassandra/blob/09c89e5f5f8604301c233130dfb6e82a36ae30f3/src/java/org/apache/cassandra/service/StorageService.java#L1488]
> and it's only applied to the singleton rate limiter when new sstable stream
> writer are created (see
> [here|https://github.com/apache/cassandra/blob/09c89e5f5f8604301c233130dfb6e82a36ae30f3/src/java/org/apache/cassandra/streaming/StreamManager.java#L66-L76]).
> This could happen much later than the definition of the new throughput, and
> by then the setting of the new rate in the rate limiter will fail with an
> {{IllegalArgumentException}} due to the negative value.
> I think we should either immediately reject negative throughputs or consider
> them unlimited, as we do with zero. Also we should probably apply the new
> throughput to the rate limiter immediately, since I don't see why we should
> wait to start using the new throughput.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
