Fix timeout handlers to max duration instead of time to first byte (#…

…12970) * fix our timeout handlers to max duration instead of time to first byte adds a new field for RevisionSpec for ResponseStartTimeout to handle situations when first byte timeout is needed * expose revision idle timeout * add validation for new fields * update description and property name * update conformance test for timeout * change default * move timeout test to e2e
knative · Aug 11, 2022 · 2e77abf · 2e77abf
1 parent 5bba016
commit 2e77abf
Show file tree

Hide file tree

Showing 20 changed files with 689 additions and 225 deletions.
diff --git a/config/core/300-resources/configuration.yaml b/config/core/300-resources/configuration.yaml
@@ -523,6 +523,10 @@ spec:
                             description: This is accessible behind a feature flag - kubernetes.podspec-hostaliases
                             type: object
                             x-kubernetes-preserve-unknown-fields: true
+                        idleTimeoutSeconds:
+                          description: IdleTimeoutSeconds is the maximum duration in seconds a request will be allowed to stay open while not receiving any bytes from the user's application. If unspecified, a system default will be provided.
+                          type: integer
+                          format: int64
                         imagePullSecrets:
                           description: 'ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. If specified, these secrets will be passed to individual puller implementations for them to use. For example, in the case of docker, only DockerConfig type secrets are honored. More info: https://kubernetes.io/docs/concepts/containers/images#specifying-imagepullsecrets-on-a-pod'
                           type: array
@@ -550,6 +554,10 @@ spec:
                           description: This is accessible behind a feature flag - kubernetes.podspec-priorityclassname
                           type: string
                           x-kubernetes-preserve-unknown-fields: true
+                        responseStartTimeoutSeconds:
+                          description: ResponseStartTimeoutSeconds is the maximum duration in seconds that the request routing layer will wait for a request delivered to a container to begin sending any network traffic.
+                          type: integer
+                          format: int64
                         runtimeClassName:
                           description: This is accessible behind a feature flag - kubernetes.podspec-runtimeclassname
                           type: string
@@ -566,7 +574,7 @@ spec:
                           description: 'ServiceAccountName is the name of the ServiceAccount to use to run this pod. More info: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/'
                           type: string
                         timeoutSeconds:
-                          description: TimeoutSeconds is the maximum duration in seconds that the request routing layer will wait for a request delivered to a container to begin replying (send network traffic). If unspecified, a system default will be provided.
+                          description: TimeoutSeconds is the maximum duration in seconds that the request instance is allowed to respond to a request. If unspecified, a system default will be provided.
                           type: integer
                           format: int64
                         tolerations:

diff --git a/config/core/300-resources/revision.yaml b/config/core/300-resources/revision.yaml
@@ -502,6 +502,10 @@ spec:
                     description: This is accessible behind a feature flag - kubernetes.podspec-hostaliases
                     type: object
                     x-kubernetes-preserve-unknown-fields: true
+                idleTimeoutSeconds:
+                  description: IdleTimeoutSeconds is the maximum duration in seconds a request will be allowed to stay open while not receiving any bytes from the user's application. If unspecified, a system default will be provided.
+                  type: integer
+                  format: int64
                 imagePullSecrets:
                   description: 'ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. If specified, these secrets will be passed to individual puller implementations for them to use. For example, in the case of docker, only DockerConfig type secrets are honored. More info: https://kubernetes.io/docs/concepts/containers/images#specifying-imagepullsecrets-on-a-pod'
                   type: array
@@ -529,6 +533,10 @@ spec:
                   description: This is accessible behind a feature flag - kubernetes.podspec-priorityclassname
                   type: string
                   x-kubernetes-preserve-unknown-fields: true
+                responseStartTimeoutSeconds:
+                  description: ResponseStartTimeoutSeconds is the maximum duration in seconds that the request routing layer will wait for a request delivered to a container to begin sending any network traffic.
+                  type: integer
+                  format: int64
                 runtimeClassName:
                   description: This is accessible behind a feature flag - kubernetes.podspec-runtimeclassname
                   type: string
@@ -545,7 +553,7 @@ spec:
                   description: 'ServiceAccountName is the name of the ServiceAccount to use to run this pod. More info: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/'
                   type: string
                 timeoutSeconds:
