add: blk telemetry - #564
VasilyStepanov wants to merge 2 commits into
Conversation
blk_session had no telemetry at all, and ost_session's per-attempt slat/rtt/clat/op tracking lived under a generic, unnamespaced telemetry API despite being ost-only. Move it under rawstor::telemetry::ost, and give rawstor::telemetry::blk its own equivalent (slat+clat, no rtt -- a blk session is a direct fd read/write with no round-trip to a remote peer). dump() now prints each domain as its own section when it has samples; the shared cross-retry lat stat recorded by Connection::_finish() is unaffected.
|
@exordy /review |
exordy
left a comment
exordy
left a comment
There was a problem hiding this comment.
exo-review: 1 medium, nothing blocking
Intent: Add blk-session telemetry (slat/clat/top-N), splitting the telemetry API into ost/blk domains.
Verify before merge
- poll_queue.cpp (--without-liburing backend) submission path for the same synchronous-throw possibility before merging
- clang-format-21 pass over the 5 duplicated blk_session.cpp blocks (not run per instructions; CI covers it)
Note: None of the six listed sibling PRs touch librawstor's C++ internals or this telemetry API — no merge-order dependency here.
Coverage: 5 axes clean · 3 checks run · mode shipped
Clean axes: security — no external input, no target resource, no tenancy surface; diff scope — every hunk traces to the ost/blk split; no stray unnamespaced call sites; compat/upgrade — stderr-only diagnostic, no schema/API/migration surface; reconciliation — n/a, client-side telemetry only; repo hygiene — English, imperative subjects, ChangeLog matches the change
Checks run:
- grep for stray unnamespaced telemetry::record_*/op_started/op_finished call sites — none found
- read uring_queue.cpp pread/preadv/pwrite/pwritev/fsync to confirm synchronous ENOBUFS throw path
- read Awaitable ctor — not default-constructible, informed the suggested fix shape
21 turns · 189s
| rawstor::telemetry::TimePoint t_created = rawstor::telemetry::now(); | ||
| rawstor::telemetry::blk::op_started(); | ||
|
|
||
| rawio::Awaitable<size_t> awaitable = _queue.pread(fd(), buf, size, offset); | ||
| rawstor::telemetry::TimePoint t_submitted = rawstor::telemetry::now(); | ||
| rawstor::telemetry::blk::record_slat(t_submitted - t_created); |
There was a problem hiding this comment.
[medium] blk::op_started() runs before the queue submit call, so a synchronous submission throw leaks the in-flight counter.
Why
uring_queue.cpp's pread/preadv/pwrite/pwritev/fsync throw ENOBUFS synchronously (ring full) before returning the Awaitable. op_started() already ran; the try/catch only wraps co_await awaitable, not its construction, so op_finished() never runs.
| rawstor::telemetry::TimePoint t_created = rawstor::telemetry::now(); | |
| rawstor::telemetry::blk::op_started(); | |
| rawio::Awaitable<size_t> awaitable = _queue.pread(fd(), buf, size, offset); | |
| rawstor::telemetry::TimePoint t_submitted = rawstor::telemetry::now(); | |
| rawstor::telemetry::blk::record_slat(t_submitted - t_created); | |
| rawstor::telemetry::TimePoint t_created = rawstor::telemetry::now(); | |
| rawio::Awaitable<size_t> awaitable = _queue.pread(fd(), buf, size, offset); | |
| rawstor::telemetry::blk::op_started(); | |
| rawstor::telemetry::TimePoint t_submitted = rawstor::telemetry::now(); | |
| rawstor::telemetry::blk::record_slat(t_submitted - t_created); |
3 participants
No description provided.