[
https://issues.apache.org/jira/browse/CASSANDRA-16959?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17419061#comment-17419061
]
Aleksei Zotov edited comment on CASSANDRA-16959 at 9/23/21, 9:08 AM:
---------------------------------------------------------------------
[~adelapena]
I put a couple of minor comments to the PR. 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}The tiny tests for {{nodetool
setstreamthroughput/setinterdcstreamthroughput}} are mine.
{quote}
The tests are great actually!
was (Author: azotcsit):
[~adelapena]
I put a couple of minor comments to the PR.
{quote}The tiny tests for {{nodetool
setstreamthroughput/setinterdcstreamthroughput}} are mine.
{quote}
The tests are great actually!
> 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: 40m
> 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: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
