-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[OSDOCS-17843]: Adding etcd TTL docs (TP) #104752
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
base: main
Are you sure you want to change the base?
Conversation
|
🤖 Fri Jan 23 16:41:52 - Prow CI generated the docs preview: https://104752--ocpdocs-pr.netlify.app/openshift-enterprise/latest/etcd/etcd-performance.html |
34245df to
5b65691
Compare
|
|
||
| .Procedure | ||
|
|
||
| . Edit the `KubeAPIServer` custom resource by entering the following command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lahinson , I think a note about valid values should be added (and maybe also an explanation about the property eventTTLMinutes, as currently it is only shown in the example without additional information).
From [1], the values should be between 5 and 180 (minutes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the above, as this is a TP feature and a Feature Gate was created for it [2], does that Feature Gate needs to be enabled? In that case, that should be added to the procedure.
cc @tjungblu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most of those are mentioned in our enablement doc here: https://docs.google.com/document/d/1ERRTmlcj5W_zNiP_0sCfM2D5RHwDAh3B5InCqnI1RHY/edit?tab=t.0#heading=h.ei2fzjm8eblm
when you enable TP on your cluster, you get it automatically enabled. When you do CustomNoUpgrade then you need to supply that feature flag.
5b65691 to
88778ea
Compare
modules/etcd-customize-ttl.adoc
Outdated
| $ oc patch kubeapiserver/<kubeapiserver_custom_resource_name> -p='{"spec": {"eventTTLMinutes": 5 }}' --type=merge | ||
| ---- | ||
| + | ||
| * The `KubeAPIServer` custom resource name is typically `cluster`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason this is not directly in the command? is there another resources besides cluster there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If cluster is typically the term used for the kubeapiserver CR name, I'm okay with including it in the command. I thought maybe the name of the kubeapiserver CR might vary depending on the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, that was too rhetorical. AFAIK there is only the cluster resource, so you can just run oc patch kubeapiserver/cluster
| + | ||
| [source,terminal] | ||
| ---- | ||
| $ etcdctl lease list | xargs -I {} etcdctl lease timetolive --keys {} | grep "/kubernetes.io/events" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason not to directly use that command instead of the non-event grep version above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a preferred alternative? I used what I saw in the enablement doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no I think it is a good explanation, I just wanted to save you a few more paragraphs that are not directly related to the event TTL itself.
| In the example, the TTL value is 5 minutes (`300s`), but the output shows `315s` because a lease buffer reuse of 5% is added. | ||
| + | ||
| [NOTE] | ||
| ==== | ||
| If a lease can be reused, etcd reuses it. | ||
| ==== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glad that you mention it, I think this will cause a lot of confusion, so we maybe want to explain a bit more how it works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have more information somewhere about how it works? I looked at https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/storage/etcd3/lease_manager.go#L88-L120, but unfortunately, I can't link from the official docs to that site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately not, I've sent you my suggestion on slack :)
06e7b5e to
fe5f6c7
Compare
|
/lgtm |
fe5f6c7 to
a32cb8b
Compare
|
New changes are detected. LGTM label has been removed. |
|
@lahinson: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Version(s): 4.21+
Issue: https://issues.redhat.com/browse/OSDOCS-17843
Link to docs preview: https://104752--ocpdocs-pr.netlify.app/openshift-enterprise/latest/etcd/etcd-performance.html#etcd-customize-ttl_etcd-performance
QE review:
Additional information: