Skip to content
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

[bugfix] Vald gRPC Client and Pool logic makes huge backoff #1953

Conversation

kpango
Copy link
Collaborator

@kpango kpango commented Feb 14, 2023

Signed-off-by: kpango kpango@vdaas.org

Description:

This PR inclueds many features.

Related Issue:

Versions:

  • Go Version: 1.20
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 2.0.9

Checklist:

Special notes for your reviewer:

@cloudflare-pages
Copy link

cloudflare-pages bot commented Feb 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d091470
Status: ✅  Deploy successful!
Preview URL: https://c3ce081a.vald.pages.dev
Branch Preview URL: https://bugfix-lb-backoff-gateway-lb.vald.pages.dev

View logs

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

@kpango kpango force-pushed the bugfix/lb-backoff/gateway-lb-exists-api-notfound-backoff-problem branch 3 times, most recently from 85bad2e to 4c8b24d Compare February 14, 2023 12:35
@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

Patch coverage: 3.61% and project coverage change: +0.01 🎉

Comparison is base (0f6c1e6) 29.68% compared to head (d091470) 29.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1953      +/-   ##
==========================================
+ Coverage   29.68%   29.70%   +0.01%     
==========================================
  Files         366      364       -2     
  Lines       34101    34130      +29     
==========================================
+ Hits        10122    10137      +15     
- Misses      23573    23576       +3     
- Partials      406      417      +11     
Impacted Files Coverage Δ
hack/tools/metrics/main.go 0.00% <0.00%> (ø)
internal/circuitbreaker/manager.go 0.00% <0.00%> (ø)
internal/client/v1/client/agent/core/client.go 0.00% <0.00%> (ø)
internal/client/v1/client/discoverer/discover.go 0.00% <0.00%> (ø)
internal/client/v1/client/vald/option.go 0.00% <ø> (ø)
internal/client/v1/client/vald/vald.go 0.00% <0.00%> (ø)
internal/config/lb.go 100.00% <ø> (ø)
internal/errors/grpc.go 75.00% <ø> (ø)
internal/file/file.go 12.00% <0.00%> (ø)
internal/net/control/control.go 98.09% <ø> (-0.02%) ⬇️
... and 44 more

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

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

@github-actions github-actions bot removed the size/M label Feb 14, 2023
@kpango kpango force-pushed the bugfix/lb-backoff/gateway-lb-exists-api-notfound-backoff-problem branch from 4c8b24d to ac4ab69 Compare February 14, 2023 13:11
@kpango kpango force-pushed the bugfix/lb-backoff/gateway-lb-exists-api-notfound-backoff-problem branch 5 times, most recently from d09391f to 38dc8b4 Compare February 15, 2023 09:27
}

func (l *logger) Fatal(vals ...interface{}) {
l.glg.Fatal(vals...)
l.retry.Out(l.glg.Fatal, vals...)
Copy link

Choose a reason for hiding this comment

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

26% of developers fix this issue

typecheck: cannot use l.glg.Fatal (value of type func(val ...interface{})) as func(vals ...interface{}) error value in argument to l.retry.Out

❗❗ 2 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
internal/log/glg/glg.go 141
internal/log/glg/glg.go 145

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@kpango kpango force-pushed the bugfix/lb-backoff/gateway-lb-exists-api-notfound-backoff-problem branch from 38dc8b4 to d2112fe Compare February 15, 2023 14:09
@github-actions github-actions bot added size/XL and removed size/L labels Feb 15, 2023
@kpango kpango force-pushed the bugfix/lb-backoff/gateway-lb-exists-api-notfound-backoff-problem branch 3 times, most recently from 70e66a2 to b8cd101 Compare February 16, 2023 08:55
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Mar 2, 2023

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
other images

Signed-off-by: kpango <kpango@vdaas.org>
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Mar 2, 2023

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
other images

@kpango kpango merged commit 4a4673a into main Mar 3, 2023
@kpango kpango deleted the bugfix/lb-backoff/gateway-lb-exists-api-notfound-backoff-problem branch March 3, 2023 01:44
@kpango kpango mentioned this pull request Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Refactor/Re-Construct MultiXXX APIs.
6 participants