set net.ipv4.tcp_keepalive_time and net.core.somaxconn for tidb and tikv in init container #1107
Conversation
DanielZhangQD
requested review from
cofyc,
tennix,
Yisaer,
aylei and
weekface
|
After discussion with @tennix and @weekface, will update the solution as below:
|
| tcpKeepaliveIntvl: 75 | ||
| {{- end }} | ||
| {{- if .Values.soMaxConn }} | ||
| soMaxConn: {{ .Values.soMaxConn }} |
There was a problem hiding this comment.
Putting these parameters at cluster level is a little misleading, how about moving these to component? e.g. .sepc.tidb.tcpKeepaliveTime, .spec.tikv.soMaxConn
There was a problem hiding this comment.
I prefer a general solution. How about using the sysctl struct from Kubernetes?
It's an alternative way when configuring via Kubelet is not available, no need to use another structure.
There was a problem hiding this comment.
+1 config per component, like podSecurityContext which we are using now
There was a problem hiding this comment.
Reuse the PodSecurityContext in each component.
| privileged := true | ||
| initContainers = append(initContainers, corev1.Container{ | ||
| Name: "init", | ||
| Image: controller.GetSlowLogTailerImage(tc), |
There was a problem hiding this comment.
Use another image or promote the slow log tailer image to a general util image
There was a problem hiding this comment.
Actually, the image does not matter, so I just reuse the image from the slow log tailer and we also have a default value for it, this does not involve any change to values.yaml and users do not need to know about it at all.
| @@ -266,6 +266,31 @@ func getNewTiDBSetForTidbCluster(tc *v1alpha1.TidbCluster) *apps.StatefulSet { | |||
| }) | |||
| } | |||
|
|
|||
| sysctls := "sysctl -w" | |||
| if tc.Spec.TCPKeepaliveTime > 0 { | |||
There was a problem hiding this comment.
| if tc.Spec.TCPKeepaliveTime > 0 { | |
| if tc.Spec.TCPKeepaliveTime > 0 && tc.Spec.TCPKeepaliveIntvl > 0 { |
| privileged := true | ||
| initContainers = append(initContainers, corev1.Container{ | ||
| Name: "init", | ||
| Image: controller.GetSlowLogTailerImage(tc), |
There was a problem hiding this comment.
I suggest using an annotation to configure the init container image, and also set a default image when the annotation is not available.
There was a problem hiding this comment.
I prefer a dedicate field instead of annotation if we really want it to be configurable, for troubleshooting purpose, changing the image via kubectl edit is more straightforward.
There was a problem hiding this comment.
I think it's unnecessary to introduce user interface change for this image, it does not matter what it is and for TiDB version >= v2.1.8, slow log tailer must be enabled to check slow log.
|
/run-e2e-in-kind |
change description when using kubectl deleting tidbmonitor.
What problem does this PR solve?
fix #880 for GCP and Alibaba
What is changed and how does it work?
Set net.ipv4.tcp_keepalive_time and net.core.somaxconn for tidb in init container
Set net.core.somaxconn for tikv in init container
Check List
Tests
Code changes
Related changes
Does this PR introduce a user-facing change?: