Rebroadcast request for latest epoch until we have reached enough responses#426
Rebroadcast request for latest epoch until we have reached enough responses#426samliok wants to merge 5 commits into
Conversation
samliok
changed the title
Signed-off-by: Sam Liokumovich <65994425+samliok@users.noreply.github.com>
| attempt := taskIds[0] + 1 | ||
|
|
||
| // too many attempts, don't rebroadcast | ||
| if attempt > maxRebroadcastAttempts { |
There was a problem hiding this comment.
I don't understand why limit the number of retries... what happens if there is no connectivity and we have exhausted the number of retries?
There was a problem hiding this comment.
However it's possible that we restarted our non-validator and its already up-to-date. In that case, we limit the number of retries to maxRebroadcastAttempts as to not indefinitely re-broadcast.
There was a problem hiding this comment.
I am not sure what's the point in rebroadcasting it after the validator boots but only for 5 iterations.
What is the problem that we are trying to solve?
| } | ||
|
|
||
| n.Logger.Debug("Rebroadcasting latest epoch", zap.Uint64("Next to commit", n.nextSeqToCommit())) | ||
| n.broadcastLatestEpoch() |
There was a problem hiding this comment.
shouldn't this method be called under a lock?
| // Seed the first attempt, then tick a few intervals past the cap. Each tick | ||
| // fires one rebroadcast and schedules the next attempt; ticks past the cap | ||
| // should be no-ops. | ||
| nv.bootstrapRebroadcastHandler.AddTask(0) |
There was a problem hiding this comment.
we should not be accessing internal fields in tests in such a manner, but instead relying on exported APIs to drive the test.
|
|
||
| // Validating a new epoch should drop every pending rebroadcast task. | ||
| nv.bootstrapRebroadcastHandler.AddTask(0) | ||
| nv.maybeValidateNextEpoch(tc.appendSealing(testNodes)) |
There was a problem hiding this comment.
same, we should not be doing this.
2 participants
In case the first broadcast message was not responded to by a threshold of validators, we want to continue rebroadcasting requests for the latest epoch until we validate an epoch.
However it's possible that we restarted our non-validator and its already up-to-date. In that case, we limit the number of retries to
maxRebroadcastAttemptsas to not indefinitely re-broadcast.