Skip to content
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.

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

improve logging of rate limits #18907

Merged
merged 2 commits into from
Nov 21, 2024
Merged

improve logging of rate limits #18907

merged 2 commits into from
Nov 21, 2024

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Nov 20, 2024

Purpose:

Our rate limits sometimes have hard-to-predict behaviors. Trouble shooting is made more difficult by only logging at debug level when an outgoing message is dropped.

Current Behavior:

Outgoing messages being dropped by the rate limit is only logged at debug level, even though this should be a rare event.

New Behavior:

Dropping outgoing messages is logged at info-level along with which limit was exceeded, causing the message to drop.
When disconnecting a peer for sending a message exceeding the incoming rate limit, we print which limit was exceeded.

Testing Notes:

I've tested this by syncing two nodes on my local network.

@arvidn arvidn requested a review from altendky November 20, 2024 11:40
@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Nov 20, 2024
@arvidn arvidn force-pushed the rate-limit-logging branch from 4eea3d7 to 658475e Compare November 20, 2024 12:28
@arvidn arvidn marked this pull request as ready for review November 20, 2024 13:21
@arvidn arvidn requested a review from a team as a code owner November 20, 2024 13:21
chia/server/rate_limits.py Outdated Show resolved Hide resolved
chia/server/rate_limits.py Outdated Show resolved Hide resolved
chia/server/rate_limits.py Outdated Show resolved Hide resolved
chia/server/ws_connection.py Outdated Show resolved Hide resolved
chia/server/ws_connection.py Outdated Show resolved Hide resolved
@arvidn arvidn force-pushed the rate-limit-logging branch from 2c61993 to 2f212cf Compare November 20, 2024 15:24
altendky
altendky previously approved these changes Nov 20, 2024
Copy link
Contributor

File Coverage Missing Lines
chia/server/rate_limits.py 88.9% lines 103
Total Missing Coverage
60 lines Unknown 98%

@pmaslana pmaslana merged commit e17e657 into main Nov 21, 2024
362 of 363 checks passed
@pmaslana pmaslana deleted the rate-limit-logging branch November 21, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants