Make validator stable when beacon node goes offline #8278
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shayzluf
commented
Jan 18, 2021
| } | ||
| connectionErrorChannel := make(chan error) | ||
| go v.ReceiveBlocks(ctx, connectionErrorChannel) |
There was a problem hiding this comment.
there was no error handling for ReceiveBlocks while it was running in a separate goroutine
shayzluf
changed the title
rauljordan
reviewed
Jan 19, 2021
validator/client/runner.go
Outdated
| continue | ||
| } | ||
| if err != nil { | ||
| log.Fatalf("Could not determine if beacon chain started: %v", err) |
There was a problem hiding this comment.
Do we ned to fatal and can we instead do the backoff continue?
validator/client/runner.go
Outdated
| continue | ||
| } | ||
| if err != nil { | ||
| log.Fatalf("Could not determine if beacon node synced: %v", err) |
nisdas
reviewed
Jan 19, 2021
shayzluf
commented
Jan 21, 2021
| ctx, span := trace.StartSpan(ctx, "validator.processSlot") | ||
|
|
||
| select { | ||
| case <-ctx.Done(): | ||
| log.Info("Context canceled, stopping validator") | ||
| span.End() | ||
| cancel() |
There was a problem hiding this comment.
missing call to cancel? @rauljordan goland detected a possible context leak
…fail_safe_validator # Conflicts: # validator/client/mock_validator.go # validator/client/runner.go # validator/client/runner_test.go
…/prysm into beacon_fail_safe_validator
shayzluf
changed the title
rauljordan
previously approved these changes
Jan 26, 2021
There was a problem hiding this comment.
This all makes sense to me, nice tests @shayzluf - I think this will be a very nice improvement
…fail_safe_validator
terencechain
approved these changes
Jan 28, 2021
rauljordan
added
OK to merge
Ready For Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Feature
What does this PR do? Why is it needed?
Currently validator client crashes before entering the mail loop. it can crash on few cases:
WaitForChainStartconnection issueWaitForSyncconnection issueCanonicalHeadSlotconnection issuewhile investigating the right way to implement the fail safe mechanism for beacon client availability i found a case where validator could stop getting blocks
I have marked this problematic line with a comment
I tested it in runtime while crushing the beacon node on several cases (looks stable)
This pr polls the endpoint of beacon node while it is nor reachable and waits till its up to get on with its functionality
Which issues(s) does this PR fix?
Fixes #8188