apache / apache/pulsar

PIP-172 : Introduce the `HEALTH_CHECK` command in the binary protocol

Open
#15,859 14 comments 3 reactions 1 assignee Claimed by @coderzc View on GitHub
Stale type/PIP
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

Mailing list thread: https://lists.apache.org/thread/6td3wyfybsys4vf1tgf7fxy36w10nb5k

## Motivation

Currently, the broker has an admin [healthCheck](https://github.com/apache/pulsar/blob/d5cfc9dc41d35d5c2d30ca16c3e8630ae63ac8c1/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L342) endpoint, which is very good. But in some scenarios, we may not be able to use the admin API (e.g [auto failover on client side](https://github.com/apache/pulsar/pull/13316#discussion_r773313991)), so I propose to introduce the HEALTH_CHECK command in the broker binary protocol.

## Goal

This PIP proposes to introduce the `HEALTH_CHECK` command in the broker binary protocol for probe clusters is healthy.

### How to do check health
* Plan A:
Keep pace with admin [healthCheck](https://github.com/apache/pulsar/blob/d5cfc9dc41d35d5c2d30ca16c3e8630ae63ac8c1/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L342) endpoint, the client sends the `HEALTH_CHECK` command to one broker service, the broker sends a message to the topic associated with this broker and uses reader API to check that message. (ref: [internalRunHealthCheck](https://github.com/apache/pulsar/blob/7800fbd5d8cdbeb307f98a8d9408bb5309f41c0d/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L385))
This Plan only can check the current broker is healthy and it can't ensure each broker is healthy, but it is simple and effective enough.

* Plan B:
The client sends the `HEALTH_CHECK` command to all broker services, each broker uses the same way as Plan A to check whether a current broker is healthy.
This Plan can ensure each broker is healthy, but it will increase a lot of costs for the client such as CPU load and memory overhead, and The time for health check handle too long.

* Plan C:
The client sends the `HEALTH_CHECK` command to one broker service, the broker sends a message to each broker and uses reader API to check them.
This Plan can ensure each broker is healthy, but it will increase a lot of costs for the broker.

I think we can implement a health check like `Plan A`, it is effective enough and covers most of the cases.

For reducing broker load the broker will be triggered `healthCheck` by the client and increase the lock to ensure its serial execution. The broker will cache results if the broker is healthy, for another client request broker will return cache results. And the cache expiry time should be configured.

## Configuration Change

Add a new config `healthCheckCacheInSeconds` in the `broker.conf`, it expresses health check result cache expiry time that unit is seconds.

Add a new config `healthCheckTimeoutInSeconds` in the `broker.conf`, it expresses health check request wait for lock and internal check timeout that unit is seconds.

## Protocol Changes

```proto
message CommandHealthCheck {
required uint64 request_id = 1;
}

message CommandHealthCheckResponse {
required uint64 request_id = 1;
required bool ok = 2 [default = false];

optional ServerError error_code = 3;
optional string error_message = 4;
}
```
When the `ok == true` indicates that the cluster is in a healthy state.
Conversely, when the ok == false that the cluster is unhealthy, the specific error message can view `error_code` and `error_message`.

## Client API Changes

```java
Class HealthCheckResult {
private boolean ok;
private int errorCode;
private String errorMessage;
}

interface PulsarClient {
// ....
CompletableFuture healthCheck();
}
```

We can use the API to check the connected cluster, such as:
```java
PulsarClient pulsarClient = PulsarClient.builder().serviceUrl(url).build();

HealthCheckResult healthCheckResult = pulsarClient.healthCheck().get();

if (healthCheckResult.isOk()) {
// This cluster is healthy currently
// ......
} else {
// This cluster is not healthy currently
log.info("The cluster is not healthy, reason: {}", healthCheckResult.getErrorMessage());
// ......
}
```

## Implementation

* Add a handler function to handle `CommandHealthCheck` at `ServerCnx`.
* Add a handler function to handle `CommandHealthCheckResponse` at `ClientCnx`.
* Add a ServiceNameResolver in the PulsarClient to resolve the serviceUrl.
* Use `client.getCnxPool().getConnection(serviceNameResolver.resolveHost())` get a connection.
* In order to better reuse some functions, move the [internalRunHealthCheck](https://github.com/apache/pulsar/blob/7800fbd5d8cdbeb307f98a8d9408bb5309f41c0d/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L385) and [checkDeadlockedThreads](https://github.com/apache/pulsar/blob/7800fbd5d8cdbeb307f98a8d9408bb5309f41c0d/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java#L364) to `BrokerService`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.