-                  description: TimeoutSeconds is the maximum duration in seconds that the request routing layer will wait for a request delivered to a container to begin replying (send network traffic). If unspecified, a system default will be provided.
+                  description: TimeoutSeconds is the maximum duration in seconds that the request instance is allowed to respond to a request. If unspecified, a system default will be provided.
                   type: integer
                   format: int64
                 tolerations:

diff --git a/config/core/300-resources/service.yaml b/config/core/300-resources/service.yaml
@@ -527,6 +527,10 @@ spec:
                             description: This is accessible behind a feature flag - kubernetes.podspec-hostaliases
                             type: object
                             x-kubernetes-preserve-unknown-fields: true
+                        idleTimeoutSeconds:
+                          description: IdleTimeoutSeconds is the maximum duration in seconds a request will be allowed to stay open while not receiving any bytes from the user's application. If unspecified, a system default will be provided.
+                          type: integer
+                          format: int64
                         imagePullSecrets:
                           description: 'ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. If specified, these secrets will be passed to individual puller implementations for them to use. For example, in the case of docker, only DockerConfig type secrets are honored. More info: https://kubernetes.io/docs/concepts/containers/images#specifying-imagepullsecrets-on-a-pod'
                           type: array
@@ -554,6 +558,10 @@ spec:
                           description: This is accessible behind a feature flag - kubernetes.podspec-priorityclassname
                           type: string
                           x-kubernetes-preserve-unknown-fields: true
+                        responseStartTimeoutSeconds:
+                          description: ResponseStartTimeoutSeconds is the maximum duration in seconds that the request routing layer will wait for a request delivered to a container to begin sending any network traffic.
+                          type: integer
+                          format: int64
                         runtimeClassName:
                           description: This is accessible behind a feature flag - kubernetes.podspec-runtimeclassname
                           type: string
@@ -570,7 +578,7 @@ spec:
                           description: 'ServiceAccountName is the name of the ServiceAccount to use to run this pod. More info: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/'
                           type: string
                         timeoutSeconds:
-                          description: TimeoutSeconds is the maximum duration in seconds that the request routing layer will wait for a request delivered to a container to begin replying (send network traffic). If unspecified, a system default will be provided.
+                          description: TimeoutSeconds is the maximum duration in seconds that the request instance is allowed to respond to a request. If unspecified, a system default will be provided.
                           type: integer
                           format: int64
                         tolerations:

diff --git a/config/core/configmaps/defaults.yaml b/config/core/configmaps/defaults.yaml
@@ -22,7 +22,7 @@ metadata:
     app.kubernetes.io/component: controller
     app.kubernetes.io/version: devel
   annotations:
-    knative.dev/example-checksum: "a0feb4c6"
+    knative.dev/example-checksum: "4b3e0057"
 data:
   _example: |
     ################################
@@ -54,6 +54,16 @@ data:
     # should also be increased to prevent in-flight requests being disrupted.
     max-revision-timeout-seconds: "600"  # 10 minutes
 
+    # revision-response-start-timeout-seconds contains the default number of
+    # seconds a request will be allowed to stay open while waiting to
+    # receive any bytes from the user's application, if none is specified.
+    revision-response-start-timeout-seconds: "300"
+
+    # revision-idle-timeout-seconds contains the default number of
+    # seconds a request will be allowed to stay open while not receiving any
+    # bytes from the user's application, if none is specified.
+    revision-idle-timeout-seconds: "0"  # infinite
+
     # revision-cpu-request contains the cpu allocation to assign
     # to revisions by default.  If omitted, no value is specified
     # and the system default is used.

diff --git a/docs/serving-api.md b/docs/serving-api.md
@@ -915,9 +915,37 @@ int64
 </td>
 <td>
 <em>(Optional)</em>
-<p>TimeoutSeconds is the maximum duration in seconds that the request routing
-layer will wait for a request delivered to a container to begin replying
-(send network traffic). If unspecified, a system default will be provided.</p>
+<p>TimeoutSeconds is the maximum duration in seconds that the request instance
+is allowed to respond to a request. If unspecified, a system default will
+be provided.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>responseStartTimeoutSeconds</code><br/>
+<em>
+int64
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>ResponseStartTimeoutSeconds is the maximum duration in seconds that the request
+routing layer will wait for a request delivered to a container to begin
+sending any network traffic.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>idleTimeoutSeconds</code><br/>
+<em>
+int64
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>IdleTimeoutSeconds is the maximum duration in seconds a request will be allowed
+to stay open while not receiving any bytes from the user&rsquo;s application. If
+unspecified, a system default will be provided.</p>
 </td>
 </tr>
 </table>
@@ -1377,9 +1405,37 @@ int64
 </td>
 <td>
 <em>(Optional)</em>
-<p>TimeoutSeconds is the maximum duration in seconds that the request routing
-layer will wait for a request delivered to a container to begin replying
-(send network traffic). If unspecified, a system default will be provided.</p>
+<p>TimeoutSeconds is the maximum duration in seconds that the request instance
+is allowed to respond to a request. If unspecified, a system default will
+be provided.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>responseStartTimeoutSeconds</code><br/>
+<em>
+int64
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>ResponseStartTimeoutSeconds is the maximum duration in seconds that the request
+routing layer will wait for a request delivered to a container to begin
+sending any network traffic.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>idleTimeoutSeconds</code><br/>
+<em>
+int64
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>IdleTimeoutSeconds is the maximum duration in seconds a request will be allowed
+to stay open while not receiving any bytes from the user&rsquo;s application. If
+unspecified, a system default will be provided.</p>
 </td>
 </tr>
 </tbody>
@@ -1577,9 +1633,37 @@ int64
 </td>
 <td>
 <em>(Optional)</em>
-<p>TimeoutSeconds is the maximum duration in seconds that the request routing
-layer will wait for a request delivered to a container to begin replying
-(send network traffic). If unspecified, a system default will be provided.</p>
+<p>TimeoutSeconds is the maximum duration in seconds that the request instance
+is allowed to respond to a request. If unspecified, a system default will
+be provided.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>responseStartTimeoutSeconds</code><br/>
+<em>
+int64
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>ResponseStartTimeoutSeconds is the maximum duration in seconds that the request
+routing layer will wait for a request delivered to a container to begin
+sending any network traffic.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>idleTimeoutSeconds</code><br/>
+<em>
+int64
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>IdleTimeoutSeconds is the maximum duration in seconds a request will be allowed
+to stay open while not receiving any bytes from the user&rsquo;s application. If
+unspecified, a system default will be provided.</p>
 </td>
 </tr>
 </table>

