Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

https://github.com/redis-rs/redis-rs/pull/888 #6

Merged
merged 2 commits into from
Jul 17, 2023
Merged

Conversation

nihohit
Copy link

@nihohit nihohit commented Jul 16, 2023

this PR is based over https://github.com/nihohit/redis-rs/pull/5.

This change implements the response_policy tip in the async cluster.
https://redis.io/docs/reference/command-tips/#response_policy

This means that the results from fan-out commands will be aggregated
differently, based on the sent command.

@nihohit nihohit requested a review from barshaul July 16, 2023 10:50
redis/src/cluster_async/mod.rs Outdated Show resolved Hide resolved
Some(ResponsePolicy::Special) | None => {
// This is our assumption - if there's no coherent way to aggregate the responses, we just map each response to the sender, and pass it to the user.
// TODO - once RESP3 is merged, return a map value here.
future::try_join_all(connections.into_iter().map(|(addr, conn)| async move {

Choose a reason for hiding this comment

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

try_join_all => us join_all, when mapping we can allow some requests to succeed and others to fail

Copy link
Author

Choose a reason for hiding this comment

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

This doesn't work without ValueError. Added TODO

@@ -533,64 +534,116 @@ where
Ok(())
}

async fn execute_on_all_nodes(
async fn execute_on_multiple_nodes(
func: fn(C, Arc<Cmd>) -> RedisFuture<'static, Response>,
cmd: &Arc<Cmd>,
only_primaries: bool,

Choose a reason for hiding this comment

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

for a different PR - I think we should pass the routing info to the function and that the function will retrieve its relevant addresses, so it could be used to other groups of nodes besides to PRIMARES/ALL_NODES (e.g. - specific shard, all replicas, group of nodes the user passes..)

Copy link
Author

Choose a reason for hiding this comment

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

next PR :)

Copy link
Author

Choose a reason for hiding this comment

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

nihohit@0a7558d
as requested :)

This change implements the response_policy tip in the async cluster.
https://redis.io/docs/reference/command-tips/#response_policy

This means that the results from fan-out commands will be aggregated
differently, based on the sent command.
@nihohit nihohit merged commit 8caeef4 into main Jul 17, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants