Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
discovered at failover. Full support (id-pinned record forms + replica
apply arms + snapshot coverage) is tracked as follow-up work, alongside
the pre-existing Lua-EVAL and expiry/eviction propagation gaps.
### Fixed — listing parity: `INFO # Keyspace` all dbs × all shards, `GRAPH.LIST` all shards

- `INFO`'s `# Keyspace` section always printed a single `db0:` line holding
the SELECTED db's LOCAL-shard key count — `SELECT 2; SET k v; INFO`
reported the db-2 count as db0, every other db was invisible, and at
`--shards N` the other shards' keys were uncounted. It now lists every
NON-EMPTY logical db (Redis semantics) with `(keys, expires)` summed
across all shards via a new `KeyspaceStats` scatter (O(#dbs) counter
reads per shard, no key iteration). All three dispatch paths
(monoio / tokio sharded / single).
- `GRAPH.LIST` at `--shards N` listed only the connection shard's graphs
(~1/N of them, since a graph lives on the shard that owns its name). It
now scatters to every shard and returns the sorted, deduplicated union.

### Fixed — CLIENT TRACKING dead on the monoio runtime (H-3 reorder regression)

Expand Down
55 changes: 55 additions & 0 deletions src/command/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ fn format_memory_human(bytes: u64) -> String {
/// INFO command handler.
///
/// Returns a BulkString with server info sections matching Redis INFO format.
///
/// The `# Keyspace` section reports the CALLING shard's `db` as `db0` only —
/// the single-`Database` fallback for paths with no scatter access (generic
/// dispatch). The connection handlers pass cross-shard, all-db stats through
/// [`info_with_keyspace`] instead.
pub fn info(db: &Database, _args: &[Frame]) -> Frame {
use std::fmt::Write as _;
let mut sections = String::with_capacity(2048);
Expand Down Expand Up @@ -329,6 +334,38 @@ pub fn info(db: &Database, _args: &[Frame]) -> Frame {
Frame::BulkString(Bytes::from(sections))
}

/// INFO with an externally-gathered `# Keyspace` section: one `(keys,
/// expires)` entry per logical db, already summed across shards
/// (`coordinate_keyspace_info`). Lists every NON-EMPTY db like Redis —
/// previously the section always read `db0:` with the SELECTED db's local
/// count, so `SELECT 2; SET k v; INFO` reported the db-2 count as db0 and
/// every other db was invisible.
pub fn info_with_keyspace(db: &Database, args: &[Frame], keyspace: &[(u64, u64)]) -> Frame {
use std::fmt::Write as _;
let base = match info(db, args) {
Frame::BulkString(b) => b,
other => return other,
};
let text = String::from_utf8_lossy(&base);
// Rebuild everything up to the fallback "# Keyspace" section, then emit
// the accurate per-db lines.
let Some(cut) = text.find("# Keyspace\r\n") else {
return Frame::BulkString(base);
};
let mut sections = String::with_capacity(text.len() + keyspace.len() * 32);
sections.push_str(&text[..cut]);
sections.push_str("# Keyspace\r\n");
for (db_idx, (keys, expires)) in keyspace.iter().enumerate() {
if *keys > 0 {
let _ = write!(
sections,
"db{db_idx}:keys={keys},expires={expires},avg_ttl=0\r\n"
);
}
}
Frame::BulkString(Bytes::from(sections))
}

/// INFO command handler (read-only variant for RwLock read path).
///
/// Identical to info() -- Database methods used (len, expires_count) are already &self.
Expand Down Expand Up @@ -868,6 +905,24 @@ pub fn hello(
mod tests {
use super::*;

#[test]
fn info_with_keyspace_lists_every_nonempty_db() {
let db = Database::new();
let stats = vec![(0u64, 0u64), (0, 0), (7, 2), (0, 0), (1, 0)];
let Frame::BulkString(b) = info_with_keyspace(&db, &[], &stats) else {
panic!("expected bulk string");
};
let text = String::from_utf8_lossy(&b);
assert!(text.contains("# Keyspace\r\n"));
assert!(text.contains("db2:keys=7,expires=2,avg_ttl=0\r\n"));
assert!(text.contains("db4:keys=1,expires=0,avg_ttl=0\r\n"));
assert!(!text.contains("db0:"), "empty dbs must not be listed");
assert!(!text.contains("db1:"));
assert!(!text.contains("db3:"));
// The fallback single-db section must have been replaced, not doubled.
assert_eq!(text.matches("# Keyspace").count(), 1);
}

#[test]
fn test_ping_no_args() {
let result = ping(&[]);
Expand Down
26 changes: 26 additions & 0 deletions src/command/graph/graph_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,32 @@ pub fn graph_list(store: &GraphStore) -> Frame {
Frame::Array(frames.into())
}

/// Merge the local shard's GRAPH.LIST reply with the other shards' replies
/// (multi-shard scatter — a graph lives on the shard that owns its name, so
/// a local-only answer listed roughly 1/N of the graphs). Union of names,
/// sorted for a stable client-visible order. Error frames from remote shards
/// are skipped (prefer a partial listing over failing the whole command).
pub fn merge_graph_list_responses(local: Frame, remotes: &[Frame]) -> Frame {
let mut names: Vec<bytes::Bytes> = Vec::new();
let mut collect = |frame: &Frame| {
if let Frame::Array(items) = frame {
for item in items.iter() {
if let Frame::BulkString(name) = item {
names.push(name.clone());
}
}
}
};
collect(&local);
for r in remotes {
collect(r);
}
names.sort_unstable();
names.dedup();
let frames: Vec<Frame> = names.into_iter().map(Frame::BulkString).collect();
Frame::Array(frames.into())
}

// ---------------------------------------------------------------------------
// GRAPH.QUERY, GRAPH.RO_QUERY, GRAPH.EXPLAIN
// ---------------------------------------------------------------------------
Expand Down
30 changes: 30 additions & 0 deletions src/command/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod graph_write;
pub use graph_read::{
graph_explain, graph_hybrid, graph_info, graph_list, graph_neighbors, graph_profile,
graph_query, graph_query_or_write, graph_query_write, graph_ro_query, graph_vsearch,
merge_graph_list_responses,
};
pub use graph_write::{graph_addedge, graph_addnode, graph_create, graph_delete};

Expand Down Expand Up @@ -225,6 +226,35 @@ mod tests {
Frame::Array(FrameVec::from_vec(frames))
}

#[test]
fn merge_graph_list_unions_sorts_dedups_and_skips_errors() {
let mk = |names: &[&[u8]]| {
let frames: Vec<Frame> = names
.iter()
.map(|n| Frame::BulkString(Bytes::from(n.to_vec())))
.collect();
Frame::Array(FrameVec::from_vec(frames))
};
let local = mk(&[b"g2", b"g1"]);
let remotes = vec![
mk(&[b"g3", b"g1"]), // g1 duplicated across shards
Frame::Error(Bytes::from_static(b"ERR boom")), // skipped
mk(&[]),
];
let merged = merge_graph_list_responses(local, &remotes);
let Frame::Array(items) = merged else {
panic!("expected array");
};
let names: Vec<&[u8]> = items
.iter()
.map(|f| match f {
Frame::BulkString(b) => b.as_ref(),
_ => panic!("expected bulk string"),
})
.collect();
assert_eq!(names, vec![&b"g1"[..], &b"g2"[..], &b"g3"[..]]);
}

/// Decode `[headers, rows, stats]` from a GRAPH.QUERY response into rows of cells.
fn result_rows(resp: &Frame) -> Vec<Vec<Frame>> {
let Frame::Array(outer) = resp else {
Expand Down
14 changes: 12 additions & 2 deletions src/server/conn/handler_monoio/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ pub(super) fn try_handle_psync(

/// Handle INFO command. Returns `true` if consumed.
#[inline]
pub(super) fn try_handle_info(
pub(super) async fn try_handle_info(
cmd: &[u8],
cmd_args: &[Frame],
conn: &ConnectionState,
Expand All @@ -638,8 +638,18 @@ pub(super) fn try_handle_info(
if !cmd.eq_ignore_ascii_case(b"INFO") {
return false;
}
// # Keyspace parity: per-db (keys, expires) summed across ALL shards —
// previously the section reported the selected db's LOCAL count as db0
// (other dbs invisible, other shards uncounted).
let keyspace = crate::shard::coordinator::coordinate_keyspace_info(
ctx.shard_id,
ctx.num_shards,
&ctx.dispatch_tx,
&ctx.spsc_notifiers,
)
.await;
let response_text = crate::shard::slice::with_shard_db(conn.selected_db, |db| {
let resp_frame = conn_cmd::info_readonly(db, cmd_args);
let resp_frame = conn_cmd::info_with_keyspace(db, cmd_args, &keyspace);
Comment on lines +641 to +652

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Only scatter when the requested INFO section includes Keyspace.

Line 644 runs the cross-shard gather for INFO server, INFO clients, etc.; info_with_keyspace then returns the unchanged local response when # Keyspace is absent. Gate this call using the same section parsing as conn_cmd::info, including the default/all forms, so unrelated diagnostics do not incur shard timeouts.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/conn/handler_monoio/dispatch.rs` around lines 641 - 652, Gate the
cross-shard call to coordinate_keyspace_info in the INFO handling path so it
runs only when the requested sections include Keyspace. Reuse the section
parsing logic from conn_cmd::info, covering explicit Keyspace, default, and
all-section requests; otherwise skip coordination and pass an empty or absent
keyspace result to info_with_keyspace while preserving existing responses.

match resp_frame {
Frame::BulkString(b) => String::from_utf8_lossy(&b).to_string(),
_ => String::new(),
Expand Down
3 changes: 2 additions & 1 deletion src/server/conn/handler_monoio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,8 @@ pub(crate) async fn handle_connection_sharded_monoio<
continue;
}
}
if cmd_len == 4 && dispatch::try_handle_info(cmd, cmd_args, &conn, ctx, &mut responses)
if cmd_len == 4
&& dispatch::try_handle_info(cmd, cmd_args, &conn, ctx, &mut responses).await
{
continue;
}
Expand Down
39 changes: 37 additions & 2 deletions src/server/conn/handler_monoio/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,43 @@ pub(super) async fn try_handle_graph_command(
// thread-local once ShardSlice is initialized, so non-owner commands MUST
// hop via ShardMessage::GraphCommand — the shard-side handler dispatches
// on its own store and drains graph WAL records locally.
// GRAPH.LIST has no name argument and stays connection-local (it reports
// this shard's graphs only — recorded as a v3 observe delta).
// GRAPH.LIST has no name argument: scatter to EVERY shard and union the
// names (a local-only answer listed roughly 1/N of the graphs).
if ctx.num_shards > 1 && cmd.eq_ignore_ascii_case(b"GRAPH.LIST") {
let mut receivers = Vec::with_capacity(ctx.num_shards - 1);
for target in 0..ctx.num_shards {
if target == ctx.shard_id {
continue;
}
let (reply_tx, reply_rx) = crate::runtime::channel::oneshot();
let msg = crate::shard::dispatch::ShardMessage::GraphCommand {
command: std::sync::Arc::new(frame.clone()),
reply_tx,
};
let _ = crate::shard::coordinator::spsc_send(
&ctx.dispatch_tx,
ctx.shard_id,
target,
msg,
&ctx.spsc_notifiers,
)
.await;
receivers.push(reply_rx);
}
let mut remotes = Vec::with_capacity(receivers.len());
for rx in receivers {
if let Ok(f) = crate::shard::coordinator::recv_reply_bounded(rx).await {
remotes.push(f);
}
}
let local = crate::shard::slice::with_shard(|s| {
crate::command::graph::dispatch_graph_read(&s.graph_store, cmd, cmd_args, None)
});
responses.push(crate::command::graph::merge_graph_list_responses(
local, &remotes,
));
return true;
}
Comment on lines +857 to +891

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
grep -n '^futures' Cargo.toml
rg -n 'join_all|FuturesUnordered|select_all' src --type rust
ast-grep run --pattern 'fn race2($$$) { $$$ }' --lang rust src/runtime

Repository: pilotspace/moon

Length of output: 1359


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant files first
ast-grep outline src/server/conn/handler_monoio/write.rs --view expanded
ast-grep outline src/shard/coordinator.rs --view expanded
ast-grep outline src/runtime/channel.rs --view expanded
ast-grep outline src/server/conn/blocking.rs --view expanded

# Inspect the graph-list branch and the bounded receive helper
sed -n '840,910p' src/server/conn/handler_monoio/write.rs
rg -n 'recv_reply_bounded|XSHARD_REPLY_TIMEOUT|FuturesUnordered|join_all|select_all' src/shard src/runtime src/server/conn --type rust
sed -n '1,220p' src/shard/coordinator.rs
sed -n '230,390p' src/runtime/channel.rs
sed -n '200,290p' src/server/conn/blocking.rs

Repository: pilotspace/moon

Length of output: 30386


Await the GRAPH.LIST replies concurrently.
The current serial for rx in receivers loop can turn K stalled shards into K × XSHARD_REPLY_TIMEOUT total latency. Use FuturesUnordered or join_all so the per-shard timeouts run in parallel and partial results return promptly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/conn/handler_monoio/write.rs` around lines 857 - 891, Update the
GRAPH.LIST fan-out handling in the sharded branch to await all remote replies
concurrently instead of serially iterating over receivers. Use FuturesUnordered,
join_all, or an equivalent concurrent pattern around recv_reply_bounded so each
shard’s timeout runs in parallel, while preserving successful partial replies in
remotes before merging with the local response.

if ctx.num_shards > 1 && !cmd.eq_ignore_ascii_case(b"GRAPH.LIST") {
if let Some(name) = cmd_args.first().and_then(extract_bytes) {
let owner = crate::shard::dispatch::graph_to_shard(&name, ctx.num_shards);
Expand Down
14 changes: 12 additions & 2 deletions src/server/conn/handler_sharded/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ pub(super) fn try_handle_replicaof(
}

/// Handle INFO command. Returns `true` if consumed.
pub(super) fn try_handle_info(
pub(super) async fn try_handle_info(
cmd: &[u8],
cmd_args: &[Frame],
conn: &ConnectionState,
Expand All @@ -318,9 +318,19 @@ pub(super) fn try_handle_info(
if !cmd.eq_ignore_ascii_case(b"INFO") {
return false;
}
// # Keyspace parity: per-db (keys, expires) summed across ALL shards —
// previously the section reported the selected db's LOCAL count as db0
// (other dbs invisible, other shards uncounted).
let keyspace = crate::shard::coordinator::coordinate_keyspace_info(
ctx.shard_id,
ctx.num_shards,
&ctx.dispatch_tx,
&ctx.spsc_notifiers,
)
.await;
// ShardSlice path: access the local shard's database via thread-local.
let mut response_text = crate::shard::slice::with_shard_db(conn.selected_db, |db| {
let resp_frame = conn_cmd::info_readonly(db, cmd_args);
let resp_frame = conn_cmd::info_with_keyspace(db, cmd_args, &keyspace);
match resp_frame {
Frame::BulkString(b) => String::from_utf8_lossy(&b).to_string(),
_ => String::new(),
Expand Down
2 changes: 1 addition & 1 deletion src/server/conn/handler_sharded/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ pub(crate) async fn handle_connection_sharded_inner<
}

// --- INFO ---
if dispatch::try_handle_info(cmd, cmd_args, &conn, ctx, &mut responses) {
if dispatch::try_handle_info(cmd, cmd_args, &conn, ctx, &mut responses).await {
continue;
}

Expand Down
39 changes: 37 additions & 2 deletions src/server/conn/handler_sharded/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,8 +759,43 @@ pub(super) async fn try_handle_graph_command(
// thread-local once ShardSlice is initialized, so non-owner commands MUST
// hop via ShardMessage::GraphCommand — the shard-side handler dispatches
// on its own store and drains graph WAL records locally.
// GRAPH.LIST has no name argument and stays connection-local (it reports
// this shard's graphs only — recorded as a v3 observe delta).
// GRAPH.LIST has no name argument: scatter to EVERY shard and union the
// names (a local-only answer listed roughly 1/N of the graphs).
if ctx.num_shards > 1 && cmd.eq_ignore_ascii_case(b"GRAPH.LIST") {
let mut receivers = Vec::with_capacity(ctx.num_shards - 1);
for target in 0..ctx.num_shards {
if target == ctx.shard_id {
continue;
}
let (reply_tx, reply_rx) = crate::runtime::channel::oneshot();
let msg = crate::shard::dispatch::ShardMessage::GraphCommand {
command: std::sync::Arc::new(frame.clone()),
reply_tx,
};
let _ = crate::shard::coordinator::spsc_send(
&ctx.dispatch_tx,
ctx.shard_id,
target,
msg,
&ctx.spsc_notifiers,
)
.await;
receivers.push(reply_rx);
}
let mut remotes = Vec::with_capacity(receivers.len());
for rx in receivers {
if let Ok(f) = crate::shard::coordinator::recv_reply_bounded(rx).await {
remotes.push(f);
}
}
let local = crate::shard::slice::with_shard(|s| {
crate::command::graph::dispatch_graph_read(&s.graph_store, cmd, cmd_args, None)
});
responses.push(crate::command::graph::merge_graph_list_responses(
local, &remotes,
));
return true;
}
Comment on lines +764 to +798

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant files and symbols first.
ast-grep outline src/server/conn/handler_sharded/write.rs --view expanded | sed -n '1,220p'
printf '\n---\n'
ast-grep outline src/server/conn/handler_monoio/write.rs --view expanded | sed -n '1,240p'
printf '\n---\n'
rg -n "recv_reply_bounded|XSHARD_REPLY_TIMEOUT|GraphCommand|GRAPH.LIST|dispatch_graph_read|merge_graph_list_responses" src/server src/shard src/command -S

Repository: pilotspace/moon

Length of output: 9011


🏁 Script executed:

ast-grep outline src/server/conn/handler_sharded/write.rs --view expanded | sed -n '1,220p'
printf '\n---\n'
ast-grep outline src/server/conn/handler_monoio/write.rs --view expanded | sed -n '1,240p'
printf '\n---\n'
rg -n "recv_reply_bounded|XSHARD_REPLY_TIMEOUT|GraphCommand|GRAPH.LIST|dispatch_graph_read|merge_graph_list_responses" src/server src/shard src/command -S

Repository: pilotspace/moon

Length of output: 9011


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '746,802p' src/server/conn/handler_sharded/write.rs
printf '\n---\n'
sed -n '839,893p' src/server/conn/handler_monoio/write.rs
printf '\n---\n'
sed -n '191,230p' src/shard/coordinator.rs

Repository: pilotspace/moon

Length of output: 6677


Collect shard replies concurrently. The for loop waits on recv_reply_bounded(rx).await one shard at a time, so a stalled early shard can delay already-ready replies and stretch GRAPH.LIST latency by roughly N × XSHARD_REPLY_TIMEOUT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/conn/handler_sharded/write.rs` around lines 764 - 798, GRAPH.LIST
waits for remote shard replies sequentially, allowing one stalled shard to delay
all others. Update the reply-collection loop in the sharded GRAPH.LIST handling
block to await all receivers concurrently, such as by joining or spawning
bounded receive futures, then collect each successful response into remotes
before merging with the local result via merge_graph_list_responses.

if ctx.num_shards > 1 && !cmd.eq_ignore_ascii_case(b"GRAPH.LIST") {
if let Some(name) = cmd_args.first().and_then(extract_bytes) {
let owner = crate::shard::dispatch::graph_to_shard(&name, ctx.num_shards);
Expand Down
13 changes: 12 additions & 1 deletion src/server/conn/handler_single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,8 +874,19 @@ pub async fn handle_connection(
// --- INFO (append replication section) ---
if cmd.eq_ignore_ascii_case(b"INFO") {
if let Some(ref rs) = repl_state {
// # Keyspace parity: every non-empty db, not
// just the selected one mislabeled as db0.
// Single shard — the local dbs are the truth.
let keyspace: Vec<(u64, u64)> = db
.iter()
.map(|d| {
let g = d.read();
(g.len() as u64, g.expires_count() as u64)
})
.collect();
let guard = db[conn.selected_db].read();
let resp_frame = conn_cmd::info_readonly(&guard, cmd_args);
let resp_frame =
conn_cmd::info_with_keyspace(&guard, cmd_args, &keyspace);
drop(guard);
let mut response_text = match resp_frame {
Frame::BulkString(b) => String::from_utf8_lossy(&b).to_string(),
Expand Down
Loading
Loading