mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-21 11:56:38 +00:00
fix(obs): remove the dial9 task-dump switch that could never work; correct measured claims (#4688)
* fix(obs): remove the dial9 task-dump switch that could never do anything Measured on a bench host (Linux x86_64) against the code merged in #4663: with `RUSTFS_RUNTIME_DIAL9_TASK_DUMP_ENABLED=true`, a `dial9-taskdump` build, and `--cfg tokio_taskdump`, dial9 recorded **zero** TaskDump events. dial9 captures a task dump only for futures it wrapped itself — those spawned through `dial9_tokio_telemetry::spawn`, which is where `TaskDumped<F>` gets applied. `tokio::spawn` gets no wrapper, and RustFS spawns with `tokio::spawn` throughout. Same workload, same binary, only the spawner changed: tokio::spawn -> 0 dumps dial9::spawn -> 14709 dumps, all with callchains Upstream documents this (README line 151) and tracks the doc gap at dial9-rs/dial9#477. I did not read it before wiring `with_task_dumps` in #4663, and so shipped exactly the kind of lying configuration knob that PR set out to delete. Remove it: the two environment variables, the config fields, the `with_task_dumps` call, and the `dial9-taskdump` feature — whose only effect was to constrain the build to Linux while recording nothing. Re-adding it only makes sense together with migrating the paths under investigation to dial9's spawner. Tracked as D9-16 in rustfs/backlog#1157. Also drop the `--cfg tokio_taskdump` requirement from the Makefile. Measured: dumps are captured with and without it (14709 vs 14674, within noise), and upstream never asked for it. That requirement was mine, invented and untested. Cargo.lock loses tokio's `backtrace` dependency, which `tokio/taskdump` pulled in. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(obs): replace guessed dial9 retention numbers with measured ones Three corrections, all to claims I wrote in #4663 without measuring them. "Under a high poll rate that budget can wrap in minutes" was a guess. Measured on a single-node 4-drive cluster under warp mixed (66 MiB/s, 110 obj/s, 32 concurrent): 13023 events/s, 0.16 MiB/s, so the default 1 GiB budget wraps after roughly 108 minutes. Even at ten times the throughput that is ~11 minutes. State the measured rate and how to scale it instead. dial9 was described as the tool for drive stalls. It is not. RustFS does disk I/O on the blocking pool and through io_uring, never on an async worker, so a slow drive never lengthens a poll. Injecting 200 ms of latency on one of four drives cut throughput by 64% and left the poll distribution unchanged (polls >= 5 ms: 49 -> 56; p999: 2.67 ms -> 2.75 ms). Enabling dial9's CPU and sched profilers does not help: sched events are per-worker only, and the CPU profiler samples on-CPU while a stalled drive is an off-CPU wait. Say so plainly, and point at the `rustfs_io_*` metrics instead. What dial9 *is* good for, on the same traces: single polls of 418-625 ms with no fault injected at all — real worker stalls nothing else in the obs stack surfaces. Lead with that. Also link the two upstream issues filed for the gaps we documented: dial9-rs/dial9#658 (writer death unobservable) and #659 (worker-s3 CVEs). Measurements: rustfs/backlog#1157 (D9-11, D9-13, D9-18). Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -23,7 +23,7 @@ use super::config::Dial9Config;
|
||||
use super::state::{dial9_runtime_state, measure_disk_usage_bytes};
|
||||
use super::{EVENT_DIAL9_STATE, LOG_COMPONENT_OBS, LOG_SUBSYSTEM_DIAL9};
|
||||
use crate::TelemetryError;
|
||||
use dial9_tokio_telemetry::telemetry::{ProcessResourceUsageConfig, RotatingWriter, TaskDumpConfig, TracedRuntime};
|
||||
use dial9_tokio_telemetry::telemetry::{ProcessResourceUsageConfig, RotatingWriter, TracedRuntime};
|
||||
use std::time::Duration;
|
||||
use tracing::{info, warn};
|
||||
|
||||
@@ -115,19 +115,16 @@ pub fn build_traced_runtime(
|
||||
.with_runtime_name(RUNTIME_NAME)
|
||||
.with_process_resource_usage(ProcessResourceUsageConfig::default());
|
||||
|
||||
let traced = if config.task_dump_enabled {
|
||||
traced.with_task_dumps(
|
||||
TaskDumpConfig::builder()
|
||||
.idle_threshold(config.task_dump_idle_threshold)
|
||||
.build(),
|
||||
)
|
||||
} else {
|
||||
traced
|
||||
};
|
||||
|
||||
// `build_and_start` rather than `build`: `build` returns a live guard that
|
||||
// never records, writing segments that contain only a header.
|
||||
//
|
||||
// No `with_task_dumps` here. dial9 captures a task dump only for futures it
|
||||
// wrapped itself, i.e. those spawned via `dial9_tokio_telemetry::spawn`;
|
||||
// `tokio::spawn` gets no wrapper. RustFS spawns with `tokio::spawn`
|
||||
// throughout, so calling `with_task_dumps` records nothing. Measured on an
|
||||
// identical workload: 0 dumps via `tokio::spawn`, 14709 via `dial9::spawn`.
|
||||
// See rustfs/backlog#1157 (D9-16) and dial9-rs/dial9#477.
|
||||
//
|
||||
// No `with_s3_uploader` here: dial9's `worker-s3` feature carries a
|
||||
// vulnerable TLS stack. See the note in `crates/obs/Cargo.toml`.
|
||||
finish_traced_runtime(traced.build_and_start(builder, writer), config)
|
||||
@@ -165,7 +162,6 @@ fn finish_traced_runtime(
|
||||
output_dir = %config.output_dir,
|
||||
file_prefix = %config.file_prefix,
|
||||
disk_budget_bytes = config.total_disk_budget(),
|
||||
task_dumps = config.task_dump_enabled,
|
||||
"dial9 state changed"
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user