diff --git a/pkg/apis/config/defaults.go b/pkg/apis/config/defaults.go
@@ -44,6 +44,14 @@ const (
 	// DefaultMaxRevisionTimeoutSeconds will be set if MaxRevisionTimeoutSeconds is not specified.
 	DefaultMaxRevisionTimeoutSeconds = 10 * 60
 
+	// DefaultRevisionResponseStartTimeoutSeconds will be set if ResponseStartTimeoutSeconds is not specified.
+	// for backward compatibility will keep default similar to DefaultRevisionTimeoutSeconds,
+	// should be revised in future releases.
+	DefaultRevisionResponseStartTimeoutSeconds = 5 * 60
+
+	// DefaultRevisionIdleTimeoutSeconds will be set if idleTimeoutSeconds not specified.
+	DefaultRevisionIdleTimeoutSeconds = 0
+
 	// DefaultInitContainerName is the default name we give to the init containers
 	// specified by the user, if `name:` is omitted.
 	DefaultInitContainerName = "init-container"
@@ -71,14 +79,16 @@ var (
 
 func defaultDefaultsConfig() *Defaults {
 	return &Defaults{
-		RevisionTimeoutSeconds:        DefaultRevisionTimeoutSeconds,
-		MaxRevisionTimeoutSeconds:     DefaultMaxRevisionTimeoutSeconds,
-		InitContainerNameTemplate:     DefaultInitContainerNameTemplate,
-		UserContainerNameTemplate:     DefaultUserContainerNameTemplate,
-		ContainerConcurrency:          DefaultContainerConcurrency,
-		ContainerConcurrencyMaxLimit:  DefaultMaxRevisionContainerConcurrency,
-		AllowContainerConcurrencyZero: DefaultAllowContainerConcurrencyZero,
-		EnableServiceLinks:            ptr.Bool(false),
+		RevisionTimeoutSeconds:             DefaultRevisionTimeoutSeconds,
+		MaxRevisionTimeoutSeconds:          DefaultMaxRevisionTimeoutSeconds,
+		RevisionRequestStartTimeoutSeconds: DefaultRevisionResponseStartTimeoutSeconds,
+		RevisionIdleTimeoutSeconds:         DefaultRevisionIdleTimeoutSeconds,
+		InitContainerNameTemplate:          DefaultInitContainerNameTemplate,
+		UserContainerNameTemplate:          DefaultUserContainerNameTemplate,
+		ContainerConcurrency:               DefaultContainerConcurrency,
+		ContainerConcurrencyMaxLimit:       DefaultMaxRevisionContainerConcurrency,
+		AllowContainerConcurrencyZero:      DefaultAllowContainerConcurrencyZero,
+		EnableServiceLinks:                 ptr.Bool(false),
 	}
 }
 
@@ -111,6 +121,9 @@ func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {
 
 		cm.AsInt64("revision-timeout-seconds", &nc.RevisionTimeoutSeconds),
 		cm.AsInt64("max-revision-timeout-seconds", &nc.MaxRevisionTimeoutSeconds),
+		cm.AsInt64("revision-response-start-timeout-seconds", &nc.RevisionRequestStartTimeoutSeconds),
+		cm.AsInt64("revision-idle-timeout-seconds", &nc.RevisionIdleTimeoutSeconds),
+
 		cm.AsInt64("container-concurrency", &nc.ContainerConcurrency),
 		cm.AsInt64("container-concurrency-max-limit", &nc.ContainerConcurrencyMaxLimit),
 
@@ -127,6 +140,12 @@ func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {
 	if nc.RevisionTimeoutSeconds > nc.MaxRevisionTimeoutSeconds {
 		return nil, fmt.Errorf("revision-timeout-seconds (%d) cannot be greater than max-revision-timeout-seconds (%d)", nc.RevisionTimeoutSeconds, nc.MaxRevisionTimeoutSeconds)
 	}
+	if nc.RevisionRequestStartTimeoutSeconds > 0 && nc.RevisionRequestStartTimeoutSeconds > nc.RevisionTimeoutSeconds {
+		return nil, fmt.Errorf("revision-response-start-timeout-seconds (%d) cannot be greater than revision-timeout-seconds (%d)", nc.RevisionRequestStartTimeoutSeconds, nc.RevisionTimeoutSeconds)
+	}
+	if nc.RevisionIdleTimeoutSeconds > 0 && nc.RevisionIdleTimeoutSeconds > nc.RevisionTimeoutSeconds {
+		return nil, fmt.Errorf("revision-idle-timeout-seconds (%d) cannot be greater than revision-timeout-seconds (%d)", nc.RevisionIdleTimeoutSeconds, nc.RevisionTimeoutSeconds)
+	}
 	if nc.ContainerConcurrencyMaxLimit < 1 {
 		return nil, apis.ErrOutOfBoundsValue(
 			nc.ContainerConcurrencyMaxLimit, 1, math.MaxInt32, "container-concurrency-max-limit")
@@ -157,6 +176,14 @@ type Defaults struct {
 	// RevisionTimeoutSeconds must be less than this value.
 	MaxRevisionTimeoutSeconds int64
 
+	// This is  the default number of seconds a request will be allowed to
+	// stay open while waiting to receive any bytes from the user's application
+	RevisionRequestStartTimeoutSeconds int64
+
+	// RevisionIdleTimeoutSeconds is the maximum duration in seconds a request
+	// will be allowed to stay open while not receiving any bytes from the user's application.
+	RevisionIdleTimeoutSeconds int64
+
 	InitContainerNameTemplate *ObjectMetaTemplate
 
 	UserContainerNameTemplate *ObjectMetaTemplate