pkg icon indicating copy to clipboard operation
pkg copied to clipboard

Allow configuration of threads-per-controller

Open SaschaSchwarze0 opened this issue 3 years ago • 6 comments

Changes

The Knative controller infrastructure uses by default two threads per controller. This setting is not configurable in Knative itself while Tekton allows to configure it. I was playing around with that setting in Knative and saw massively better numbers to bring Knative up when it reconciles everything in the system.

  • :gift: introduce threads-per-controller flag to configure the number of go routines per controller

/kind enhancement

Release Note

You can now provide a `threads-per-controller` command line flag to configure the number of go routines per thread, the default is 2

Docs

Please let me know if this needs to be documented now given this is "only" in the library and needs to be picked up by the serving repo first.

SaschaSchwarze0 avatar Aug 11 '22 06:08 SaschaSchwarze0

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: SaschaSchwarze0 / name: Sascha Schwarze (6d3c0c2269ee1e45e93c55b2def9d34158cc80b8)

Welcome @SaschaSchwarze0! It looks like this is your first PR to knative/pkg 🎉

knative-prow[bot] avatar Aug 11 '22 06:08 knative-prow[bot]

Hi @SaschaSchwarze0. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

knative-prow[bot] avatar Aug 11 '22 06:08 knative-prow[bot]

/cc mattmoor /cc n3wscott

SaschaSchwarze0 avatar Aug 12 '22 05:08 SaschaSchwarze0

Codecov Report

Base: 81.23% // Head: 81.29% // Increases project coverage by +0.05% :tada:

Coverage data is based on head (5a7a6a6) compared to base (a6afcab). Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2567      +/-   ##
==========================================
+ Coverage   81.23%   81.29%   +0.05%     
==========================================
  Files         162      162              
  Lines        9774     9820      +46     
==========================================
+ Hits         7940     7983      +43     
- Misses       1595     1597       +2     
- Partials      239      240       +1     
Impacted Files Coverage Δ
webhook/stats_reporter.go 85.13% <0.00%> (-0.32%) :arrow_down:
configmap/load.go 79.31% <0.00%> (ø)
websocket/connection.go 93.85% <0.00%> (ø)
configmap/hash-gen/main.go 62.96% <0.00%> (ø)
webhook/admission.go 85.18% <0.00%> (+0.56%) :arrow_up:
webhook/conversion.go 88.57% <0.00%> (+1.90%) :arrow_up:
...k/resourcesemantics/validation/reconcile_config.go 82.85% <0.00%> (+2.69%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Aug 15 '22 14:08 codecov[bot]

SaschaSchwarze0 requested review from n3wscott and removed request for mattmoor 27 seconds ago

Hm, no? I requested re-review from n3wscott but have no permission to add or remove somebody without the use of the bot.

/cc mattmoor

SaschaSchwarze0 avatar Aug 25 '22 16:08 SaschaSchwarze0

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, SaschaSchwarze0

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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

knative-prow[bot] avatar Sep 14 '22 15:09 knative-prow[bot]

this unit test failure looks like a flake to me.

=== RUN   TestDialWithBackoffTimeout
    transports_test.go:150: Unexpected success dialing
--- FAIL: TestDialWithBackoffTimeout (0.05s)

/test unit-tests

The other failures are version mismatches with pkg and the downstream.

n3wscott avatar Sep 14 '22 15:09 n3wscott