-
Notifications
You must be signed in to change notification settings - Fork 94
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
Run PREPARE on all nodes (Issue #106) #115
Conversation
scylla/src/transport/session.rs
Outdated
let mut results = try_join_all(handles).await?; | ||
|
||
// Check if every received statement has the same ID | ||
for p in results.windows(2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this additional check to a separate helper function (called validate_prepare_ids or something alike).
Also, consider writing it in a more declarative fashion, e.g.:
results.windows(2).all(|p| p[0].get_id() == p[1].get_id()) // + add error handling at the end of the chain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
scylla/src/transport/session.rs
Outdated
)), | ||
_ => Err(anyhow!("Unexpected frame received")), | ||
let connections = self.get_connections()?; | ||
let all_connections = connections.iter().map(|(_, v)| v).flatten(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use flat_map
:
let all_connections = connections.iter().flat_map(|(_, v)| v);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@havaker There were two big PRs merged which caused this PR to have conflicts with the main branch - please rebase. |
828c206
to
ebdacee
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one small nit, otherwise LGTM
scylla/src/transport/session.rs
Outdated
match results.pop() { | ||
Some(result) => Ok(result), | ||
None => Err(TransportError::NoConnectionsAvailable), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert it to:
results.pop().ok_or_else(TransportError::NoConnectionsAvailable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (I chose to use ok_or
instead of ok_or_else
)
ebdacee
to
c104d16
Compare
This pull request implements #106 by sending PREPARE via all available connections.