-
Notifications
You must be signed in to change notification settings - Fork 351
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
Passive health checks #2888
Passive health checks #2888
Conversation
458288c
to
a7f8f3e
Compare
a7f8f3e
to
56b9fee
Compare
Let me comment on the idea.
yes you need requests total and errors total of a chosen time period.
For the stats collection I would rather have 2 counters for each period in time and swap them.
|
43d2d23
to
c9abb70
Compare
19bbbe0
to
3eb3c5f
Compare
519bddf
to
fe7d257
Compare
4dda575
to
563f5e3
Compare
I am testing this on my computer:
|
👍 |
563f5e3
to
51b32a3
Compare
|
Signed-off-by: Roman Zavodskikh <roman.zavodskikh@zalando.de>
51b32a3
to
3fcf81b
Compare
👍 |
1 similar comment
👍 |
Load tested the latest image from this PR with such an options on k8s cluster. 1 healthy and 1 completely unhealthy endpoint.
Seems fair enough to merge. |
if p < e.Metrics.HealthCheckDropProbability() { | ||
/* drop */ | ||
} else { | ||
filtered = append(filtered, e) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not
if p >= e.Metrics.HealthCheckDropProbability() {
filtered = append(filtered, e)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to be clear about the decision that if p < e.Metrics.HealthCheckDropProbability()
holds true we want to skip the endpoint.
@@ -145,6 +145,69 @@ type OpenTracingParams struct { | |||
ExcludeTags []string | |||
} | |||
|
|||
type PassiveHealthCheck struct { | |||
// The period of time after which the endpointregistry begins to calculate endpoints statistics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for godocs you need to start the comment with field name
// Period ....
same for other comments
MaxDropProbability float64 | ||
} | ||
|
||
func InitPassiveHealthChecker(o map[string]string) (bool, *PassiveHealthCheck, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe rename o to opts?
YAML flag parses (preferably flow-style) YAML value from the command line or unmarshals object yaml value from the yaml config file. Example value: ``` bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}' ``` and equivalent branch in config yaml: ```yaml foo-flag: foo: hello bar: - world - "1" baz: 2 qux: baz: 3 ``` This will be useful for #2104 It is also a better alternative to manual parsing of micro-syntax like e.g. implemented in #2888 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
YAML flag parses (preferably flow-style) YAML value from the command line or unmarshals object yaml value from the yaml config file. Example value: ``` bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}' ``` and equivalent branch in config yaml: ```yaml foo-flag: foo: hello bar: - world - "1" baz: 2 qux: baz: 3 ``` This will be useful for #2104 It is also a better alternative to manual parsing of micro-syntax like e.g. implemented in #2888 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
YAML flag parses (preferably flow-style) YAML value from the command line or unmarshals object yaml value from the yaml config file. Example value: ``` bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}' ``` and equivalent branch in config yaml: ```yaml foo-flag: foo: hello bar: - world - "1" baz: 2 qux: baz: 3 ``` This will be useful for #2104 It is also a better alternative to manual parsing of micro-syntax like e.g. implemented in #2888 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
YAML flag parses (preferably flow-style) YAML value from the command line or unmarshals object yaml value from the yaml config file. Example value: ``` bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}' ``` and equivalent branch in config yaml: ```yaml foo-flag: foo: hello bar: - world - "1" baz: 2 qux: baz: 3 ``` This will be useful for zalando#2104 It is also a better alternative to manual parsing of micro-syntax like e.g. implemented in zalando#2888 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
There is huge ongoing effort to enable Passive Health Checks is Skipper to check backends for errors and try not to send requests to the backends that tend to fail the requests.
The initial idea I have at the moment is the following:
statsResetPeriod
) update the stats to check endpoints' states change