-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implement gRPC standard health checking service #135
Comments
@jen20 separate crate makes sense 👍 |
Has there been any progress on this? Just wondering before I implement my own 😄 |
@xd009642 I haven't made any yet, but do need to at some point. If you do it before Saturday you'll likely beat me to it though :-) |
I think I'll be starting on Monday so you may still beat me 😄 |
I’ve done the other end of this (client health checking) using tonic - https://github.com/zakhenry/nightingale It might make sense for this lib to be in the same repo? I’d be happy to contribute 😁 It would be cool to move it into the hyperium namespace to make it more “official” |
Here's something for someone to start on, I hacked this together quickly a few weeks back, I don't think it quite follows the spec, and is kinda gross, but it was good enough for my needs initially.
|
I started something and used some of @rlabrecque's work to go off https://github.com/xd009642/tonic-health I was thinking of using a spmc queue to effectively signal the wait method and potentially adding the signal handling for graceful shutdown (although looking at it I think I won't do that here). |
Okay I'm not there yet but it's starting to look some what there. My wait implementation is still a bit up in the air though |
If you know how to get rid of the NamedService stuff, that would be fantastic! |
@xd009642 As I have also been busy with the health check service, I will share my experience, thanks for sharing yours. From your The register can shared in threads. Now, my experience with I have published my code as a gist. If @LucioFranco thinks this is going into the right direction, I could create a PR to have the register and service build into the library. |
@frederikbosch oh nice, I kept meaning to get back to this but there's so many other things I have to do. I'll try out your version and see if it sorts the issues I found with my initial impl 👍 |
This seems fine but I don't quite see how an end user would use this? Like how would you know that your service is unavailable? Do you have any code that shows how you may do that? |
(so I'm also xd009642, just on my work account). So the standard usage is if you implement graceful shutdown any pending requests will be finished, new requests rejected and the server close when the last requests are processed or a timeout has elapsed. With that implementation, if you receive a SIGTERM the signal handler will set all the services to not serving, the services will check the registry when they receive a request and respond back with unavailable for new requests and the health check service will be send back not serving. Anything else is less standard and more application specific |
@LucioFranco The following code is used by the end user. // initialize service register on boot of app
let health_services = ServiceRegister::new(ServingStatus::Serving);
let guarded_services: Arc<RwLock<ServiceRegister>> = Arc::new(RwLock::new(health_services));
// inject the guarded services into some struct as service_register
pub struct SomeStruct {
service_register: Arc<RwLock<ServiceRegister>>,
}
// and then call the service register to register the service
self.service_register.write().await.set_status("", ServingStatus::Serving).unwrap(); |
Ok, I think this makes sense to make a |
I would love to learn how we can bypass |
@frederikbosch well easily, you can just make that an internal implementation detail :) or you can use channels. |
@LucioFranco I did not use channels because that would require another thread, one for listening health check requests and one for awaiting updates from the channel. With the solution I presented above, you only need a thread for the health check requests. How to make |
Sure, so I think really what you need to do is just store the arc mutex internally to some struct and expose easy to use async fn to set the correct items. |
This commit adds a new crate `tonic-health` which implements the [standard GRPC Health Checking][checking] protocol. Currently there is only a server implementation, though others have alluded in the discussion in #135 that client implementations exist which could also be imported as necessary. A example server has also been added - once the client work is done a client for this should be added also. [checking]: https://github.com/grpc/grpc/blob/master/doc/health-checking.md
This commit adds a new crate `tonic-health` which implements the [standard GRPC Health Checking][checking] protocol. Currently there is only a server implementation, though others have alluded in the discussion in #135 that client implementations exist which could also be imported as necessary. A example server has also been added - once the client work is done a client for this should be added also. [checking]: https://github.com/grpc/grpc/blob/master/doc/health-checking.md
This commit adds a new crate `tonic-health` which implements the [standard GRPC Health Checking][checking] protocol. Currently there is only a server implementation, though others have alluded in the discussion in #135 that client implementations exist which could also be imported as necessary. A example server has also been added - once the client work is done a client for this should be added also. [checking]: https://github.com/grpc/grpc/blob/master/doc/health-checking.md
This commit adds a new crate `tonic-health` which implements the [standard GRPC Health Checking][checking] protocol. Currently there is only a server implementation, though others have alluded in the discussion in #135 that client implementations exist which could also be imported as necessary. A example server has also been added - once the client work is done a client for this should be added also. [checking]: https://github.com/grpc/grpc/blob/master/doc/health-checking.md
This commit adds a new crate `tonic-health` which implements the [standard GRPC Health Checking][checking] protocol. Currently there is only a server implementation, though others have alluded in the discussion in #135 that client implementations exist which could also be imported as necessary. A example server has also been added - once the client work is done a client for this should be added also. [checking]: https://github.com/grpc/grpc/blob/master/doc/health-checking.md
This commit adds a new crate `tonic-health` which implements the [standard GRPC Health Checking][checking] protocol. Currently there is only a server implementation, though others have alluded in the discussion in #135 that client implementations exist which could also be imported as necessary. A example server has also been added - once the client work is done a client for this should be added also. [checking]: https://github.com/grpc/grpc/blob/master/doc/health-checking.md Fixes #135.
Awesome |
Feature Request
Motivation
gRPC defines a standard health checking service, here. This is a somewhat common requirement for services.
Proposal
Implement a crate which allows this to be implemented via a simple trait and exposed via the Tonic
add_service
function.Alternatives
Implement this as a crate outside tonic. This may be a better option, but I don't think there is a better place to track it than here regardless of which path we take implementing it.
The text was updated successfully, but these errors were encountered: