Skip to content
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

Bump the default periodSeconds to 10s. #8148

Merged
merged 1 commit into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/queue/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ func probeQueueHealthPath(port, timeoutSeconds int, env config) error {
}
defer res.Body.Close()
success := health.IsHTTPProbeReady(res)

// The check for preferForScaledown() fails readiness faster
// in the presence of the label
if preferScaleDown, err := preferPodForScaledown(env.DownwardAPILabelsPath); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/revision/resources/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ var (
Command: []string{"/ko-app/queue", "-probe-period", "0"},
},
},
PeriodSeconds: 1,
PeriodSeconds: 10,
TimeoutSeconds: 10,
},
SecurityContext: queueSecurityContext,
Expand Down
18 changes: 13 additions & 5 deletions pkg/reconciler/revision/resources/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,19 @@ func makeQueueProbe(in *corev1.Probe) *corev1.Probe {
Command: []string{"/ko-app/queue", "-probe-period", "0"},
},
},
// We want to mark the service as not ready as soon as the
// PreStop handler is called, so we need to check a little
// bit more often than the default. It is a small
// sacrifice for a low rate of 503s.
PeriodSeconds: 1,
// The exec probe enables us to retry failed probes quickly to get sub-second
// resolution and achieve faster cold-starts. However, for draining pods as
// part of the K8s lifecycle this period will bound the tail of how quickly
// we can remove a Pod's endpoint from the K8s service.
//
// The trade-off here is that exec probes cost CPU to run, and for idle pods
// (e.g. due to minScale) we see ~50m/{period} of idle CPU usage in the
// queue-proxy. So while setting this to 1s results in slightly faster drains
// it also means that in the steady state the queue-proxy is consuming 10x
// more CPU due to probes than with a period of 10s.
//
// See also: https://github.com/knative/serving/issues/8147
PeriodSeconds: 10,
// We keep the connection open for a while because we're
// actively probing the user-container on that endpoint and
// thus don't want to be limited by K8s granularity here.
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/revision/resources/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ func TestTCPProbeGeneration(t *testing.T) {
Command: []string{"/ko-app/queue", "-probe-period", "0"},
},
},
PeriodSeconds: 1,
PeriodSeconds: 10,
TimeoutSeconds: 10,
}
c.Env = env(map[string]string{"USER_PORT": strconv.Itoa(userPort)})
Expand Down