From d1c2c42c90c878d7b92603d6ba19c257ec458db4 Mon Sep 17 00:00:00 2001 From: houseme Date: Fri, 24 Jul 2026 21:32:44 +0800 Subject: [PATCH] fix(internode): align msgpack benchmark gates (#5197) Update the internode gRPC benchmark P2 env output to require both the msgpack-only request flag and the fleet-confirmed gate before measuring a true msgpack-only phase. Document the dual-gate rollback semantics and add a dry-run script test so the benchmark driver cannot silently regress to the single-flag flow. Co-authored-by: heihutu --- .config/make/tests.mak | 1 + .../architecture/unified-object-generation.md | 2 +- .../internode-grpc-benchmark-runbook.md | 15 ++++---- scripts/run_internode_grpc_ab_bench.sh | 23 ++++++++++--- scripts/test_internode_grpc_ab_bench.sh | 34 +++++++++++++++++++ 5 files changed, 62 insertions(+), 13 deletions(-) create mode 100755 scripts/test_internode_grpc_ab_bench.sh diff --git a/.config/make/tests.mak b/.config/make/tests.mak index 73ff9aaeb..9825d505b 100644 --- a/.config/make/tests.mak +++ b/.config/make/tests.mak @@ -26,6 +26,7 @@ script-tests: ## Run shell script tests @echo "Running script tests..." ./scripts/test_build_rustfs_options.sh ./scripts/test_entrypoint_credentials.sh + ./scripts/test_internode_grpc_ab_bench.sh ./scripts/test_object_batch_bench_enhanced.sh bash -n ./scripts/validate_object_data_cache_cold_stampede.sh python3 ./scripts/check_object_data_cache_follower_samples.py --self-test diff --git a/docs/architecture/unified-object-generation.md b/docs/architecture/unified-object-generation.md index e4e6535e5..fffed4e74 100644 --- a/docs/architecture/unified-object-generation.md +++ b/docs/architecture/unified-object-generation.md @@ -182,7 +182,7 @@ The on-disk persistence of generation must not perturb the file format: The internode RPC layer is mid-migration from JSON to msgpack binary, and generation-bearing fields must respect that migration window — this is not optional context, it changes how epoch is transported. - **Dual-field transport.** Each dual-encoded RPC field exists twice in `crates/protos/src/node.proto`: a JSON `string` field and a msgpack `bytes _bin` field (e.g. `file_info` #4 alongside `file_info_bin` #7 on `RenameDataRequest`). Senders emit both; receivers `decode_msgpack_or_json` prefer the `_bin` form and fall back to the JSON string only when `_bin` is empty (`crates/ecstore/src/cluster/rpc/remote_disk.rs`). -- **Capability flag, default off.** `rustfs_protos::internode_rpc_msgpack_only()` (env `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY`, default **false**) gates dropping the redundant JSON copy. It may only be flipped after the `record_msgpack_json_fallback` metric reads zero fleet-wide and the convergence runbook is followed (`crates/protos/src/lib.rs:146`). **Reuse this exact capability + metric-reads-zero model as the mixed-version gate for generation** rather than inventing a parallel handshake; the section above ("Capability negotiation") is layered on top of it, not instead of it. +- **Capability flags, default off.** `rustfs_protos::internode_rpc_msgpack_only()` only drops the redundant JSON copy when both `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true` and `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=true` are deliberately enabled after the `record_msgpack_json_fallback` metric reads zero fleet-wide and the convergence runbook is followed. If only the request flag is set, RustFS keeps dual-writing JSON compatibility fields. **Reuse this exact capability + metric-reads-zero model as the mixed-version gate for generation** rather than inventing a parallel handshake; the section above ("Capability negotiation") is layered on top of it, not instead of it. - **Generation must ride both encodings during the window.** If epoch lives in the version's internal metadata map, that map is carried inside `FileInfo`, so it is present in both the msgpack `_bin` and JSON copies automatically — good. But any new *top-level* generation datum must be added to **both** the msgpack and JSON representations (and, for msgpack, be safe under both the array and named-map encoders). A field added to only one encoding is silently lost the moment a peer falls back to the other — exactly the failure the JSON-fallback metric exists to catch. - **Signature must bind a canonical form.** Because a field is transmitted as both JSON and msgpack and a peer may consume either, the body-digest binding in "RPC signature binding" above must be computed over a single canonical representation (the msgpack `_bin` bytes) — not over whichever copy happened to be decoded. Once the `generation` capability is negotiated for a request, a fenced / generation-bearing request must **reject the JSON fallback path** so a downgrade to the unsigned/loosely-bound JSON copy cannot bypass the epoch check. diff --git a/docs/operations/internode-grpc-benchmark-runbook.md b/docs/operations/internode-grpc-benchmark-runbook.md index c9e48a28d..5e8aefffa 100644 --- a/docs/operations/internode-grpc-benchmark-runbook.md +++ b/docs/operations/internode-grpc-benchmark-runbook.md @@ -49,7 +49,7 @@ gitignored — attach the paired directories to the PR / issue. | `rustfs_system_network_internode_operation_payload_bytes` | payload size distribution (P0/P1 sizing) | | `rustfs_system_network_internode_operation_large_payloads_total` | large unary RPCs sharing the channel (P1 target) | | `rustfs_system_network_internode_dial_avg_time_nanos`, `..._dial_errors_total` | connect cost (P3 prewarm) | -| `rustfs_system_network_internode_msgpack_json_fallback_total{direction,message}` | must be **0** before enabling msgpack-only (P2) | +| `rustfs_system_network_internode_msgpack_json_fallback_total{direction,message}` | must be **0** before enabling both msgpack-only gates (P2) | | `rustfs_cluster_servers_offline_total` | offline detection correctness (P3 bypass) | | lock p99 (lock metrics) | P1 head-of-line-blocking win | @@ -66,7 +66,8 @@ column, everything else at defaults. Roll a restart between runs. | P0 msg limit | `RUSTFS_INTERNODE_RPC_MAX_MESSAGE_SIZE` | `4194304` | unset (100 MiB) | | P1 isolation | `RUSTFS_INTERNODE_CHANNEL_ISOLATION` | `false` (default) | `true` | | P1 bulk pool | `RUSTFS_INTERNODE_BULK_CHANNELS` | `1` | `2`–`4` | -| P2 msgpack-only | `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY` | `false` (default) | `true` (only after fallback counter = 0 across a window) | +| P2 msgpack-only request | `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY` | `false` (default) | `true` (only after fallback counter = 0 across a window) | +| P2 fleet confirmation | `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED` | `false` (default) | `true` (only after mixed-version, fallback-zero, soak, and rollback gates pass) | | P3 prewarm | `RUSTFS_INTERNODE_PREWARM` | `false` (default) | `true` | | P3 offline bypass | `RUSTFS_INTERNODE_OFFLINE_BYPASS` | `false` (default) | `true` | | P3 reprobe / threshold | `RUSTFS_INTERNODE_OFFLINE_REPROBE_SECS` / `RUSTFS_INTERNODE_OFFLINE_FAILURE_THRESHOLD` | defaults | `5` / `3` | @@ -86,7 +87,8 @@ column, everything else at defaults. Roll a restart between runs. - **P1** — mixed workload (large `ReadAll` + high-frequency `Refresh`). Acceptance gate from the design doc: **lock p99 down ≥ 20%** with `RUSTFS_INTERNODE_CHANNEL_ISOLATION=true`. - **P2** — observe `msgpack_json_fallback_total` across a release window; it must stay **0** - before flipping `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true` (see the msgpack convergence + before flipping both `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true` and + `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=true` (see the msgpack convergence runbook). Codec allocation via a `dhat`/`heaptrack` micro-run. - **P3** — cold-start: first cross-node op latency should drop ~one connect RTT with prewarm. Failover: `run_four_node_cluster_failover_bench.sh`, kill a node with @@ -106,8 +108,8 @@ the gate verdict (pass/fail + measured delta) in the paired directory's `summary | **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. +> must read **0** across a full release window before both msgpack-only env gates are enabled (see +> the msgpack convergence runbook). Do not benchmark P2 as before/after throughput. Artifact layout per stage (attach both halves + the diff): @@ -139,5 +141,4 @@ Captured while running the first real A/B on a 4-node ansible cluster; needed be ## Rollback -Every stage is a single-env rollback (set the env back to its baseline column and restart). -No wire-format is broken except P2 stage 2 (proto field removal), which is a separate release. +Every stage rolls back by setting the env back to its baseline column and restarting. P2 rolls back by unsetting either msgpack-only gate, or by setting both `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=false` and `RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=false`; no wire format is broken because the JSON fields, `_bin` fields, and proto field numbers remain additive. Do not remove or reuse JSON proto fields as part of this benchmark stage. diff --git a/scripts/run_internode_grpc_ab_bench.sh b/scripts/run_internode_grpc_ab_bench.sh index 7325797a1..f3acce85f 100755 --- a/scripts/run_internode_grpc_ab_bench.sh +++ b/scripts/run_internode_grpc_ab_bench.sh @@ -95,10 +95,15 @@ stage_env() { ;; p2) if [[ "${phase}" == "after" ]]; then - # Only enable after the msgpack json-fallback counter has read zero across a window. - printf '%s\n' 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true' + # Only enable after the msgpack json-fallback counter has read zero across a window and + # the fleet confirmation gate has passed. + printf '%s\n' \ + 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true' \ + 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=true' else - printf '%s\n' 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=false' + printf '%s\n' \ + 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=false' \ + 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=false' fi ;; p3) @@ -155,10 +160,18 @@ case "${STAGE}" in p0|p1|p2) echo "-- NOTE: (re)start rustfs on all nodes with the env above before this measurement is valid." [[ -x "${TRANSPORT_BENCH}" ]] || { echo "ERROR: ${TRANSPORT_BENCH} not found/executable" >&2; exit 1; } - run "${TRANSPORT_BENCH}" --out-dir "${OUT_DIR}" "${PASSTHROUGH[@]}" + if ((${#PASSTHROUGH[@]} > 0)); then + run "${TRANSPORT_BENCH}" --out-dir "${OUT_DIR}" "${PASSTHROUGH[@]}" + else + run "${TRANSPORT_BENCH}" --out-dir "${OUT_DIR}" + fi ;; p3) [[ -x "${FAILOVER_BENCH}" ]] || { echo "ERROR: ${FAILOVER_BENCH} not found/executable" >&2; exit 1; } - OUT_DIR="${OUT_DIR}" run "${FAILOVER_BENCH}" "${PASSTHROUGH[@]}" + if ((${#PASSTHROUGH[@]} > 0)); then + OUT_DIR="${OUT_DIR}" run "${FAILOVER_BENCH}" "${PASSTHROUGH[@]}" + else + OUT_DIR="${OUT_DIR}" run "${FAILOVER_BENCH}" + fi ;; esac diff --git a/scripts/test_internode_grpc_ab_bench.sh b/scripts/test_internode_grpc_ab_bench.sh new file mode 100755 index 000000000..6257d2201 --- /dev/null +++ b/scripts/test_internode_grpc_ab_bench.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +RUNNER="${SCRIPT_DIR}/run_internode_grpc_ab_bench.sh" +TMP_DIR="$(mktemp -d)" + +cleanup() { + rm -rf "$TMP_DIR" +} +trap cleanup EXIT + +"$RUNNER" --stage p2 --phase before --out-root "$TMP_DIR" --dry-run >/dev/null +"$RUNNER" --stage p2 --phase after --out-root "$TMP_DIR" --dry-run >/dev/null + +BEFORE_ENV="${TMP_DIR}/p2-before/server-env.sh" +AFTER_ENV="${TMP_DIR}/p2-after/server-env.sh" + +rg -qx 'export RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=false' "$BEFORE_ENV" +rg -qx 'export RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=false' "$BEFORE_ENV" +rg -qx 'export RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true' "$AFTER_ENV" +rg -qx 'export RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=true' "$AFTER_ENV" + +if grep -q 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY=true' "$BEFORE_ENV"; then + echo "p2 before must not request msgpack-only" >&2 + exit 1 +fi + +if grep -q 'RUSTFS_INTERNODE_RPC_MSGPACK_ONLY_FLEET_CONFIRMED=true' "$BEFORE_ENV"; then + echo "p2 before must not confirm fleet msgpack-only" >&2 + exit 1 +fi + +echo "internode grpc A/B bench env tests passed"