From 69dcf9e6cbfacdbe257d59a85219194125100149 Mon Sep 17 00:00:00 2001 From: Henry Guo Date: Thu, 21 May 2026 13:47:32 +0800 Subject: [PATCH] fix(tooling): harden internode transport benchmark setup (#3037) * refactor(config): centralize internode transport constants * fix(bench): guard all ripgrep calls behind dry-run check Move require_cmd rg and metrics collection inside the non-dry-run path so that --dry-run works on hosts without rg installed. * feat(tooling): cross-platform protoc setup for Linux and macOS Make install-protoc.sh support Linux (x86_64, aarch64) alongside macOS, and bump CI protoc from 29.3 to 33.1 to match the version required by the gproto build script. * fix(bench): record internode baseline error counts * fix(skill): correct YAML frontmatter formatting for release-version-bump * chore(ci): bump protoc version to 34.1 * fix(tooling): bump protoc 33.1 to 34.1 in install script, restore SKILL.md description --------- Co-authored-by: Henry Guo --- .../rustfs-release-version-bump/SKILL.md | 1 - .github/actions/setup/action.yml | 2 +- crates/config/src/constants/internode.rs | 8 +++ .../src/rpc/internode_data_transport.rs | 9 ++-- scripts/install-protoc.sh | 40 +++++++++++---- scripts/run_internode_transport_baseline.sh | 4 +- scripts/run_object_batch_bench.sh | 50 ++++++++++++++----- 7 files changed, 85 insertions(+), 29 deletions(-) diff --git a/.agents/skills/rustfs-release-version-bump/SKILL.md b/.agents/skills/rustfs-release-version-bump/SKILL.md index 75222230b..7b4e0530b 100644 --- a/.agents/skills/rustfs-release-version-bump/SKILL.md +++ b/.agents/skills/rustfs-release-version-bump/SKILL.md @@ -2,7 +2,6 @@ name: rustfs-release-version-bump description: Publish a RustFS alpha/beta/stable release with an auditable flow: confirm target version and scope, update workspace and release assets (including strict rustfs.spec changelog identity/date/version format), run required verification, and finish with commit, push, and GitHub PR creation. --- - # RustFS Release Version Bump Use this skill to publish a RustFS release (alpha, beta, or stable) with a minimal, auditable diff and a complete ship flow (`edit -> verify -> commit -> push -> PR`). diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml index d30454ea1..06949d83f 100644 --- a/.github/actions/setup/action.yml +++ b/.github/actions/setup/action.yml @@ -59,7 +59,7 @@ runs: - name: Install protoc uses: rustfs/setup-protoc@v3.0.1 with: - version: "29.3" + version: "34.1" repo-token: ${{ github.token }} - name: Install flatc diff --git a/crates/config/src/constants/internode.rs b/crates/config/src/constants/internode.rs index c45febf74..003a7207e 100644 --- a/crates/config/src/constants/internode.rs +++ b/crates/config/src/constants/internode.rs @@ -36,6 +36,12 @@ pub const DEFAULT_INTERNODE_RPC_TIMEOUT_SECS: u64 = 10; pub const ENV_RUSTFS_INTERNODE_DATA_TRANSPORT: &str = "RUSTFS_INTERNODE_DATA_TRANSPORT"; pub const DEFAULT_INTERNODE_DATA_TRANSPORT: &str = "tcp-http"; +/// Legacy alias for "tcp-http". Both values select the TCP/HTTP transport backend. +pub const INTERNODE_DATA_TRANSPORT_TCP: &str = "tcp"; + +/// Known internode transport backend names accepted by the config parser. +pub const KNOWN_INTERNODE_DATA_TRANSPORT_BACKENDS: &[&str] = &[DEFAULT_INTERNODE_DATA_TRANSPORT, INTERNODE_DATA_TRANSPORT_TCP]; + #[cfg(test)] mod tests { use super::*; @@ -64,5 +70,7 @@ mod tests { assert_eq!(ENV_INTERNODE_RPC_TIMEOUT_SECS, "RUSTFS_INTERNODE_RPC_TIMEOUT_SECS"); assert_eq!(ENV_RUSTFS_INTERNODE_DATA_TRANSPORT, "RUSTFS_INTERNODE_DATA_TRANSPORT"); assert_eq!(DEFAULT_INTERNODE_DATA_TRANSPORT, "tcp-http"); + assert_eq!(INTERNODE_DATA_TRANSPORT_TCP, "tcp"); + assert_eq!(KNOWN_INTERNODE_DATA_TRANSPORT_BACKENDS, &["tcp-http", "tcp"]); } } diff --git a/crates/ecstore/src/rpc/internode_data_transport.rs b/crates/ecstore/src/rpc/internode_data_transport.rs index 7892cbb90..590ebf21c 100644 --- a/crates/ecstore/src/rpc/internode_data_transport.rs +++ b/crates/ecstore/src/rpc/internode_data_transport.rs @@ -17,12 +17,14 @@ use crate::disk::{FileReader, FileWriter}; use crate::rpc::build_auth_headers; use async_trait::async_trait; use http::{HeaderMap, HeaderValue, Method, header::CONTENT_TYPE}; -use rustfs_config::{DEFAULT_INTERNODE_DATA_TRANSPORT, ENV_RUSTFS_INTERNODE_DATA_TRANSPORT}; +use rustfs_config::{ + DEFAULT_INTERNODE_DATA_TRANSPORT, ENV_RUSTFS_INTERNODE_DATA_TRANSPORT, INTERNODE_DATA_TRANSPORT_TCP, + KNOWN_INTERNODE_DATA_TRANSPORT_BACKENDS, +}; use rustfs_rio::{HttpReader, HttpWriter}; use std::sync::{Arc, OnceLock}; use std::time::Duration; -pub const INTERNODE_DATA_TRANSPORT_TCP: &str = "tcp"; static INTERNODE_DATA_TRANSPORT: OnceLock, String>> = OnceLock::new(); const READ_FILE_STREAM_PATH: &str = "/rustfs/rpc/read_file_stream"; @@ -32,7 +34,8 @@ const CONTENT_TYPE_JSON: &str = "application/json"; fn unsupported_transport_message(transport: &str) -> String { format!( - "invalid {ENV_RUSTFS_INTERNODE_DATA_TRANSPORT}={transport:?}; supported values: {DEFAULT_INTERNODE_DATA_TRANSPORT}, {INTERNODE_DATA_TRANSPORT_TCP}" + "invalid {ENV_RUSTFS_INTERNODE_DATA_TRANSPORT}={transport:?}; supported values: {}", + KNOWN_INTERNODE_DATA_TRANSPORT_BACKENDS.join(", ") ) } diff --git a/scripts/install-protoc.sh b/scripts/install-protoc.sh index 3d85cf219..ecf3568dc 100755 --- a/scripts/install-protoc.sh +++ b/scripts/install-protoc.sh @@ -1,22 +1,42 @@ #!/usr/bin/env bash -# Install protoc 33.1 on macOS +# Install protoc 34.1 on macOS and Linux set -e -PROTOC_VERSION="33.1" +PROTOC_VERSION="34.1" ARCH=$(uname -m) INSTALL_DIR="${HOME}/.local/bin" PROTOC_BIN="${INSTALL_DIR}/protoc" -# Select download URL based on architecture -if [ "$ARCH" = "arm64" ]; then - # Apple Silicon (M1/M2/M3) - PROTOC_URL="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-osx-aarch_64.zip" -elif [ "$ARCH" = "x86_64" ]; then - # Intel Mac - PROTOC_URL="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-osx-x86_64.zip" +# Detect OS +OS="$(uname -s)" + +# Select download URL based on OS and architecture +if [ "$OS" = "Darwin" ]; then + if [ "$ARCH" = "arm64" ]; then + # Apple Silicon (M1/M2/M3) + PROTOC_URL="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-osx-aarch_64.zip" + elif [ "$ARCH" = "x86_64" ]; then + # Intel Mac + PROTOC_URL="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-osx-x86_64.zip" + else + echo "Error: Unsupported macOS architecture $ARCH" + exit 1 + fi +elif [ "$OS" = "Linux" ]; then + if [ "$ARCH" = "x86_64" ]; then + PROTOC_URL="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-linux-x86_64.zip" + elif [ "$ARCH" = "aarch64" ]; then + PROTOC_URL="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-linux-aarch_64.zip" + else + echo "Error: Unsupported Linux architecture $ARCH" + exit 1 + fi else - echo "Error: Unsupported architecture $ARCH" + echo "Error: Unsupported OS $OS" + echo "On Windows, install protoc via:" + echo " choco install protoc --version=${PROTOC_VERSION}" + echo " or download from https://github.com/protocolbuffers/protobuf/releases" exit 1 fi diff --git a/scripts/run_internode_transport_baseline.sh b/scripts/run_internode_transport_baseline.sh index 056b6bedf..e9e4022fc 100755 --- a/scripts/run_internode_transport_baseline.sh +++ b/scripts/run_internode_transport_baseline.sh @@ -125,7 +125,7 @@ setup_output() { OUT_DIR="target/bench/internode-transport-$(date +%Y%m%d-%H%M%S)" fi mkdir -p "${OUT_DIR}" - echo "scenario,endpoint,workload,concurrency,size,status,throughput,requests_per_sec,avg_latency,log_file,run_dir" > "${OUT_DIR}/summary.csv" + echo "scenario,endpoint,workload,concurrency,size,status,throughput,requests_per_sec,avg_latency,error_count,log_file,run_dir" > "${OUT_DIR}/summary.csv" if [[ -n "${INTERNODE_METRICS_URL}" ]]; then echo "scenario,workload,concurrency,size,metric,operation,before,after,delta" > "${OUT_DIR}/internode_metric_deltas.csv" fi @@ -253,7 +253,7 @@ append_object_summary() { awk -F',' -v scenario="${scenario}" -v endpoint="${endpoint}" -v workload="${workload}" -v run_dir="${run_dir}" ' NR == 1 { next } { - printf "%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s\n", scenario, endpoint, workload, $3, $1, $4, $5, $6, $7, $8, run_dir + printf "%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%s\n", scenario, endpoint, workload, $3, $1, $4, $5, $6, $7, $8, $9, run_dir } ' "${src}" >> "${OUT_DIR}/summary.csv" } diff --git a/scripts/run_object_batch_bench.sh b/scripts/run_object_batch_bench.sh index 0f35ceef2..76b9cc70a 100755 --- a/scripts/run_object_batch_bench.sh +++ b/scripts/run_object_batch_bench.sh @@ -187,7 +187,7 @@ setup_output() { fi mkdir -p "$OUT_DIR" SUMMARY_CSV="$OUT_DIR/summary.csv" - echo "size,tool,concurrency,status,throughput,requests_per_sec,avg_latency,log_file" > "$SUMMARY_CSV" + echo "size,tool,concurrency,status,throughput,requests_per_sec,avg_latency,error_count,log_file" > "$SUMMARY_CSV" } resolve_bucket() { @@ -214,6 +214,27 @@ collect_metrics() { echo "${throughput:-N/A},${reqps:-N/A},${latency:-N/A}" } +collect_error_count() { + local log_file="$1" + local count + + count="$( + { rg -io '(errors?|failures?)[[:space:]:=]+[0-9]+' "$log_file" || true; } \ + | awk '{ value = $0; gsub(/[^0-9]/, "", value); if (value != "") sum += value } END { if (NR > 0) print sum }' + )" + if [[ -n "$count" ]]; then + echo "$count" + return + fi + + if [[ "$TOOL" == "warp" ]]; then + count="$(rg -c 'warp: ' "$log_file" || true)" + else + count="$(rg -ci '(^|[[:space:]])(error|failed|failure)(:|[[:space:]])' "$log_file" || true)" + fi + echo "${count:-0}" +} + run_one() { local size="$1" local log_file="$OUT_DIR/${TOOL}_${size}.log" @@ -279,29 +300,34 @@ run_one() { fi fi - if [[ "$TOOL" == "warp" ]]; then + local metrics throughput reqps latency error_count + if [[ "$DRY_RUN" != "true" ]]; then + metrics="$(collect_metrics "$log_file")" + throughput="$(echo "$metrics" | cut -d',' -f1)" + reqps="$(echo "$metrics" | cut -d',' -f2)" + latency="$(echo "$metrics" | cut -d',' -f3)" + error_count="$(collect_error_count "$log_file")" # Warp may still exit with code 0 even when it prints runtime failures. - # Treat explicit error lines as failed runs to keep summary.csv reliable. - if rg -q 'warp: ' "$log_file"; then + if [[ "$TOOL" == "warp" && "$error_count" != "0" ]]; then status="failed" fi + else + throughput="N/A" + reqps="N/A" + latency="N/A" + error_count="N/A" fi - local metrics throughput reqps latency - metrics="$(collect_metrics "$log_file")" - throughput="$(echo "$metrics" | cut -d',' -f1)" - reqps="$(echo "$metrics" | cut -d',' -f2)" - latency="$(echo "$metrics" | cut -d',' -f3)" - - echo "$size,$TOOL,$CONCURRENCY,$status,$throughput,$reqps,$latency,$log_file" >> "$SUMMARY_CSV" + echo "$size,$TOOL,$CONCURRENCY,$status,$throughput,$reqps,$latency,$error_count,$log_file" >> "$SUMMARY_CSV" } main() { parse_args "$@" validate_args resolve_bucket - require_cmd rg if [[ "$DRY_RUN" != "true" ]]; then + require_cmd rg + require_cmd awk if [[ "$TOOL" == "warp" ]]; then require_cmd "$WARP_BIN" else