asafm commented on issue #20389:
URL: https://github.com/apache/pulsar/issues/20389#issuecomment-1784190582
Few comments before you continue with implementation:
1. Please refactor out the core logic in BrokerBase.`internalRunHealthCheck`
into it's own `HealthCheckService`.
2. Please refrain from adding more code to `BrokerService` - this class is
3600 lines - what good reason do you have adding more lines to this class?
`HealthCheckService` sounds likes a reasonable place to host the scheduling
part.
3. Please don't user `scheduleAtFixedRate` since you can overload the
internal queue of the executor service. You can use `scheduleAtFixedDelay`.
4. You intend to place `rMetrics.put("brk_health", healthCheckStatus);` in
`getConnectionMetrics()` but as far as I know, this metric as nothing to do
with metrics of connections. Can you please find / create a more suitable place?
--
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]
