You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Aleksei Zotov (Jira)" <ji...@apache.org> on 2021/09/23 09:09:00 UTC
[jira] [Comment Edited] (CASSANDRA-16959) nodetool
setstreamthroughput accepts invalid arguments that are not immediately
applied
[ 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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org