Dispute Coordinator: Batch queries #3459
Conversation
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
force-pushed
the
batch_queries
branch
from
cd0a9fe to
681aece
Compare
Lldenaurois
added
A0-please_review
Lldenaurois
force-pushed
the
batch_queries
branch
2 times, most recently
from
dc932bc to
39ab51b
Compare
Lldenaurois
force-pushed
the
batch_queries
branch
from
39ab51b to
0aeee12
Compare
There was a problem hiding this comment.
Migrated comments from the other PR over.
#3447 was closed when another PR was merged. I just noticed today.
| )? { | ||
| query_output.push(v.into()); | ||
| } else { | ||
| return Err( |
There was a problem hiding this comment.
@rphmeier Here is the modification that ensures we only respond if every (session_index, candidate_hash) exists in the db or we return an error.
There was a problem hiding this comment.
This is returning an io Error from the entire subsystem. We should probably just log something and continue. There's no need to break the message processing loop.
There was a problem hiding this comment.
It's important to make a distinction between errors the subsystem encounters while processing requests and requests the subsystem can't fulfill, but is fine with being able to skip those. As written, any request referencing missing data could interrupt the subsystem and potentially take it down.
| Err(oneshot::Canceled) => { | ||
| tracing::debug!( | ||
| target: LOG_TARGET, | ||
| "Unable to query candidate votes - subsystem disconnected?", |
There was a problem hiding this comment.
@rphmeier Added the added debug! log
|
Is this still missing guide changes? |
Lldenaurois
force-pushed
the
batch_queries
branch
from
bab0be5 to
c5ebfb4
Compare
|
Added guide changes and moved away from the all-or-nothing model. Instead, the session_index and candidate_hash are being sent in the response. This removes the need for a clone and seems to handle the overall behavior in a cleaner fashion. |
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