-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add a document to tell users how to configure Thread, QPS and Burst #3508
Add a document to tell users how to configure Thread, QPS and Burst #3508
Conversation
|
Hi @xiujuan95. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions 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/test-infra repository. |
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.
/kind documentation
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.
/ok-to-test
docs/install.md
Outdated
@@ -363,6 +363,10 @@ data: | |||
|
|||
To customize the behavior of HA for the Tekton Pipelines controller, please refer to the related [documentation](developers/enabling-ha.md). | |||
|
|||
## Overwriting the ThreadsPerController, QPS and Burst |
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.
Suggest a slightly broader title of Configuring Tekton Controller Performance
.
ThreadsPerController
, QPS
, and Burst
are specific settings but we might one day add more ways to configure performance so placing them under a broader title makes sense I think.
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.
Done!
@@ -0,0 +1,42 @@ | |||
# Configure ThreadsPerController, QPS and Burst |
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.
I don't think the developers
subdirectory is the right place for this doc. developers
is for documentation that people coding Tekton Pipelines might need.
Suggest putting this file at docs/performance-configuration.md
or something similar. What do you think?
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.
@sbwsg Thanks for your suggestions!
Actually, I agree with you. But I saw this doc enabling-ha.md
also under developers
folder. I think these two files have the same target. So I also put it in developers
subdirectory. Anyway, I can change it to docs/performance-configuration.md
. Thanks again!
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.
Done! Pls take a look again, thanks!
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.
Ah, you're right. I think enabling-ha.md
should probably also be in the docs section. I will make a PR, thank you!
docs/install.md
Outdated
@@ -363,6 +363,10 @@ data: | |||
|
|||
To customize the behavior of HA for the Tekton Pipelines controller, please refer to the related [documentation](developers/enabling-ha.md). | |||
|
|||
## Overwriting the ThreadsPerController, QPS and Burst | |||
|
|||
If you want to modify the default ThreadsPerController, QPS and Burst to meet your performance requirements, then please refer to this [guidance](developers/configuring-qps-burst-thread.md). |
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.
It might be worth adding a little bit of context here. Something like this:
Out-of-the-box, Tekton Pipelines is configured for relatively small-scale deployments but several options for configuring Pipelines' performance are available. See the Performance Configuration document which describes how to change the
ThreadsPerController
,QPS
andBurst
settings to meet your requirements.
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.
Done!
/test check-pr-has-kind-label |
/approve |
@xiujuan95 Could you squash your commits into 1? Once that is done I think the PR is in good shape to merge. /approve cancel |
Document about how to configure controller's performance Add a document to show how to configure Thread, QPS and Burst A doc about configuring tekton controller's performance Refactor the way timeouts are handled `{Task,Pipeline}Run` now handle timeouts via `EnqueueAfter` on the workqueue. `pkg/timeout` is now removed. We now have consistent `GetTimeout(ctx)` methods on types. updating readme with 0.18 Adding doc/examples link for 0.18 correct the disabled link Use dogfooding buildx image for multi-arch builds Signed-off-by: Yulia Gaponenko <yulia.gaponenko1@de.ibm.com> Use dogfooding skopeo image for e2e and examples tests Signed-off-by: Yulia Gaponenko <yulia.gaponenko1@de.ibm.com> Fix markdown styling The header of the markdown styling does not render properly on tekton.dev. Changing the formatting so the site can render properly.
@sbwsg I have merged all commits into one, pls take a look again! Thanks a lot! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
Changes
Follow up this PR. Especially for this comment.
This PR add a new document about how to configuring the ThreadPerController, QPS and Burst. The default ThreadPerController, QPS and Burst are
2
,5.0
and10
accordingly. Sometimes, users want to modify these default values. So a guidance is necessary.Beside, for QPS and Burst, the actual values of them are multiplied by 2, so the doc is more necessary. Because if a user isn't aware of this, then there will cause some misunderstand.
Fyi, @afrittoli
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmd
dir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes