Skip to content

Implement the inbound stream throuttler #3683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Runtian wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Implement the inbound stream throuttler #3683

Runtian wants to merge 1 commit into from

Conversation

@Runtian
Copy link
Contributor

@Runtian Runtian commented Nov 13, 2024

Thanks for sending a pull request! Here are some tips if you're new here:

  • Ensure you have added or run the appropriate tests for your PR.
  • Be sure to keep the PR description updated to reflect all changes.
  • Write your PR title to summarize what this PR proposes.
  • If possible, provide a concise example to reproduce the issue for a faster review.
  • Read our contributor guidelines
  • If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

@Runtian Runtian force-pushed the CASSANDRA-11303-trunk branch from d694ef4 to 4fe47fd Compare January 17, 2025 00:15
@belliottsmith belliottsmith force-pushed the trunk branch 2 times, most recently from df3eb40 to 54e39a9 Compare July 23, 2025 11:19
clohfink
clohfink requested changes Aug 21, 2025
View reviewed changes
Copy link
Contributor

@clohfink clohfink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple minor nitpicks

src/java/org/apache/cassandra/streaming/StreamManager.java
private static final RateLimiter ENTIRE_SSTABLE_LIMITER = RateLimiter.create(calculateEntireSSTableRateInBytes());
private static final RateLimiter ENTIRE_SSTABLE_INTER_DC_LIMITER = RateLimiter.create(calculateEntireSSTableInterDCRateInBytes());
private static final RateLimiter INBOUND_LIMITTER = RateLimiter.create(calculateInboundRateInBytes());
private static final RateLimiter ENTIRE_SSTABLE_INBOUND_LIMITTER = RateLimiter.create(calculateEntireSSTableInboundRateInBytes());
Copy link
Contributor

@clohfink clohfink Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: LIMITER for both

src/java/org/apache/cassandra/db/streaming/CassandraCompressedStreamReader.java
try (CompressedInputStream cis = new CompressedInputStream(inputPlus, compressionInfo, ChecksumType.CRC32, cfs::getCrcCheckChance))
{
TrackedDataInputPlus in = new TrackedDataInputPlus(cis);
RateLimitedTrackedDataInputPlus in = new RateLimitedTrackedDataInputPlus(cis, -1, limiter, totalSize, 1 << 16);
Copy link
Contributor

@clohfink clohfink Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the 64kb is kinda hard coded in all places using this right? maybe make it a constant and overridable with yaml or system property

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clohfink clohfink clohfink requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Runtian @clohfink