Feature/ccm 16073 rate limit it fix#166
Feature/ccm 16073 rate limit it fix#166mjewildnhs merged 11 commits intofeature/CCM-16073-rate-limitfrom
Conversation
Additional handler logging and observability Fix retry time for partial batch rate limiting Add follow option to debug test script Add since var to int debug script Fix issue when half open and all probes fail High resolution storage metrics
mjewildnhs
force-pushed
the
feature/CCM-16073-rate-limit-it-fix
branch
from
9c573aa to
34af71e
Compare
This reverts commit 5d19fcca6531e3c7de3ffb4f8cd816db38f03ac1.
mjewildnhs
force-pushed
the
feature/CCM-16073-rate-limit-it-fix
branch
from
e641be8 to
bbfc5d7
Compare
| if isOpen then | ||
| if not cbEnabled then | ||
| effectiveRate = targetRateLimit | ||
| elseif isOpen then |
There was a problem hiding this comment.
I think we can incorporate cbEnabled more nicely than this... We should only have to:
- bring up new circuit state as closed (and not recovering)
- disable the circuit open/close mechanisms
That should be enough.
There was a problem hiding this comment.
CB/RL design docs have been updated, so this is not needed any more.
There was a problem hiding this comment.
I've incorporated the changes.
With 2 additional fixes for fresh state no is_open in Redis yet:
-
(lines 95-97): When the circuit is open on a fresh endpoint,
bucketTokensis zeroed andbucketRefilledAtis set tonow, producing 0 generated tokens. Without seeding 1 probe token, nothing is ever processed,record-resultnever runs,is_openis never written, and the endpoint is permanently blocked. -
bucketRefilledAtindependent check:needInitis derived fromis_openexistence, but admit.lua never writesis_open— onlyrecord-result.luadoes. With CB disabled,needInitstays true across calls, resettingbucketRefilledAt = nowevery time and producing 0 tokens indefinitely. The fix checksbucket_refilled_at's own raw value so it reads back what admit wrote on the previous call.
| -- LOAD STATE | ||
| -------------------------------------------------------------------------------- | ||
|
|
||
| local cbEnabled = probeRateLimit > 0 |
There was a problem hiding this comment.
Isn't this a separate, per-target config param which should be passed into the script?
There was a problem hiding this comment.
Probe rate is a global env var currently
There was a problem hiding this comment.
No, I mean cbEnabled. Isn't this a per-target config which is passed in?
2 participants
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.