feature: admissionControl filter #2030
Merged
Conversation
This file contains 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
szuecs
force-pushed
the
feature/admission-control-filter
branch
3 times, most recently
from
5ab2f3a
to
ce6eeea
Compare
szuecs
force-pushed
the
feature/admission-control-filter
branch
3 times, most recently
from
0346e82
to
a13acbc
Compare
szuecs
changed the title
Comment on lines
+185
to
+188
| epsilon := 0.05 * N // maybe 5% instead of 0.1% | ||
| expectedFails := (N - failBackend) * ti.pExpectedAdmissionShedding | ||
|
|
||
| if expectedFails-epsilon > fail || fail > expectedFails+epsilon { | ||
| t.Errorf("Failed to get expected fails should be in: %0.2f < %0.2f < %0.2f", expectedFails-epsilon, fail, expectedFails+epsilon) | ||
| } |
There was a problem hiding this comment.
Maybe a comment on what we check here, it is hard to follow.
For the epsilon check maybe it is better to calculate the diff first
diff := math.Abs(fails - expectedFails)
if diff > epsilon {
t.Error(...)
}There was a problem hiding this comment.
I think it's more easy, than you suggestion and if you read it like a math equation it's simpler to follow:
A-e > fail > A+e
a61765d
to
3edd5a7
Compare
bocytko
reviewed
Jul 4, 2022
…ackend errors
admissionControl(metricsSuffix, mode, d, windowSize, minRPS, successThreshold, maxRejectProbability, exponent)
admissionControl("myapp", "active", "1s", 5, 10, 0.9, 0.95, 2.0)
metricSuffix is the suffix key to expose reject counter, should be unique by filter instance
mode is one of "active", "inactive", "log"
windowSize is within [minWindowSize, maxWindowSize]
minRPS
successThreshold is within (0,1] and sets the lowest request success rate at which the filter will not reject requests.
maxRejectProbability is within (0,1] and sets the upper bound of reject probability.
exponent >0, 1: linear, 1/2: qudratic, ..
see also https://opensource.zalando.com/skipper/reference/filters/#admissioncontrol
safety: handles background goroutine close on update and delete
feature: add preprocessor to remove filter duplicates, last wins
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
szuecs
force-pushed
the
feature/admission-control-filter
branch
from
d704159
to
97ee825
Compare
cleanup: deleting a filter will close that filter Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
| } | ||
|
|
||
| type validationPostProcessorNumberOfFilters struct { | ||
| ch chan []*routing.Route |
There was a problem hiding this comment.
minor: Maybe it could have *testing.T field and call t.Error if admission filter counter is more than one
There was a problem hiding this comment.
The problem is that t is not Thread safe. At least in my tests I got warnings or a panic, don't remember anymore.
|
👍 |
1 similar comment
|
👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
load shedding filter admissionControl, part of #2004
Signed-off-by: Sandor Szücs sandor.szuecs@zalando.de