Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Dispute Coordinator: Move to batch Request - Response model #3447

Closed

Conversation

Lldenaurois
Copy link
Contributor

@Lldenaurois Lldenaurois commented Jul 8, 2021

This PR modifies the communication protocol between the provisioner and the dispute-coordinator. In the past, the dispute-coordinator kept track of a FuturesOrdered of oneshot-receivers and constructed a view of the recent disputes in this fashion.

Instead, it is more efficient to allow batch queries and only iterator over the recent disputes once. We leverage a single oneshot channel to reply to the batch Vec<(SessionIndex, CandidateHash)> request with the Vec<(SessionIndex, Candidatehash, CandidateVotes)> batch response.

Closes #3439 . Depends on #3348 merge.

This commit moves to batch queries when responding to QueryCandidateVotes
messages. This simplifies the code in the provisioner and dispute-coordinator
by no longer requiring to make use of a FuturesOrdered when awaiting multiple
quries. Instead, the provisioner need only request the batch itself.
@Lldenaurois Lldenaurois added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 8, 2021
@Lldenaurois Lldenaurois changed the base branch from master to rh-wire-up-disputes July 8, 2021 20:24

vote_sets
rx.await.unwrap_or_default().into_iter().zip(recent_disputes.into_iter())
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like a debug! log as done above

let _ = rx.send(candidate_votes.map(Into::into));
let mut query_output = Vec::new();
for (session_index, candidate_hash) in query.into_iter() {
if let Some(v) = db::v1::load_candidate_votes(
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to fail the request if any of the votes aren't present. As such, the zip done in the provisioner is inaccurate.

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

Nits and needs guide update as well.

@ghost ghost closed this Jul 9, 2021
@ghost ghost deleted the branch paritytech:rh-wire-up-disputes July 9, 2021 21:15
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DisputeCoordinator QueryCandidateVotes should be able to answer multiple queries at once
2 participants