Skip to content

Commit cbc2895

Browse files
AlexStocksOmX
andauthored
feat(storage,runtime): add request-local storage stats collection (#393)
* feat(storage,runtime): connect real StorageStatsCollector at storage I/O points Replace the NoopStorageStatsCollector stub with a per-request RealStorageStatsCollector installed via the STORAGE_STATS_COLLECTOR task-local, so per-request storage metrics (keys/bytes read, written, deleted, cache hit, compaction level) are recorded at the actual I/O points instead of being inferred from command arguments. - Move StorageStats / StorageStatsCollector trait into the client leaf crate to avoid a storage <-> runtime dependency cycle; runtime::message re-exports them for backward compatibility. - Instrument the Storage facade methods in storage_impl.rs (the single convergence layer the cmd layer calls), requiring zero Cmd::execute signature changes. - Add StorageStats.bytes_deleted per issue #312. Fixes #312 * fix(storage): correct request-local stats accounting Record logical storage statistics only after backend outcomes are known, aggregate completed response statistics in the client and runtime metrics, and sample request-local RocksDB block-cache hits. Add fault-injection coverage for failed and partial multi-shard mutations, while keeping new serialized fields and optional collector methods backward compatible. Constraint: Limit this follow-up to PR #393 CI failures, CodeRabbit findings, request-local statistics correctness, and directly related regression tests. Confidence: High; failure, no-op, partial-success, aggregation, reset, serialization, and trait-compatibility paths have direct tests. Scope-risk: Storage statistics instrumentation and runtime metrics only; uninstrumented Storage facade methods and request-attributable compaction levels remain explicitly outside this incremental PR. Red: The previous PR Head failed license header, rustfmt, and all three Clippy jobs; statistics were charged before failed or no-op mutations and lacked storage regression coverage. Green: WSL cargo test -p client; cargo test -p runtime; cargo test -p storage --features test-fault-injection --test storage_stats_test -- --test-threads=1 --nocapture; cargo fmt --all -- --check; CI-equivalent workspace Clippy; git diff --cached --check. Tested: client 6/6, runtime 135/135, storage statistics 6/6, exact Rust license-header comparison, rustfmt, workspace all-features Clippy, and staged diff validation. Not-tested: GitHub's macOS and Windows Clippy runners and SkyWalking Eyes action will run after push. Co-authored-by: OmX <omx@oh-my-codex.dev> --------- Co-authored-by: OmX <omx@oh-my-codex.dev>
1 parent 1f67524 commit cbc2895

12 files changed

Lines changed: 888 additions & 122 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/client/Cargo.toml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,8 @@ rust-version.workspace = true
1111
async-trait = "0.1"
1212
resp = { path = "../resp" }
1313
parking_lot = { workspace = true }
14-
tokio = { version = "1.50", features = ["sync"] }
14+
serde = { workspace = true, features = ["derive"] }
15+
tokio = { version = "1.50", features = ["sync", "rt"] }
16+
17+
[dev-dependencies]
18+
serde_json = { workspace = true }

src/client/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ use async_trait::async_trait;
2121
use resp::{ProtocolNegotiator, RespCommand, RespData, RespResult};
2222
use tokio::sync::Mutex;
2323

24+
pub mod storage_stats;
25+
pub use storage_stats::*;
26+
2427
#[async_trait]
2528
pub trait StreamTrait: Send + Sync {
2629
async fn read(&mut self, buf: &mut [u8]) -> Result<usize, std::io::Error>;

src/client/src/storage_stats.rs

Lines changed: 327 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
// Copyright (c) 2024-present, arana-db Community. All rights reserved.
2+
//
3+
// Licensed to the Apache Software Foundation (ASF) under one or more
4+
// contributor license agreements. See the NOTICE file distributed with
5+
// this work for additional information regarding copyright ownership.
6+
// The ASF licenses this file to You under the Apache License, Version 2.0
7+
// (the "License"); you may not use this file except in compliance with
8+
// the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing, software
13+
// distributed under the License is distributed on an "AS IS" BASIS,
14+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
// See the License for the specific language governing permissions and
16+
// limitations under the License.
17+
18+
//! Request-local storage-layer instrumentation.
19+
//!
20+
//! These types were originally declared in `runtime::message` (see
21+
//! <https://github.com/arana-db/kiwi/issues/312>). They live in the `client`
22+
//! crate so that `storage`, `runtime` and `cmd` can share them without
23+
//! creating a dependency cycle (`client` is a leaf crate that everything else
24+
//! already depends on).
25+
//!
26+
//! The real collector ([`RealStorageStatsCollector`]) is installed once per
27+
//! storage request via the [`STORAGE_STATS_COLLECTOR`] task-local and read by
28+
//! the storage facade after backend outcomes are known. Key and byte counters
29+
//! describe successful logical storage operations and their payload sizes;
30+
//! RocksDB-specific observations are reported separately when available.
31+
32+
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
33+
use std::sync::{Arc, Mutex as StdMutex};
34+
35+
use serde::{Deserialize, Serialize};
36+
37+
/// Statistics about storage operations for monitoring.
38+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
39+
#[serde(default)]
40+
pub struct StorageStats {
41+
/// Number of keys read during the operation
42+
pub keys_read: u64,
43+
/// Number of keys written during the operation
44+
pub keys_written: u64,
45+
/// Number of keys deleted during the operation
46+
pub keys_deleted: u64,
47+
/// Logical payload read in bytes (key bytes + returned value bytes)
48+
pub bytes_read: u64,
49+
/// Logical payload written in bytes (key bytes + accepted value bytes)
50+
pub bytes_written: u64,
51+
/// Logical key bytes affected by successful delete operations
52+
pub bytes_deleted: u64,
53+
/// Whether the operation hit a cache (e.g. block/moka cache)
54+
pub cache_hit: bool,
55+
/// RocksDB compaction level accessed, when the engine can attribute one
56+
pub compaction_level: Option<u32>,
57+
}
58+
59+
impl StorageStats {
60+
/// Merge another completed request into this aggregate without wrapping.
61+
pub fn merge(&mut self, request: &Self) {
62+
self.keys_read = self.keys_read.saturating_add(request.keys_read);
63+
self.keys_written = self.keys_written.saturating_add(request.keys_written);
64+
self.keys_deleted = self.keys_deleted.saturating_add(request.keys_deleted);
65+
self.bytes_read = self.bytes_read.saturating_add(request.bytes_read);
66+
self.bytes_written = self.bytes_written.saturating_add(request.bytes_written);
67+
self.bytes_deleted = self.bytes_deleted.saturating_add(request.bytes_deleted);
68+
self.cache_hit |= request.cache_hit;
69+
if request.compaction_level.is_some() {
70+
self.compaction_level = request.compaction_level;
71+
}
72+
}
73+
}
74+
75+
/// Request-local collector for storage-layer instrumentation.
76+
///
77+
/// Implementations record sizes observed by the storage facade after the
78+
/// backend outcome is known, rather than inferring success in command parsing.
79+
pub trait StorageStatsCollector: Send + Sync {
80+
/// Record a successful logical storage read.
81+
fn record_read(&self, key_bytes: u64, value_bytes: u64);
82+
83+
/// Record a logical storage write after the backend accepts the mutation.
84+
fn record_write(&self, key_bytes: u64, value_bytes: u64);
85+
86+
/// Record a logical delete after the backend confirms a mutation.
87+
fn record_delete(&self, key_bytes: u64);
88+
89+
/// Record that this request was served from a storage cache.
90+
fn record_cache_hit(&self) {}
91+
92+
/// Record the RocksDB compaction level that served this request.
93+
fn record_compaction_level(&self, _level: u32) {}
94+
95+
/// Return the accumulated statistics for the request.
96+
fn finish(&self) -> StorageStats;
97+
}
98+
99+
/// Placeholder collector used until storage-layer instrumentation is wired in.
100+
#[derive(Debug, Default, Clone, Copy)]
101+
pub struct NoopStorageStatsCollector;
102+
103+
impl StorageStatsCollector for NoopStorageStatsCollector {
104+
fn record_read(&self, _key_bytes: u64, _value_bytes: u64) {}
105+
106+
fn record_write(&self, _key_bytes: u64, _value_bytes: u64) {}
107+
108+
fn record_delete(&self, _key_bytes: u64) {}
109+
110+
fn finish(&self) -> StorageStats {
111+
StorageStats::default()
112+
}
113+
}
114+
115+
/// A real per-request collector backed by lock-free atomic counters.
116+
///
117+
/// Safe to share across the (single) task that executes one storage request:
118+
/// every field is an atomic and `finish` snapshots the accumulated values.
119+
pub struct RealStorageStatsCollector {
120+
keys_read: AtomicU64,
121+
keys_written: AtomicU64,
122+
keys_deleted: AtomicU64,
123+
bytes_read: AtomicU64,
124+
bytes_written: AtomicU64,
125+
bytes_deleted: AtomicU64,
126+
cache_hit: AtomicBool,
127+
compaction_level: StdMutex<Option<u32>>,
128+
}
129+
130+
impl Default for RealStorageStatsCollector {
131+
fn default() -> Self {
132+
Self::new()
133+
}
134+
}
135+
136+
impl RealStorageStatsCollector {
137+
/// Create an empty collector ready to accumulate one request's stats.
138+
pub fn new() -> Self {
139+
Self {
140+
keys_read: AtomicU64::new(0),
141+
keys_written: AtomicU64::new(0),
142+
keys_deleted: AtomicU64::new(0),
143+
bytes_read: AtomicU64::new(0),
144+
bytes_written: AtomicU64::new(0),
145+
bytes_deleted: AtomicU64::new(0),
146+
cache_hit: AtomicBool::new(false),
147+
compaction_level: StdMutex::new(None),
148+
}
149+
}
150+
}
151+
152+
impl StorageStatsCollector for RealStorageStatsCollector {
153+
fn record_read(&self, key_bytes: u64, value_bytes: u64) {
154+
self.keys_read.fetch_add(1, Ordering::Relaxed);
155+
self.bytes_read
156+
.fetch_add(key_bytes.saturating_add(value_bytes), Ordering::Relaxed);
157+
}
158+
159+
fn record_write(&self, key_bytes: u64, value_bytes: u64) {
160+
self.keys_written.fetch_add(1, Ordering::Relaxed);
161+
self.bytes_written
162+
.fetch_add(key_bytes.saturating_add(value_bytes), Ordering::Relaxed);
163+
}
164+
165+
fn record_delete(&self, key_bytes: u64) {
166+
self.keys_deleted.fetch_add(1, Ordering::Relaxed);
167+
self.bytes_deleted.fetch_add(key_bytes, Ordering::Relaxed);
168+
}
169+
170+
fn record_cache_hit(&self) {
171+
self.cache_hit.store(true, Ordering::Relaxed);
172+
}
173+
174+
fn record_compaction_level(&self, level: u32) {
175+
*self
176+
.compaction_level
177+
.lock()
178+
.expect("compaction_level poisoned") = Some(level);
179+
}
180+
181+
fn finish(&self) -> StorageStats {
182+
StorageStats {
183+
keys_read: self.keys_read.load(Ordering::Relaxed),
184+
keys_written: self.keys_written.load(Ordering::Relaxed),
185+
keys_deleted: self.keys_deleted.load(Ordering::Relaxed),
186+
bytes_read: self.bytes_read.load(Ordering::Relaxed),
187+
bytes_written: self.bytes_written.load(Ordering::Relaxed),
188+
bytes_deleted: self.bytes_deleted.load(Ordering::Relaxed),
189+
cache_hit: self.cache_hit.load(Ordering::Relaxed),
190+
compaction_level: *self
191+
.compaction_level
192+
.lock()
193+
.expect("compaction_level poisoned"),
194+
}
195+
}
196+
}
197+
198+
tokio::task_local! {
199+
/// Per-request storage stats collector, scoped for the duration of a single
200+
/// storage command execution. The storage layer reads it via
201+
/// [`try_collector`].
202+
pub static STORAGE_STATS_COLLECTOR: Arc<dyn StorageStatsCollector + Send + Sync>;
203+
}
204+
205+
/// Returns the active request-local collector, if one was installed for the
206+
/// current async task.
207+
///
208+
/// Returns `None` when no collector is scoped (e.g. unit tests that call the
209+
/// storage engine directly, or background storage tasks), so callers can record
210+
/// unconditionally without special-casing those paths.
211+
pub fn try_collector() -> Option<Arc<dyn StorageStatsCollector + Send + Sync>> {
212+
STORAGE_STATS_COLLECTOR.try_with(|c| c.clone()).ok()
213+
}
214+
215+
#[cfg(test)]
216+
mod tests {
217+
#![allow(clippy::unwrap_used)]
218+
219+
use super::*;
220+
221+
struct LegacyCollector;
222+
223+
impl StorageStatsCollector for LegacyCollector {
224+
fn record_read(&self, _key_bytes: u64, _value_bytes: u64) {}
225+
226+
fn record_write(&self, _key_bytes: u64, _value_bytes: u64) {}
227+
228+
fn record_delete(&self, _key_bytes: u64) {}
229+
230+
fn finish(&self) -> StorageStats {
231+
StorageStats::default()
232+
}
233+
}
234+
235+
#[test]
236+
fn real_collector_accumulates_reads_writes_deletes() {
237+
let c = RealStorageStatsCollector::new();
238+
c.record_read(3, 5);
239+
c.record_write(3, 7);
240+
c.record_read(2, 4);
241+
c.record_delete(3);
242+
243+
let stats = c.finish();
244+
assert_eq!(stats.keys_read, 2);
245+
assert_eq!(stats.keys_written, 1);
246+
assert_eq!(stats.keys_deleted, 1);
247+
assert_eq!(stats.bytes_read, (3 + 5) + (2 + 4));
248+
assert_eq!(stats.bytes_written, 3 + 7);
249+
assert_eq!(stats.bytes_deleted, 3);
250+
assert!(!stats.cache_hit);
251+
}
252+
253+
#[test]
254+
fn noop_yields_empty_stats() {
255+
assert_eq!(NoopStorageStatsCollector.finish(), StorageStats::default());
256+
}
257+
258+
#[test]
259+
fn legacy_collectors_use_default_optional_recorders() {
260+
LegacyCollector.record_cache_hit();
261+
LegacyCollector.record_compaction_level(3);
262+
assert_eq!(LegacyCollector.finish(), StorageStats::default());
263+
}
264+
265+
#[test]
266+
fn legacy_serialized_stats_default_new_fields() {
267+
let stats: StorageStats = serde_json::from_str(
268+
r#"{
269+
"keys_read": 1,
270+
"keys_written": 2,
271+
"keys_deleted": 3,
272+
"bytes_read": 4,
273+
"bytes_written": 5,
274+
"cache_hit": true,
275+
"compaction_level": null
276+
}"#,
277+
)
278+
.unwrap();
279+
280+
assert_eq!(stats.bytes_deleted, 0);
281+
assert_eq!(stats.keys_read, 1);
282+
assert!(stats.cache_hit);
283+
}
284+
285+
#[test]
286+
fn trait_object_records_cache_and_compaction_details() {
287+
let collector: Arc<dyn StorageStatsCollector + Send + Sync> =
288+
Arc::new(RealStorageStatsCollector::new());
289+
290+
collector.record_cache_hit();
291+
collector.record_compaction_level(3);
292+
293+
let stats = collector.finish();
294+
assert!(stats.cache_hit);
295+
assert_eq!(stats.compaction_level, Some(3));
296+
}
297+
298+
#[test]
299+
fn scoped_collector_is_visible_to_try_collector() {
300+
// Mirrors how `storage_server` installs the collector and how the
301+
// storage facade reads it via `try_collector` — without needing a real
302+
// RocksDB instance.
303+
let runtime = tokio::runtime::Builder::new_current_thread()
304+
.enable_all()
305+
.build()
306+
.unwrap();
307+
runtime.block_on(async {
308+
let collector: Arc<dyn StorageStatsCollector + Send + Sync> =
309+
Arc::new(RealStorageStatsCollector::new());
310+
STORAGE_STATS_COLLECTOR
311+
.scope(Arc::clone(&collector), async {
312+
if let Some(c) = try_collector() {
313+
c.record_write(3, 5);
314+
c.record_read(2, 4);
315+
} else {
316+
panic!("try_collector() must return the scoped collector");
317+
}
318+
})
319+
.await;
320+
let stats = collector.finish();
321+
assert_eq!(stats.keys_written, 1);
322+
assert_eq!(stats.bytes_written, 8);
323+
assert_eq!(stats.keys_read, 1);
324+
assert_eq!(stats.bytes_read, 6);
325+
});
326+
}
327+
}

0 commit comments

Comments
 (0)