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

Add job error to report index correction error status #2231

Merged
merged 11 commits into from
Nov 9, 2023

Conversation

ykadowak
Copy link
Contributor

@ykadowak ykadowak commented Nov 7, 2023

Description:

This PR updates index correction in two ways,

  1. To report error to the usecase layer from service.Start, update error handling in the correct function. Also made changes in some cases not to continue processing but just stop processing and report errors.
  2. Changed to call discoverer.Start outside of the correction goroutine to handle error chan.

Related Issue:

Versions:

  • Go Version: 1.21.3
  • Docker Version: 20.10.8
  • Kubernetes Version: v1.28.2
  • NGT Version: 2.1.3

Checklist:

Special notes for your reviewer:

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Nov 7, 2023

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

Copy link

cloudflare-workers-and-pages bot commented Nov 7, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 99edb2c
Status: ✅  Deploy successful!
Preview URL: https://35bae172.vald.pages.dev
Branch Preview URL: https://feature-job-correction-refac.vald.pages.dev

View logs

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Attention: 38 lines in your changes are missing coverage. Please review.

Files Coverage Δ
internal/errors/errors.go 51.38% <100.00%> (+1.86%) ⬆️
internal/net/grpc/stream.go 28.27% <100.00%> (-0.49%) ⬇️
internal/errors/corrector.go 0.00% <0.00%> (ø)
pkg/index/job/correction/usecase/corrector.go 0.00% <0.00%> (ø)
pkg/index/job/correction/service/corrector.go 18.20% <0.00%> (-0.27%) ⬇️

... and 4 files with indirect coverage changes

📢 Thoughts on this report? Let us know!

@ykadowak ykadowak requested review from hlts2, a team and vankichi and removed request for a team November 8, 2023 02:28
@ykadowak ykadowak changed the title [WIP] Add job error to report index correction error status Add job error to report index correction error status Nov 8, 2023
vankichi
vankichi previously approved these changes Nov 8, 2023
@ykadowak ykadowak force-pushed the feature/job/correction-refactor branch from 4d13235 to 5f0a887 Compare November 8, 2023 05:40
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Nov 8, 2023

Profile Report

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

Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ykadowak ykadowak requested a review from vankichi November 8, 2023 06:10
@vankichi vankichi merged commit dfc9c2f into main Nov 9, 2023
92 of 93 checks passed
@vankichi vankichi deleted the feature/job/correction-refactor branch November 9, 2023 01:07
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
@vankichi vankichi mentioned this pull request Dec 4, 2023
kmrmt pushed a commit that referenced this pull request Dec 12, 2023
* Fix to monitor discoverer error for index correction

* Add errors.RemoveDuplicates

* Add jobErrs to report job status of index correction

* Simplify function literal

* Update Corrector interface method name to
StartMonitoring for clearity

* Removed unnecessary error handling

* Update Corrector interface method name to
StartClient

* Fix error handling in corrector.go to catch all the err in defer function

---------

Co-authored-by: Kiichiro YUKAWA <[email protected]>
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.

4 participants