- Notifications
You must be signed in to change notification settings - Fork3.8k
Remove unused grpc health check methods#6113
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
We have generic `grpc_healthcheck` endpoint that get registered for all the componentsduring initialization stage.Looks like these health checks that are part of actual component's struct itself came from legacy.Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester0.1%+ distributor1%+ querier0%+ querier/queryrange0%+ iter0%+ storage0%+ chunkenc0%+ logql0%+ loki0% |
| // Check implements grpc_health_v1.HealthCheck. | ||
| func (i*Ingester)Check(ctx context.Context,req*grpc_health_v1.HealthCheckRequest) (*grpc_health_v1.HealthCheckResponse,error) { | ||
| status:=grpc_health_v1.HealthCheckResponse_SERVING | ||
| if (i.State()!=services.Running)|| (i.lifecycler.GetState()!=ring.ACTIVE) { |
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'm a bit worried that we don't seem to be checking the lifecycler state in our blanket version, but it doesn't look like these are used either.
Uh oh!
There was an error while loading.Please reload this page.
What this PR does / why we need it:
We have generic
grpc_healthcheckendpoint that get registered for all the componentsduring initialization stage.
https://github.com/grafana/loki/blob/main/pkg/loki/loki.go#L364
Looks like these old health checks that are part of actual component's struct itself came from legacy.
Signed-off-by: Kavirajkavirajkanagaraj@gmail.com
NOTE: This should beno op. No build or test failure asthere are no consumer for these methods
Which issue(s) this PR fixes:
Fixes # NA
Special notes for your reviewer:
We figured this out while investigating this ingester/querier grpc issue#6023
Checklist
CHANGELOG.md.docs/sources/upgrading/_index.md