correctly implement gRPC health checking, or remove it

risingwavelabs / risingwave

Best-in-class stream processing, analytics, and management. Perform continuous analytics, or build event-driven applications, real-time ETL pipelines, and feature stores in minutes. Unified streaming and batch. PostgreSQL compatible.

https://go.risingwave.com/slack

Apache License 2.0

7k stars 575 forks source link

correctly implement gRPC health checking, or remove it #17778

Open BugenZhao opened 3 months ago

BugenZhao commented 3 months ago

The gRPC health check is also a standard interface, but I'm afraid it's not correctly implemented yet.

Originally posted by @BugenZhao in https://github.com/risingwavelabs/risingwave/issues/17771#issuecomment-2244305973

Currently we always return Serving.

The priority might be low since

we already have cluster-level health checking through SQL interface, i.e., rw_recovery_status()
it might not be that meaningful to define "whether healthy or not" separately for different nodes in a cluster
currently there's no usage of this

BugenZhao commented 3 months ago

Or, if we find it not useful, we can remove the support to avoid confusion.

BugenZhao commented 2 months ago

We should first define node-level health. For example, a compute node is considered healthy in terms of responding batch queries even when the cluster is under recovery, while it's not in terms of making DDL changes.

github-actions[bot] commented 4 days ago

This issue has been open for 60 days with no activity.

If you think it is still relevant today, and needs to be done in the near future, you can comment to update the status, or just manually remove the no-issue-activity label.

You can also confidently close this issue as not planned to keep our backlog clean. Don't worry if you think the issue is still valuable to continue in the future. It's searchable and can be reopened when it's time. 😄