log: exempt all category-specific logs from ratelimiting when running with debug #34018
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34018. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
stickies-v
changed the title
|
Concept NAck Users that are having issues that are not power users would also use the |
Debug logs are (by design) much higher frequency than info/warning/error level logs. They have never been ratelimited, because for debugging purposes it's important to see what's happening without limitation. When running with Info logs are typically much higher signal than debug logs, that's why they're unconditional. Before this PR, if an info/warning/error log has a category associated with it (note: this is quite rare), and that category is activated for debugging, the user would find their higher-signal unconditional logs ratelimited, but not their debug/trace logs. That's unintuitive, unhelpful and doesn't really offer any additional protection, because debug logs are so much more voluminous. It also leads to more elaborate workarounds necessary such as in #34008. I hope that clears it up? |
This test is added just to ensure current behaviour.
Previously, running with -debug=<category> would exempt the debug and trace log messages in that category from rate limiting, but not the info/warning/error category-specific messages (which are rare). This is unintuitive and unnecessary. When users run with -debug, we already assume they are power users and that they will have significantly higher log volumes, so there is no real benefit from suppressing info log messages in that category. Fix this by applying ratelimiting exceptions from -debug=<category> to all logs in that category.
This way, when running with -debug=net, the log message (which can be frequent) will not get ratelimited when the user explicitly opts into getting higher log volumes. Introduces slight behaviour change by prefixing the log message with [net:info].
stickies-v
force-pushed
the
2025-12/ratelimit-exempt-entire-category
branch
from
d8c3c5e to
9485a3c
Compare
|
Force-pushed to address merge conflict from #29641, no other changes. |
|
|
Yeah the alternative option for those work arounds are not great. Having debug on should just get you less restrictions from a functionality standpoint as well. |
|
Closing this in favour of #34008, I think that PR's new approach of downgrading the inbound log to I think the approach in this PR makes sense if there are other places in the code with a similar issue (modulo addressing this comment), but until that use case pops up, there's no point keeping this PR open. |
I went ahead and implemented my comment in #34033, because it makes sense on its own, and because it makes it easier to implement the changes here in the future, if there is ever need to. |
4 participants
Previously, running with
-debug=<category>would exempt the debug and trace log messages in that category from rate limiting, but not the info/warning/error category-specific messages (which are rare). This is unintuitive and unnecessary.When users run with
-debug, we already assume they are power users and that they will have significantly higher log volumes, so there is no real benefit from suppressing info log messages in that category.Fix this by applying ratelimiting exceptions from
-debug=<category>to all logs in that category.Also updates
net_processingto log new peer connections withNETcategory. This way, when running with-debug=net, the log message (which can be frequent) will not get ratelimited when the user explicitly opts into getting higher log volumes for net.Introduces slight behaviour change by prefixing the log message with [net:info].
Alternative to #34008