Skip to content

Commit

Permalink
When no keys are provided, route to a random node to achieve the same…
Browse files Browse the repository at this point in the history
… behavior as the standalone client

(and it will also report a meaningful error to the log)

    Fixes: #2714

Signed-off-by: Eran Ifrah <[email protected]>
  • Loading branch information
eifrah-aws committed Nov 19, 2024
1 parent de7892c commit 53069b0
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
14 changes: 13 additions & 1 deletion glide-core/redis-rs/redis/src/cluster_async/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1510,12 +1510,24 @@ where
convert_result(receiver.await)
};

// Sanity
if receivers.is_empty() {
return Err(RedisError::from((
ErrorKind::ClientError,
"Client internal error",
"Failed to aggregate results for multi-slot command. Maybe a malformed command?"
.to_string(),
)));
}

// TODO - once Value::Error will be merged, these will need to be updated to handle this new value.
match response_policy {
Some(ResponsePolicy::AllSucceeded) => {
future::try_join_all(receivers.into_iter().map(get_receiver))
.await
.map(|mut results| results.pop().unwrap()) // unwrap is safe, since at least one function succeeded
.map(|mut results| {
results.pop().unwrap() // unwrap is safe, since at least one function succeeded
})
}
Some(ResponsePolicy::OneSucceeded) => future::select_ok(
receivers
Expand Down
6 changes: 5 additions & 1 deletion glide-core/redis-rs/redis/src/cluster_routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub enum LogicalAggregateOp {
// Or, omitted due to dead code warnings. ATM this value isn't constructed anywhere
}

/// Numerical aggreagting operators.
/// Numerical aggregating operators.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum AggregateOp {
/// Choose minimal value
Expand Down Expand Up @@ -512,6 +512,10 @@ where
}

let mut routes: Vec<(Route, Vec<usize>)> = routes.into_iter().collect();
if routes.is_empty() {
return None;
}

Some(if routes.len() == 1 {
RoutingInfo::SingleNode(SingleNodeRoutingInfo::SpecificNode(routes.pop().unwrap().0))
} else {
Expand Down
15 changes: 15 additions & 0 deletions glide-core/tests/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,21 @@ pub(crate) mod shared_client_tests {
});
}

#[test]
#[serial_test::serial]
fn test_multi_key_no_args_in_cluster() {
block_on_all(async {
let cluster = cluster::setup_default_cluster().await;
println!("Creating 1st cluster client...");
let mut c1 = cluster::setup_default_client(&cluster).await;
let result = c1.send_command(&redis::cmd("MSET"), None).await;
assert!(result.is_err());
let e = result.unwrap_err();
assert!(e.kind().clone().eq(&redis::ErrorKind::ResponseError));
assert!(e.to_string().contains("wrong number of arguments"));
});
}

#[rstest]
#[serial_test::serial]
#[timeout(SHORT_CLUSTER_TEST_TIMEOUT)]
Expand Down

0 comments on commit 53069b0

Please sign in to comment.