avantgardnerio commented on code in PR #2088:
URL:
https://github.com/apache/datafusion-ballista/pull/2088#discussion_r3609461366
##########
ballista/executor/src/executor_process.rs:
##########
@@ -126,6 +128,8 @@ pub struct ExecutorProcessConfig {
pub port: u16,
/// Port for the executor's gRPC service.
pub grpc_port: u16,
+ /// Port for the executor's HTTP health server (`/healthz` and `/readyz`).
+ pub health_port: u16,
Review Comment:
@phillipleblanc , I wasn't aware you (or anyone) was using it that way.
Given your use case (should we add this to a persona @andygrove ?) I think you
are absolutely correct about architecture, and I was negligent in the design.
PTAL at the current state. I have:
1. removed axum from the executor library
2. exposed a boolean is_healthy() method in both scheduler and executor
3. moved axum & the health check server into the executor binary
4. not changed the scheduler (this is the one I'm not sure about)
My thought is that if schedulers are already starting a REST API, it is
natural to put the health check endpoint there so as to not require another
port binding. However, I can see how this might be a separate concern from the
REST API.
Happy to entertain either option...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
