Skip to content

Improve client ip rate limits - #6853

Merged
nicu-da merged 2 commits into
mainfrom
nicu/rate_limits/improvements
Aug 19, 2026
Merged

Improve client ip rate limits#6853
nicu-da merged 2 commits into
mainfrom
nicu/rate_limits/improvements

Conversation

@nicu-da

@nicu-da nicu-da commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Add ability to disable fallback for headers
Remove default rate limit when no ip is known and instead just count it through a metric (we count on the global rate limiter to avoid SVs that don't extract the ip correctly to just rate limit everyone more aggressivly)

[ci]

Pull Request Checklist

Cluster Testing

  • If a cluster test is required, comment /cluster_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If an upgrade test is required, comment /upgrade_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If a hard-migration test is required (from the latest release), comment /hdm_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If a logical synchronizer upgrade test is required (from canton-3.5), comment /lsu_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.

PR Guidelines

  • Include any change that might be observable by our partners or affect their deployment in the release notes.
  • Specify fixed issues with Fixes #n, and mention issues worked on using #n
  • Include a screenshot for frontend-related PRs - see README or use your favorite screenshot tool

Merge Guidelines

  • Make the git commit message look sensible when squash-merging on GitHub (most likely: just copy your PR description).

@martinflorian-da martinflorian-da left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@nicu-da
nicu-da enabled auto-merge (squash) August 19, 2026 10:46
Add ability to disable fallback for headers
Remove default rate limit when no ip is known and instead just count it through a metric (we count on the global rate limiter to avoid SVs that don't extract the ip correctly to just rate limit everyone more aggressivly)

[ci]

Signed-off-by: Nicu Reut <[email protected]>
@nicu-da
nicu-da force-pushed the nicu/rate_limits/improvements branch from 599d3ae to 565abe6 Compare August 19, 2026 11:28
.
[ci]

Signed-off-by: Nicu Reut <[email protected]>
@nicu-da
nicu-da merged commit 306014b into main Aug 19, 2026
61 checks passed
@nicu-da
nicu-da deleted the nicu/rate_limits/improvements branch August 19, 2026 13:20
@stas-sbi

Copy link
Copy Markdown
Contributor

I would expect the headers be disabled by default. As an operator I can't disable something if I don't know it exists. I can't be sure more headers won't be added in the future either. Vice-versa, if I need the app to handle these headers I know this and I'll go to configs or docs to figure if the app supports it and how to enable.

@nicu-da

nicu-da commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

I would expect the headers be disabled by default.

The per ip rate limits should be enabled by default therefore the reasoning for the default headers as well.

As an operator I can't disable something if I don't know it exists. I can't be sure more headers won't be added in the future either.

I would expect that to be the scope for release notes.

Vice-versa, if I need the app to handle these headers I know this and I'll go to configs or docs to figure if the app supports it and how to enable.

That would go against the per ip rate limits are enabled by default.

The other reasoning is that these not being configured correct is low risk. Either you get the wrong value and rate limits gets enforced more aggressive, which would lead us to identify that issue, or users can bypass them and we would see that in the overall rate, and global rate limits are there to protect against such issues.

@giner

giner commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

... or users can bypass them and we would see that in the overall rate, and global rate limits are there to protect against such issues.

This leaves misconfiguration as a possibility (possibly unnoticed) while the software could force an operator to be tuned based on the operator's infra-setup explicitly.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants