-
Notifications
You must be signed in to change notification settings - Fork 111
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
Enhance Proof Delivery Resilience with ParSliceErrCollect in ChainPorter #1100
Conversation
94362fd
to
55c9d3b
Compare
Pull Request Test Coverage Report for Build 10579893055Details
💛 - Coveralls |
PR Ensures proofs are delivered given partial proof-courier addresses availability |
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.
Very nice improvement!
Just nits and suggestions.
fn/concurrency.go
Outdated
instanceErrors := make(map[int]error, len(s)) | ||
|
||
for idx, v := range s { | ||
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.
nit: this is no longer required, as we're now using Go 1.22.
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.
I've gone with s[idx]
in the latest commits. Are you saying that v := v
isn't required because of Go 1.22? Why is that, can you say more please?
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.
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.
Looks good, a useful func! I think we have some spots in the universe code, specifically federations, where this could be used as well.
Would be good to add a test for this alongside the test for ParSlice
.
Add ParSliceErrCollect function to handle errors without halting parallel processing. ParSliceErrCollect is similar to ParSlice but allows processing to continue even when errors occur. Instead of halting on the first error, it collects all errors for later handling, providing more robust parallel execution.
This commit adds the `PeekMap` function. This function non-deterministically selects and returns a single key-value pair from the given map.
This commit leverages the new ParSliceErrCollect function to process transfer output proofs in parallel. This change ensures that processing continues even if a transfer output proof delivery instance fails.
55c9d3b
to
ac10515
Compare
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.
Nice, LGTM 🎉
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.
LGTM 👍🏽
This PR updates the proof delivery process in
ChainPorter
by utilizing the newly introducedParSliceErrCollect
function. This change ensures that proof delivery continues even if an individual transfer output proof encounters an error, improving the reliability of the delivery process.This change ensures that as many transfer output proofs as possible are delivered, even in the presence of errors.
Changes
ParSliceErrCollect
, which collects errors during parallel processing without halting the entire operation.ParSliceErrCollect
for parallel proof delivery, allowing the process to continue despite individual delivery failures.Related to: #1082