Dennis-Mircea commented on code in PR #1186:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/1186#discussion_r3812619301
##########
docs/layouts/shortcodes/generated/kubernetes_operator_config_configuration.html:
##########
@@ -494,12 +494,36 @@
<td>Boolean</td>
<td>If enabled, FlinkSessionJob jarURI hosts that resolve to
loopback, link-local, site-local, wildcard or multicast addresses are rejected
during validation. Disable only if the operator legitimately needs to fetch
from such addresses.</td>
</tr>
+ <tr>
+
<td><h5>kubernetes.operator.user.artifacts.http.connect-timeout</h5></td>
+ <td style="word-wrap: break-word;">30 s</td>
+ <td>Duration</td>
+ <td>The connect timeout for downloading a FlinkSessionJob jarURI
over http(s). Bounds how long the reconcile thread can be blocked establishing
the connection to an unresponsive artifact host.</td>
+ </tr>
<tr>
<td><h5>kubernetes.operator.user.artifacts.http.header</h5></td>
<td style="word-wrap: break-word;">(none)</td>
<td>Map</td>
<td>Custom HTTP header for HttpArtifactFetcher. The header will be
applied when getting the session job artifacts. Expected format:
headerKey1:headerValue1,headerKey2:headerValue2.</td>
</tr>
+ <tr>
+
<td><h5>kubernetes.operator.user.artifacts.http.read-timeout</h5></td>
Review Comment:
I'd go with just one timeout property for socket level connection, because
it looks too much properties that have to be configured in the end. So, I'd say
merging connect-timeout and read-timeout into one socket-timeout makes more
sense.
##########
docs/content.zh/docs/deployment/security.md:
##########
@@ -66,6 +66,12 @@ For a `FlinkSessionJob`, the operator itself downloads the
job artifact referenc
- `kubernetes.operator.user.artifacts.disallow-restricted-hosts` (default
`true`): rejects `http` and `https` URIs whose host resolves to a loopback,
link-local, site-local, wildcard, or multicast address, so a session job cannot
point the operator at cluster-internal endpoints.
- `kubernetes.operator.user.artifacts.http.header`: custom HTTP headers sent
when fetching artifacts over `http` and `https`, typically carrying the
credentials of the artifact store.
+The fetch itself runs synchronously on the reconcile thread, so a slow or
unresponsive artifact host could otherwise pin that thread and, with enough
concurrent session jobs, exhaust the bounded reconcile pool
(`kubernetes.operator.reconcile.parallelism`). Three settings bound the
reconcile thread's exposure to such a host, and one bounds the response size:
+
+- `kubernetes.operator.user.artifacts.http.connect-timeout` (default `30 s`)
and `kubernetes.operator.user.artifacts.http.read-timeout` (default `30 s`):
the connect and per-read socket timeouts for the underlying HTTP connection.
+- `kubernetes.operator.user.artifacts.http.total-timeout` (default `5 min`):
the overall wall-clock budget for the whole fetch, covering all redirects and
the full body transfer. This is what bounds a host that trickles data just
slowly enough to keep individual reads under the read timeout without the
transfer ever completing.
+- `kubernetes.operator.user.artifacts.max-size` (default `1 gb`): the maximum
artifact size; the download is rejected once it exceeds this, whether or not
the server declares a `Content-Length` up front.
Review Comment:
I would not use `;` as part the documentation. I see the `.` as more
appropriate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
