You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "morazow (via GitHub)" <gi...@apache.org> on 2026/08/26 10:53:27 UTC
[PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
morazow opened a new pull request, #4115:
URL: https://github.com/apache/fluss/pull/4115
## Summary
Makes `coordinator.numberOfReplicas > 1` safe and observable in the Helm chart, per #4105. A standby coordinator binds the same client port as the leader but rejects every RPC except `apiVersions` with `NotCoordinatorLeaderException`, so the TCP readiness probe cannot tell a healthy standby from a wedged one — and `Admin#getClusterHealth()` could not answer on a standby at all.
**Server** (`1e79baf`):
- Allow `GET_CLUSTER_HEALTH` through the standby rejection gate in `FlussRequestHandler` (the standby init path already documents that its RPC server exists "just to serve for health check").
- Add `optional bool is_leader` / `optional bool leader_elected` to `GetClusterHealthResponse` (proto2-additive, wire-compatible both directions). A standby answers with `status=UNKNOWN`, zeroed counts, and its election state; the leader path is unchanged apart from reporting its role.
- Add a `coordinator` role to `ClusterHealthReadinessCheck` and `readiness-check.sh` (`--role` / `READINESS_ROLE`, default `tablet` — fully backward-compatible). Readiness semantics: the **leader** is Ready regardless of cluster color (its readiness controls the bootstrap DNS record, so it must not depend on tablet health — the tablet probes already gate rolling upgrades on cluster recovery); a **standby** is Ready only when the group has an elected leader, so "all coordinator pods Ready" certifies a functioning group. A standby of an older version rejects the RPC; the probe maps that to the existing API-unsupported exit code, latching the TCP fallback so mixed-version rolling upgrades never wedge.
**Helm + docs** (`bf16354`):
- Coordinator readiness becomes the role-aware exec probe (liveness stays TCP); knobs under `coordinator.readinessProbe.*` mirror the tablet ones, including `healthCheckAuth` for SASL.
- New "Coordinator High Availability" documentation: multi-replica client bootstrap (list every replica in `bootstrap.servers`), recommended PodDisruptionBudget (`maxUnavailable: 1`), rolling-update behavior, and probe semantics. Removes the now-stale note that a TCP check is sufficient for coordinators.
- Defaults stay at 1 replica — this makes >1 safe, it does not change defaults.
Fixes #4105.
## Test Plan
- New `ClusterHealthReadinessCheckTest`: coordinator-role evaluation matrix (leader ready on any color; standby ready iff leader elected; missing role fields treated as old leader-served response; invalid role is a config error).
- `CoordinatorHighAvailabilityITCase#testClusterHealthServedByLeaderAndStandby`: two live coordinators — the leader answers GREEN with `is_leader=true`, the standby answers `UNKNOWN` with `is_leader=false, leader_elected=true` instead of rejecting.
- Existing suites re-run green: `CoordinatorHighAvailabilityITCase` (7 tests), `ClusterHealthTest`, `TabletServiceITCase` (tablet forwarding path), full `fluss-rpc` tests.
- Chart: `helm lint` + `helm unittest` (153 tests, incl. new `readiness_probe_test.yaml`: default rendering, knob overrides, SASL passthrough, tablet defaults unchanged).
- Pending (draft): live multi-replica validation on Kubernetes — cold start ordering, leader kill/failover, rolling restart, old-image + new-chart skew.
🤖 AI-assisted changes - reviewed by human developer
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on code in PR #4115:
URL: https://github.com/apache/fluss/pull/4115#discussion_r4452339263
##########
fluss-rpc/src/main/java/org/apache/fluss/rpc/netty/server/FlussRequestHandler.java:
##########
@@ -65,7 +65,12 @@ public void processRequest(FlussRequest request) {
request.getPrincipal()));
// check if the coordinator server is the current leader if the API is a coordinator
// TODO: we should only check coordinator APIs instead of all APIs
- if (isCoordinator && api.getApiKey() != ApiKeys.API_VERSIONS) {
+ // GET_CLUSTER_HEALTH is answerable by a standby coordinator: it reports the
+ // server's own role and election state so a readiness probes can tell
+ // a healthy standby from one that is hung.
+ if (isCoordinator
+ && api.getApiKey() != ApiKeys.API_VERSIONS
+ && api.getApiKey() != ApiKeys.GET_CLUSTER_HEALTH) {
Review Comment:
Thanks @polyzos , applied 🤝
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on code in PR #4115:
URL: https://github.com/apache/fluss/pull/4115#discussion_r4409613048
##########
fluss-client/src/main/java/org/apache/fluss/client/admin/ClusterHealth.java:
##########
@@ -34,18 +38,24 @@ public final class ClusterHealth {
private final int numLeaderReplicas;
private final int activeLeaderReplicas;
private final ClusterHealthStatus status;
+ private final boolean servedByLeader;
+ private final boolean leaderElected;
public ClusterHealth(
Review Comment:
With an opt-in flag, I think the public API changes here, in `Admin` and in the Rust struct can be reverted. Only the readiness CLI reads the role fields, and it uses the raw response. That also avoids the constructor break, and the Rust one: new public fields break struct literals, and `#[non_exhaustive]` would too.
##########
fluss-rpc/src/main/java/org/apache/fluss/rpc/netty/server/FlussRequestHandler.java:
##########
@@ -65,7 +65,12 @@ public void processRequest(FlussRequest request) {
request.getPrincipal()));
// check if the coordinator server is the current leader if the API is a coordinator
// TODO: we should only check coordinator APIs instead of all APIs
- if (isCoordinator && api.getApiKey() != ApiKeys.API_VERSIONS) {
+ // GET_CLUSTER_HEALTH is answerable by a standby coordinator: it reports the
+ // server's own role and election state so a readiness probes can tell
+ // a healthy standby from one that is hung.
+ if (isCoordinator
+ && api.getApiKey() != ApiKeys.API_VERSIONS
+ && api.getApiKey() != ApiKeys.GET_CLUSTER_HEALTH) {
Review Comment:
Could standby answers be opt-in per request, e.g. an **optional bool allow_standby** on `GetClusterHealthRequest`? Since #4901, `NotCoordinatorLeaderException` is an `InvalidMetadataException`, so callers can refresh metadata and retry after a leader change. With this change, a client on a stale coordinator gets a successful `UNKNOWN` response instead and never gets that signal. Only the readiness probe needs the standby answer. Older servers ignore the field and keep rejecting the request, which the probe already handles.
Nit: "a readiness probes" → "a readiness probe".
##########
helm/templates/sts-coordinator.yaml:
##########
@@ -140,12 +140,31 @@ spec:
tcpSocket:
port: {{ .Values.listeners.client.port }}
readinessProbe:
- failureThreshold: 100
- timeoutSeconds: 1
- initialDelaySeconds: 10
- periodSeconds: 3
- tcpSocket:
- port: {{ .Values.listeners.client.port }}
+ failureThreshold: {{ .Values.coordinator.readinessProbe.failureThreshold | default 360 }}
+ timeoutSeconds: {{ .Values.coordinator.readinessProbe.timeoutSeconds | default 10 }}
+ initialDelaySeconds: {{ .Values.coordinator.readinessProbe.initialDelaySeconds | default 15 }}
+ periodSeconds: {{ .Values.coordinator.readinessProbe.periodSeconds | default 5 }}
+ exec:
+ command:
+ - /bin/bash
+ - -c
+ - |
+ export FLUSS_SERVER_ID=${POD_NAME##*-}
+ # Probe the LOCAL coordinator: a leader passes regardless of cluster
+ # color; a standby passes only when the coordinator group currently
+ # has an elected leader — so a standby that binds its port but never
+ # joins the group stays NotReady instead of looking healthy via TCP.
+ export READINESS_ROLE=coordinator
+ export READINESS_TIMEOUT_MS={{ .Values.coordinator.readinessProbe.rpcTimeoutMs | default 5000 }}
+ export READINESS_TCP_PORT={{ .Values.listeners.client.port }}
+ # bind.listeners binds the coordinator's CLIENT endpoint to ${POD_IP},
+ # not 0.0.0.0, so we must probe the pod's own IP instead of 127.0.0.1.
+ export READINESS_TCP_HOST=${POD_IP}
+ export READINESS_HEALTH_CHECK_TIMEOUT_SECONDS={{ .Values.coordinator.readinessProbe.healthCheckTimeoutSeconds | default 1200 }}
+ {{- with .Values.coordinator.readinessProbe.healthCheckAuth }}
Review Comment:
With client SASL on and `healthCheckAuth` unset, every coordinator, the leader included, stays NotReady for 20-minutes. Existing SASL users would hit this on upgrade as a stalled rollout. Could `_validate.tpl` reject this combination? We need a clear upgrade error. The tablet probe has the same gap
##########
website/docs/install-deploy/deploying-with-helm.md:
##########
@@ -870,14 +876,71 @@ tablet:
| Parameter | Default | Description |
|--------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------|
| `rpcTimeoutMs` | `5000` | Timeout in milliseconds for the RPC call to the Coordinator. |
-| `failureThreshold` | `200` | Max consecutive probe failures before marking the pod as unready. With `periodSeconds=5`, this allows up to ~16 minutes for recovery. |
+| `failureThreshold` | `360` | Max consecutive probe failures before marking the pod as unready. With `periodSeconds=5`, this allows up to ~30 minutes for recovery. |
| `periodSeconds` | `5` | How often the probe runs. |
:::note
-The CoordinatorServer does not need the Cluster Health API probe — it does not host data replicas, so a simple TCP check is sufficient.
The Coordinator should be upgraded **after** all TabletServers are fully upgraded and recovered.
:::
+### Coordinator High Availability
+
+Set `coordinator.numberOfReplicas` above `1` to run the CoordinatorServer with high availability.
+One replica is elected leader. The others run as warm standbys and take over through leader
+election when the leader fails. The default is `1`, so high availability is opt-in.
+
+```yaml
+coordinator:
+ numberOfReplicas: 3
+ podDisruptionBudget:
+ enabled: true
+ maxUnavailable: 1
+```
+
+#### Coordinator Readiness Probe
+
+A standby coordinator binds the same client port as the leader, so a TCP check cannot tell a
+healthy standby from a hung one. The coordinator readiness probe instead asks the local server
+for its role and election state:
+
+- The **leader** is Ready with any cluster health status. Its readiness controls the DNS record
Review Comment:
After #4925, coordinator DNS records are published while the pod is NotReady, so this reason no longer holds. Maybe "so a coordinator rollout does not depend on TabletServer health"?
##########
website/docs/install-deploy/deploying-with-helm.md:
##########
@@ -870,14 +876,71 @@ tablet:
| Parameter | Default | Description |
|--------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------|
| `rpcTimeoutMs` | `5000` | Timeout in milliseconds for the RPC call to the Coordinator. |
-| `failureThreshold` | `200` | Max consecutive probe failures before marking the pod as unready. With `periodSeconds=5`, this allows up to ~16 minutes for recovery. |
+| `failureThreshold` | `360` | Max consecutive probe failures before marking the pod as unready. With `periodSeconds=5`, this allows up to ~30 minutes for recovery. |
| `periodSeconds` | `5` | How often the probe runs. |
:::note
-The CoordinatorServer does not need the Cluster Health API probe — it does not host data replicas, so a simple TCP check is sufficient.
The Coordinator should be upgraded **after** all TabletServers are fully upgraded and recovered.
:::
+### Coordinator High Availability
+
+Set `coordinator.numberOfReplicas` above `1` to run the CoordinatorServer with high availability.
+One replica is elected leader. The others run as warm standbys and take over through leader
+election when the leader fails. The default is `1`, so high availability is opt-in.
+
+```yaml
+coordinator:
+ numberOfReplicas: 3
+ podDisruptionBudget:
+ enabled: true
+ maxUnavailable: 1
+```
+
+#### Coordinator Readiness Probe
+
+A standby coordinator binds the same client port as the leader, so a TCP check cannot tell a
+healthy standby from a hung one. The coordinator readiness probe instead asks the local server
+for its role and election state:
+
+- The **leader** is Ready with any cluster health status. Its readiness controls the DNS record
+ that clients bootstrap against, so it must not depend on TabletServer health. The TabletServer
+ probes already hold a rolling upgrade until the cluster has recovered.
+- A **standby** is Ready only while the coordinator group has an elected leader. When every
Review Comment:
Since the probe falls back to TCP after the first pass, "becomes Ready once the coordinator group has an elected leader" would be more accurate.
##########
website/docs/install-deploy/deploying-with-helm.md:
##########
@@ -870,14 +876,71 @@ tablet:
| Parameter | Default | Description |
|--------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------|
| `rpcTimeoutMs` | `5000` | Timeout in milliseconds for the RPC call to the Coordinator. |
-| `failureThreshold` | `200` | Max consecutive probe failures before marking the pod as unready. With `periodSeconds=5`, this allows up to ~16 minutes for recovery. |
+| `failureThreshold` | `360` | Max consecutive probe failures before marking the pod as unready. With `periodSeconds=5`, this allows up to ~30 minutes for recovery. |
| `periodSeconds` | `5` | How often the probe runs. |
:::note
-The CoordinatorServer does not need the Cluster Health API probe — it does not host data replicas, so a simple TCP check is sufficient.
The Coordinator should be upgraded **after** all TabletServers are fully upgraded and recovered.
:::
+### Coordinator High Availability
+
+Set `coordinator.numberOfReplicas` above `1` to run the CoordinatorServer with high availability.
+One replica is elected leader. The others run as warm standbys and take over through leader
+election when the leader fails. The default is `1`, so high availability is opt-in.
+
+```yaml
+coordinator:
+ numberOfReplicas: 3
+ podDisruptionBudget:
+ enabled: true
+ maxUnavailable: 1
+```
+
+#### Coordinator Readiness Probe
+
+A standby coordinator binds the same client port as the leader, so a TCP check cannot tell a
+healthy standby from a hung one. The coordinator readiness probe instead asks the local server
+for its role and election state:
+
+- The **leader** is Ready with any cluster health status. Its readiness controls the DNS record
+ that clients bootstrap against, so it must not depend on TabletServer health. The TabletServer
+ probes already hold a rolling upgrade until the cluster has recovered.
+- A **standby** is Ready only while the coordinator group has an elected leader. When every
+ coordinator pod is Ready, the group has a working leader, and a rolling update stops at a pod
+ whose group has none.
+
+The election check runs only while a pod starts. Once it passes, or once the
+`healthCheckTimeoutSeconds` budget (20 minutes by default) runs out, the probe switches to a
+TCP-only check for the rest of the pod's life. A later ZooKeeper outage therefore cannot mark
+every coordinator NotReady at the same time. The probe parameters are the same as the
+TabletServer ones, under `coordinator.readinessProbe.*`. Set `healthCheckAuth` when the client
+listener enforces SASL.
+
+The probe script ships inside the Fluss image, so this chart version needs an image whose
+cluster health API reports the coordinator role. With an older image that ships
Review Comment:
The old script ignores the role and probes in tablet mode, so the leader also waits for GREEN or for the budget to run out, not just standbys. Could the docs say so?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "polyzos (via GitHub) via issues" <is...@fluss.apache.org>.
polyzos commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6600239609
Thanks @morazow, all the points are addressed 🙏
LGTM once the CI passes I will merge 👍 . A few non-blocking things just FYI:
- SASL users without `healthCheckAuth` will now get a render error on chart upgrade. Could we add this to the release/upgrade notes so it doesn't catch anyone out?
- Users who keep SASL credentials in `existingSecret` now have to put a password in `healthCheckAuth`, and it ends up in the StatefulSet spec. Since the probe command is a double-quoted bash string, a `${ENV_VAR}` pointing at a secret should work, but I haven't tested it. If it does, maybe we should document it.
- After a leader crashes (not a clean shutdown), its ZooKeeper node stays until the session times out. A standby starting in that window would go Ready against a dead leader. It only affects a pod's first start, so a one-line mention in the docs is probably enough.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6602496792
Hello @polyzos,
Now using #4391 `ClusterHealth` RPC, the PR becomes leaner since we don't have to introduce new RPC calls and protobuf generation.
Mainly:
- No more server or protocol changes. I removed `allow_standby` from `GetClusterHealthRequest` and `is_leader / leader_elected` from `GetClusterHealthResponse`. I also reverted the `FlussRequestHandler` exception for cluster health, the standby-side ZooKeeper read of the leader node, and the regenerated Rust protobuf code.
- The leader still uses getClusterHealth. The probe sends a plain GetClusterHealthRequest to the local coordinator. If the leader answers, it is Ready with any cluster health status, as before.
- A standby is now recognized by its rejection. A standby answers the same plain request with NotCoordinatorLeaderException, which shows that it handles requests. Previously the probe treated that exception as "old version, fall back to TCP". Now it means "this is a standby".
- The standby check uses DescribeCluster from #4391. After the rejection, the probe calls describeCluster on the other coordinators until the leader answers. The standby is Ready when the leader lists its advertised address in coordinator_standbys. Before, the check was "the group has an elected leader". Now it is "the leader is up and sees this standby as registered in ZooKeeper".
- New probe inputs. `--coordinators / READINESS_COORDINATORS` gives the client addresses of all coordinators. And `--advertisedAddress / READINESS_ADVERTISED_ADDRESS` gives the address this coordinator advertises. Helm chart sets both from the StatefulSet pod DNS names and the pod's `advertised.listeners` value.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6490502124
Thanks @morazow, this is a really nice improvement and overall LGTM 👍
The main two inline things for me are: making standby answers opt-in and the SASL guard in the chart.
A few more things :
- Could you rebase onto `main`? #4925 publishes not-ready addresses on the coordinator Service, which the docs' DNS reasoning relies on, and it already includes your last commit. #4901 made `NotCoordinatorLeaderException` retriable, which affects the standby-answer design.
- The "Known limitation" in the description refers to a TODO on `isLeaderElected` that's no longer there.
- Is the pending live Kubernetes validation planned before merge?
- Could you squash the "address code review findings" and "refactorings" commits into the commits they fix?
Thanks again!
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "polyzos (via GitHub) via issues" <is...@fluss.apache.org>.
polyzos merged PR #4115:
URL: https://github.com/apache/fluss/pull/4115
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6598877987
Thanks @polyzos for the review! I have applied as you suggested 🤝
I have also run multiple validations using the kind cluster:
```
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "polyzos (via GitHub) via issues" <is...@fluss.apache.org>.
polyzos commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6604581862
Thanks @morazow, the `DescribeCluster` version is much leaner, and it takes care of my earlier #4901 concern 👍
A few things before we merge:
1. **Timeouts:** if one coordinator times out, the standby check stops and returns NotReady instead of asking the next one. So a single hung coordinator can keep a healthy standby NotReady for the full 20-minute budget, which is the case this PR is meant to handle. Could we query the coordinators in parallel, or at least move on to the next one after a timeout? The timeout log message also names the wrong RPC and host.
2. **Nit:** "API unsupported" can come from any older coordinator, including a 1.0 standby, not only a leader. Only the comment and log wording need changing.
3. **Crash window:** the session-timeout doc note seems to have been dropped. If I'm reading it right, a coordinator that crashes and restarts within the ZK session timeout can pass the probe based on its old registration. A one-line mention in the docs should be enough.
4. **PR description:** it still describes the `allow_standby` design. Could you update it, since it ends up in the squash commit?
5. **Kind validation:** could you share the rerun results for the new design? Ideally they'd include a standby the leader doesn't list and a coordinator that doesn't respond.
6. Could you open a follow-up issue for env var interpolation in `healthCheckAuth`?
I'll merge once these are addressed and CI is green 🙏
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on code in PR #4115:
URL: https://github.com/apache/fluss/pull/4115#discussion_r4452375523
##########
helm/templates/sts-coordinator.yaml:
##########
@@ -140,12 +140,31 @@ spec:
tcpSocket:
port: {{ .Values.listeners.client.port }}
readinessProbe:
- failureThreshold: 100
- timeoutSeconds: 1
- initialDelaySeconds: 10
- periodSeconds: 3
- tcpSocket:
- port: {{ .Values.listeners.client.port }}
+ failureThreshold: {{ .Values.coordinator.readinessProbe.failureThreshold | default 360 }}
+ timeoutSeconds: {{ .Values.coordinator.readinessProbe.timeoutSeconds | default 10 }}
+ initialDelaySeconds: {{ .Values.coordinator.readinessProbe.initialDelaySeconds | default 15 }}
+ periodSeconds: {{ .Values.coordinator.readinessProbe.periodSeconds | default 5 }}
+ exec:
+ command:
+ - /bin/bash
+ - -c
+ - |
+ export FLUSS_SERVER_ID=${POD_NAME##*-}
+ # Probe the LOCAL coordinator: a leader passes regardless of cluster
+ # color; a standby passes only when the coordinator group currently
+ # has an elected leader — so a standby that binds its port but never
+ # joins the group stays NotReady instead of looking healthy via TCP.
+ export READINESS_ROLE=coordinator
+ export READINESS_TIMEOUT_MS={{ .Values.coordinator.readinessProbe.rpcTimeoutMs | default 5000 }}
+ export READINESS_TCP_PORT={{ .Values.listeners.client.port }}
+ # bind.listeners binds the coordinator's CLIENT endpoint to ${POD_IP},
+ # not 0.0.0.0, so we must probe the pod's own IP instead of 127.0.0.1.
+ export READINESS_TCP_HOST=${POD_IP}
+ export READINESS_HEALTH_CHECK_TIMEOUT_SECONDS={{ .Values.coordinator.readinessProbe.healthCheckTimeoutSeconds | default 1200 }}
+ {{- with .Values.coordinator.readinessProbe.healthCheckAuth }}
Review Comment:
Done
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6601022216
Hello @polyzos, thanks!
For the points:
1) Added this to the helm section to the 1.1 upgrade notes
2) Good catch, the new check also rejected the existing `secrets.env` route (READINESS_HEALTH_CHECK_AUTH from a secretKeyRef). The check now accepts it and the docs also reflect it. For interpolation, I think we can come back to it again, I think maybe we need to fix first pasword interpolations so that a password containing `$` doesn't fail, e.g, `${pass$word}`
3) Documented this session-timeout window accordingly 🤝
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6605640642
Thanks @polyzos for the careful review! All points are addressed:
- Addressed points 1 - 4.
- Here is kind setup and validations:
- Setup: 3 coordinators, image built from this branch, leader coordinator-server-2
- Rolling update: finished in 64s. Each standby logged r`ole=standby, listed by leader at coordinator-server-2...` and there was no TCP fallback.
- A standby the leader does not list: NotReady (`is not listed by leader at coordinator-server-2...`). I simulated this by running the probe with an advertised address the leader doesn't know.
- A coordinator that doesn't respond: I froze `coordinator-server-0` with `SIGSTOP`. It is a standby listed before the leader, so its port accepts connections but never answers. The probe skipped it and returned Ready in 7.3s, within the 10s probe timeout. A recreated `coordinator-server-1` became Ready 28s after deletion while `-0` was still frozen. After `SIGCONT`, the `-0` registered again and the leader listed it.
- Leader failover: deleting `coordinator-server-2` made `-1` the leader. The recreated `-2` was Ready in 25s as a standby listed by the new leader.
Creted the follow-up issue: #5035
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [helm] First-class coordinator HA: standby-aware readiness gate [fluss]
Posted by "morazow (via GitHub) via issues" <is...@fluss.apache.org>.
morazow commented on PR #4115:
URL: https://github.com/apache/fluss/pull/4115#issuecomment-6601160798
Hello @polyzos , let's wait on this PR a bit.
I realized that https://github.com/apache/fluss/pull/4391 is merged, we can maybe simplify parts of RPC on this PR. Going to adapt 🤝
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org