diff --git a/crates/io-metrics/src/internode_metrics.rs b/crates/io-metrics/src/internode_metrics.rs index a16e21603..7e121669c 100644 --- a/crates/io-metrics/src/internode_metrics.rs +++ b/crates/io-metrics/src/internode_metrics.rs @@ -467,12 +467,24 @@ fn publish_offline_gauge(peers: &HashMap) { gauge!(CLUSTER_SERVERS_OFFLINE_TOTAL).set(offline as f64); } +/// Canonicalize a peer address before using it as the `CLUSTER_PEER_HEALTH` key. +/// +/// The same physical peer is referenced by different subsystems in slightly different string forms +/// — the data path keys by `endpoint.grid_host()` (`scheme://host:port`, no trailing slash) while +/// the lock path keys by `url::Url::to_string()` (`scheme://host:port/`, trailing slash). Without +/// normalization each form becomes its own health entry, so one downed node counts as 2 in the +/// `rustfs_cluster_servers_offline_total` gauge (and 2N for N nodes). Trimming trailing slashes +/// collapses them to a single canonical key. +fn normalize_peer_key(addr: &str) -> &str { + addr.trim_end_matches('/') +} + /// Record that a cluster peer is reachable: mark it online and reset its consecutive-failure /// counter. Called on a successful dial to `addr`. pub fn record_peer_reachable(addr: &str) { // Recover from a poisoned lock so peer-health tracking and the offline gauge never stall permanently. let mut peers = CLUSTER_PEER_HEALTH.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); - let entry = peers.entry(addr.to_string()).or_default(); + let entry = peers.entry(normalize_peer_key(addr).to_string()).or_default(); entry.online = true; entry.consecutive_failures = 0; entry.last_reprobe = None; @@ -484,7 +496,7 @@ pub fn record_peer_reachable(addr: &str) { pub fn record_peer_unreachable(addr: &str, failure_threshold: u32) { // Recover from a poisoned lock so peer-health tracking and the offline gauge never stall permanently. let mut peers = CLUSTER_PEER_HEALTH.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); - let entry = peers.entry(addr.to_string()).or_default(); + let entry = peers.entry(normalize_peer_key(addr).to_string()).or_default(); entry.consecutive_failures = entry.consecutive_failures.saturating_add(1); if entry.consecutive_failures >= failure_threshold.max(1) { entry.online = false; @@ -495,7 +507,7 @@ pub fn record_peer_unreachable(addr: &str, failure_threshold: u32) { /// Whether a cluster peer is currently considered offline (known and marked offline). pub fn cluster_peer_is_offline(addr: &str) -> bool { let peers = CLUSTER_PEER_HEALTH.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); - peers.get(addr).map(|peer| !peer.online).unwrap_or(false) + peers.get(normalize_peer_key(addr)).map(|peer| !peer.online).unwrap_or(false) } /// Decide whether to fast-fail (bypass) an offline peer instead of attempting to reach it @@ -507,7 +519,7 @@ pub fn cluster_peer_is_offline(addr: &str) -> bool { /// bypassed. pub fn cluster_peer_should_bypass(addr: &str, reprobe_interval: Duration) -> bool { let mut peers = CLUSTER_PEER_HEALTH.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); - let Some(entry) = peers.get_mut(addr) else { + let Some(entry) = peers.get_mut(normalize_peer_key(addr)) else { return false; }; if entry.online { @@ -529,7 +541,19 @@ pub fn cluster_peer_should_bypass(addr: &str, reprobe_interval: Duration) -> boo #[cfg(test)] fn cluster_peer_online(addr: &str) -> Option { - CLUSTER_PEER_HEALTH.lock().ok()?.get(addr).map(|peer| peer.online) + CLUSTER_PEER_HEALTH + .lock() + .ok()? + .get(normalize_peer_key(addr)) + .map(|peer| peer.online) +} + +#[cfg(test)] +fn cluster_peer_health_keys() -> Vec { + CLUSTER_PEER_HEALTH + .lock() + .map(|peers| peers.keys().cloned().collect()) + .unwrap_or_default() } #[cfg(test)] @@ -712,6 +736,30 @@ mod tests { assert_eq!(cluster_peer_online(addr), Some(true)); } + #[test] + fn peer_health_key_is_normalized_across_address_forms() { + // Same physical peer, two address forms the codebase actually uses: the data path's + // grid_host() (no trailing slash) and the lock path's url::Url::to_string() (trailing slash). + // They MUST collapse to one health entry, else one downed node counts as 2 in the gauge. + let bare = "http://cluster-peer-health-normalize-test:9000"; + let slashed = "http://cluster-peer-health-normalize-test:9000/"; + + record_peer_unreachable(bare, 1); + record_peer_unreachable(slashed, 1); + + let keys: Vec = cluster_peer_health_keys() + .into_iter() + .filter(|k| k.contains("cluster-peer-health-normalize-test")) + .collect(); + assert_eq!(keys.len(), 1, "two address forms of one peer must be a single health entry, got {keys:?}"); + + // Either form observes the peer offline, and a reachable dial via one form clears the other. + assert!(cluster_peer_is_offline(bare)); + assert!(cluster_peer_is_offline(slashed)); + record_peer_reachable(slashed); + assert!(!cluster_peer_is_offline(bare), "reachable via one form must mark the shared entry online"); + } + #[test] fn cluster_servers_offline_total_name_is_stable() { assert_eq!(CLUSTER_SERVERS_OFFLINE_TOTAL, "rustfs_cluster_servers_offline_total"); diff --git a/docs/operations/internode-grpc-benchmark-runbook.md b/docs/operations/internode-grpc-benchmark-runbook.md index 97157d30a..c9e48a28d 100644 --- a/docs/operations/internode-grpc-benchmark-runbook.md +++ b/docs/operations/internode-grpc-benchmark-runbook.md @@ -36,8 +36,10 @@ already-running server. Use `--dry-run` to preview the env and command. compose cluster, kills `FAILOVER_NODE`, benchmarks; writes `target/bench/four-node-failover-/`). -Store the paired artifacts as `target/bench/internode-transport/{baseline,after-P0,after-P1,after-P3}/` -(the paths the design docs reference). `target/` is gitignored — attach the artifacts to the PR. +The one-click driver writes each run to `target/bench/internode-transport/-/` +(e.g. `p0-before/`, `p0-after/`, `p1-before/`, `p1-after/`, `p3-before/`, `p3-after/`), each +containing the emitted `server-env.sh` plus the underlying bench artifacts. `target/` is +gitignored — attach the paired directories to the PR / issue. ## Metrics to capture (Prometheus) @@ -91,6 +93,50 @@ column, everything else at defaults. Roll a restart between runs. `RUSTFS_INTERNODE_OFFLINE_BYPASS=true`; expect faster failover and a correct `rustfs_cluster_servers_offline_total` (1 while the node is down, back to 0 after recovery). +## Acceptance gates & artifact layout + +Each stage's paired run must satisfy an explicit gate before its numbers are accepted. Record +the gate verdict (pass/fail + measured delta) in the paired directory's `summary` and attach it. + +| Stage | Bench | Acceptance gate | Primary metric(s) | +|---|---|---|---| +| **P0** | `run_internode_transport_baseline.sh` | small-RPC `duration_ms` (DiskInfo/Ping) **down**; large-metadata (ReadMultiple/BatchReadVersion) throughput **up**; a `>4 MiB` multi-version `xl.meta` no longer fails `out_of_range` (functional). | `..._operation_duration_ms{operation}`, `..._operation_payload_bytes`, object-bench throughput | +| **P1** | `run_internode_transport_baseline.sh` (mixed: large `ReadAll` + high-frequency `Refresh`) | **lock p99 down ≥ 20%** with `RUSTFS_INTERNODE_CHANNEL_ISOLATION=true` vs baseline. | lock p99 (lock metrics), `..._operation_large_payloads_total` | +| **P3 cold-start** | `run_internode_transport_baseline.sh` (fresh cluster, first cross-node op) | first cross-node op latency **drops ~one connect RTT** with `RUSTFS_INTERNODE_PREWARM=true`. | `..._dial_avg_time_nanos`, first-op `..._operation_duration_ms` | +| **P3 offline** | dedicated *sustained-offline + survivor cross-node access* experiment (the standard four-node failover bench is **not** sensitive to the bypass — quorum holds, `recovery_seconds=0`) | with `RUSTFS_INTERNODE_OFFLINE_BYPASS=true`, survivor cross-node op latency to the downed peer **fast-fails** instead of hanging the dial timeout; `rustfs_cluster_servers_offline_total` = **1** while down, back to **0** after recovery. | `rustfs_cluster_servers_offline_total`, survivor cross-node `..._operation_duration_ms`, `..._dial_errors_total` | + +> **P2 is not a throughput gate.** Its acceptance is operational: `msgpack_json_fallback_total` +> must read **0** across a full release window before `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true` +> is enabled (see the msgpack convergence runbook). Do not benchmark P2 as before/after throughput. + +Artifact layout per stage (attach both halves + the diff): + +``` +target/bench/internode-transport/ + p0-before/ p0-after/ # server-env.sh + object-bench summaries + metric deltas + p1-before/ p1-after/ # + lock p99 delta (the ≥20% gate) + p3-before/ p3-after/ # cold-start + sustained-offline experiment + offline gauge trace +``` + +## Bench-host prerequisites (ansible bare-metal) + +Captured while running the first real A/B on a 4-node ansible cluster; needed before any live run: + +- **RPC secret is mandatory on current `main`.** Internode RPC fails closed: default creds + (`RUSTFS_SECRET_KEY=rustfsadmin`) with no `RUSTFS_RPC_SECRET` → `No valid auth token` → the cluster + never reaches `storage_quorum`. Set a **non-default** `RUSTFS_RPC_SECRET`, identical on every node. +- **systemd start timeout.** The install unit is `Type=notify`; READY only fires after quorum, which on + freshly-purged disks exceeds a 30 s `TimeoutStartSec` → crash loop. Use a drop-in `TimeoutStartSec=infinity`. +- **Server-side metrics need OTLP.** RustFS has no Prometheus pull endpoint (`/admin/v3/metrics` is NDJSON, + not exposition); it only pushes via OTLP. To capture lock/offline/internode metrics, run an + otel-collector (OTLP receiver → Prometheus exporter) and set `RUSTFS_OBS_ENDPOINT`, + `RUSTFS_OBS_METRICS_EXPORT_ENABLED=true`, `RUSTFS_OBS_METER_INTERVAL=5`. For lock p99 also set + `RUSTFS_OBJECT_LOCK_DIAG_ENABLE=true` (default off). +- **P3 offline method.** Standard failover is quorum-insensitive to the bypass. Use a *sustained-offline + + survivor cross-node access* run instead: all nodes up → stop one node (sustained) → warm up to trip + offline detection → drive warp on the survivors only (`--host` excludes the dead node) → compare + survivor op p99 and `rustfs_cluster_servers_offline_total` for `RUSTFS_INTERNODE_OFFLINE_BYPASS` off/on. + ## Rollback Every stage is a single-env rollback (set the env back to its baseline column and restart).