Send prepared statements to all nodes #320
Conversation
There was a problem hiding this comment.
@bruno-barin-ifood This is an awesome work.
I had few friendly suggestions/questions related to your solution. Could you please have a look on my comments?
And thank you for submitting the PR 👍
| results.iter().find(|r| r.is_ok()).unwrap() | ||
| } | ||
|
|
||
| fn transform_error() -> error::Result<Frame> { |
There was a problem hiding this comment.
A small typo. Perhaps it had to be transport_error
| let result = results.iter().find(|r| r.is_err()) | ||
| .unwrap_or_else( ||get_any_valid_result(&results)); | ||
|
|
||
| match result { |
There was a problem hiding this comment.
Perhaps this statement can be simplified so that no clone is needed. Please consider following line to put instead the whole match.
result.map_err(|err| error::Error::General(e.description().to_string())| } | ||
|
|
||
| let result = results.iter().find(|r| r.is_err()) | ||
| .unwrap_or_else( ||get_any_valid_result(&results)); |
There was a problem hiding this comment.
Unfortunately, getting any valid result may not work as expected. Each node will prepare a query with a different ID. That's why if we have a cluster with >1 nodes load balancer may select a "wrong" node which has the query prepared with a different ID.
What I had been thinking about is to have a map of prepared queries to nodes so that during preparing new entity is added to this map. Whenever exec-like call is made, the executor picks up a node where the prepared query ID is residing. However for that a LoadBalancingStrategy should be able to return a node basing on providing criteria, e.g.
load_balancer
.find_node(|node_in_cluster| node.addr() == addr_assigned_to_prepared_id)
.send_frame(frame_bytes)
@AlexPikalov in this PR I've tried to make CDRS able to send the prepared statements to all pooled connections. I'm not well versed in rust, so you might probably find a better way to circumvent the problem of the prepared statement actually being sent to a single node.
Let me know your thoughts.