Skip to content

Commit

Permalink
Bump the default periodSeconds to 10s. (#8148)
Browse files Browse the repository at this point in the history
Fixes: #8147
  • Loading branch information
mattmoor authored Jun 1, 2020
1 parent 7028944 commit 2247211
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 7 deletions.
1 change: 1 addition & 0 deletions cmd/queue/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ func probeQueueHealthPath(timeoutSeconds int, env probeConfig) 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

0 comments on commit 2247211

Please sign in to comment.