bocharov opened a new pull request, #400:
URL: https://github.com/apache/kvrocks-controller/pull/400
### Problem
Fixes #395. Kvrocks cluster nodes do not gossip — the controller is the sole
source of topology and must push a complete, identical view to every node.
Today a node can silently diverge and never recover:
- The probe loop re-pushes `CLUSTERX SETNODES` only when a node's epoch is
**behind** the stored version. A node that has drifted at an **equal** epoch (a
dropped push, a partial view, a stale/phantom node entry) is never repaired.
- `SETNODES` is sent without `force`, so even a re-push at an equal version
is a no-op on the server (the version gate rejects a lower version and no-ops
an equal one). The only prior recovery was `CLUSTER RESET`, which requires an
empty DB and so is unusable once a node holds data. #395 reports exactly this:
divergent `cluster_nodes` snapshots that never converge, with no
operator-facing way to force a re-push.
### Changes
1. **`SyncClusterInfo(ctx, cluster, force)`** — when `force` is set, send
the SETNODES `force` flag so the topology applies unconditionally. Because
SETNODES replaces the node's whole topology, a forced push also clears
stale/phantom entries without a `CLUSTER RESET` and without data loss. Wrapped
in a bounded retry-with-backoff (a push dropped by a transient blip is
otherwise not retried until the next tick).
2. **Divergence detection at an equal epoch** — the reconcile loop parses
`cluster_known_nodes` and `cluster_slots_ok` from `CLUSTER INFO` and
force-pushes a node whose peer count or slot coverage disagrees with the
desired topology, not only one whose epoch lags. Coverage is compared against
the desired covered-slot count (not a hardcoded 16384), so a cluster that is
intentionally mid-scale is not seen as drifted; a node that omits these fields
falls back to epoch-only reconcile. The existing "node is ahead" adopt branch
is preserved.
3. **`POST /namespaces/{namespace}/clusters/{cluster}/sync`** — force-push
the stored topology to every node and report per-node results. This is the
operator-facing re-push #395 asks for. It performs no `CLUSTER RESET`, so it is
safe to run on a populated cluster.
4. **Reject blank/port-less node addresses** in `CheckNewNodes` (the
create/add-node boundary) and `Shard.ToSlotsString` (serialization), so a
half-resolved address fails loudly instead of registering a phantom,
unreachable node.
### Safety
The controller is the single writer of topology (failover routes through
it), so "newest complete view wins" holds. Force fires only when a node's
version ≤ the store; a node legitimately ahead keeps the existing
adopt-from-node branch. Verified against Apache Kvrocks 2.16.0 that `CLUSTERX
SETNODES <str> <ver> force` bypasses the equal-version no-op.
### Tests
`make test` passing. New unit tests cover the `CLUSTER INFO` parsing (incl.
sentinel/malformed values), the divergence check, address validation, the
blank-address guard, and the `/sync` handler (force-push healthy nodes, skip
failed, report failures). A behavioral test asserts the reconcile fix by
exercising the real probe path.
Signed-off-by: Alexander Bocharoff <[email protected]>
--
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]
