[HF 9.3.13] CLDSRV-979: rate limit pinned request logger - #6263
Open
tmacro wants to merge 5 commits into
Open
Conversation
WorkerTokenBucket stored the werelogs logger of the first request that
touched a resource, and the 100ms refill job then logged through it for
the lifetime of the process. RequestLogger buffers every entry it is
handed in RequestLogger.entries and only drains when something logs at
or above the dump threshold ('error'), so the refill chatter accumulated
forever - about 80MB per account per connector on a 10-worker deployment,
reclaimed only by restarting cloudserver. S3C runs at logLevel info, so
the buffered debug/trace lines were never even printed.
The logger is now supplied per call to refillIfNeeded() and never stored.
The refill job passes the long-lived server logger, which writes through
and drops sub-level entries instead of buffering them.
The existing tests could not have caught this: they all pass a sinon
stub as the logger, so the werelogs buffering that is the bug is never
exercised. The new retention tests assert the invariants directly - no
retained request logger, and refills logged through the caller's logger.
(cherry picked from commit 3c9879b)
The unit environment has no rate limit Redis instance (the feature is disabled at Config load), so every refill test bounced off isReady() into the catch block and the grant, denial, disconnected and slow paths were never executed - codecov flagged exactly those lines. tokenBucket now reads rateLimitClient.instance at call time instead of destructuring it at module load, which is behaviour-identical in production (the instance is created once, before the first request) and lets tests substitute a fake client. Five new cases cover each outcome; tokenBucket.js line coverage goes from 83% to 98%, leaving only the defensive requested <= 0 guard, unreachable while refillThreshold is below bufferSize. (cherry picked from commit 04359d0)
(cherry picked from commit 2716d81)
Contributor
Hello tmacro,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
❌ 2 Tests Failed:
View the top 1 failed test(s) by shortest run time
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
tcarmet
approved these changes
Aug 21, 2026
(cherry picked from commit 37c2945)
tmacro
force-pushed
the
bugfix/CLDSRV-979-rate-limit-pinned-request-logger_bp_9_3_13
branch
from
August 21, 2026 18:59
8d04831 to
2b3069b
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+37
to
+44
| const promise = bucket | ||
| .refillIfNeeded(logger) | ||
| .then(bucketRefilled => { | ||
| // Check if refill actually happened | ||
| if (bucketRefilled) { | ||
| refilled++; | ||
| } | ||
| }) |
Contributor
Author
|
/approve |
Contributor
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.