Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

reduce apiserver requests for dispatcher #456

Conversation

xinydev
Copy link
Contributor

@xinydev xinydev commented Mar 11, 2021

Signed-off-by: XinYang xinydev@gmail.com

The dispatcher now only gets information for the channel it cares about. reducing the GET requests to apiserver in a cluster with lots of channels.

Proposed Changes

  • return before GET requests for channels dispatcher doesn’t need to care about

Release Note


Docs

Signed-off-by: XinYang <xinydev@gmail.com>
@xinydev xinydev requested review from a team as code owners March 11, 2021 14:27
@xinydev xinydev requested a review from a user March 11, 2021 14:27
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Mar 11, 2021
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 11, 2021
@knative-prow-robot
Copy link
Contributor

Welcome @xinydev! It looks like this is your first PR to knative-sandbox/eventing-kafka 🎉

@knative-prow-robot
Copy link
Contributor

Hi @xinydev. Thanks for your PR.

I'm waiting for a knative-sandbox 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-robot knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 11, 2021
@lionelvillard
Copy link
Contributor

/ok-to-test

@knative-prow-robot knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 11, 2021
@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-knative-sandbox-eventing-kafka-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/channel/distributed/dispatcher/controller/kafkachannel.go 78.9% 71.8% -7.0

@codecov
Copy link

codecov bot commented Mar 11, 2021

Codecov Report

Merging #456 (ee04c1d) into main (9c479e0) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #456      +/-   ##
==========================================
- Coverage   73.94%   73.80%   -0.14%     
==========================================
  Files         138      138              
  Lines        5315     5315              
==========================================
- Hits         3930     3923       -7     
- Misses       1140     1145       +5     
- Partials      245      247       +2     
Impacted Files Coverage Δ
.../distributed/dispatcher/controller/kafkachannel.go 64.36% <100.00%> (-8.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c479e0...ee04c1d. Read the comment docs.

@travis-minke-sap
Copy link
Contributor

looks like a good change (thanks!) - I'm doing a quick smoke test now and then will approve!

Copy link
Contributor

@travis-minke-sap travis-minke-sap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix - nice efficiency win !

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 11, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: travis-minke-sap

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-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2021
@knative-prow-robot knative-prow-robot merged commit 5a79b3b into knative-extensions:main Mar 11, 2021
matzew pushed a commit to matzew/eventing-kafka that referenced this pull request Nov 23, 2021
Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
devguyio added a commit to devguyio/eventing-kafka that referenced this pull request Nov 25, 2021
Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
devguyio added a commit to devguyio/eventing-kafka that referenced this pull request Nov 25, 2021
Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
markusthoemmes pushed a commit to markusthoemmes/eventing-kafka that referenced this pull request Nov 26, 2021
markusthoemmes pushed a commit to markusthoemmes/eventing-kafka that referenced this pull request Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants