-
Notifications
You must be signed in to change notification settings - Fork 175
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
[Consensus] Follower engine message queue #3479
[Consensus] Follower engine message queue #3479
Conversation
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Codecov Report
@@ Coverage Diff @@
## feature/active-pacemaker #3479 +/- ##
=============================================================
- Coverage 55.75% 44.18% -11.57%
=============================================================
Files 769 228 -541
Lines 71245 25244 -46001
=============================================================
- Hits 39723 11155 -28568
+ Misses 28311 13070 -15241
+ Partials 3211 1019 -2192
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
93f89c9
to
8eda5c0
Compare
Originally I didn't plan to implement standalone startup as part of this PR but observing that some nodes already start follower loop separately I've decided to implement this for other nodes otherwise it was resulting in double |
@@ -735,19 +738,12 @@ func (exeNode *ExecutionNode) LoadIngestionEngine( | |||
return exeNode.ingestionEng, err | |||
} | |||
|
|||
func (exeNode *ExecutionNode) LoadFollowerEngine( | |||
func (exeNode *ExecutionNode) LoadConsensusCommittee( |
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.
Previously we didn't start consensus committee it kinda worked in integration tests but generally speaking we would be missing some events without it
…flow-go into yurii/6173-follower-engine-message-queue
…thub.com/onflow/flow-go into yurii/6173-follower-engine-message-queue
…flow-go into yurii/6173-follower-engine-message-queue
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.
🎉
// No errors are expected during normal operation. All returned exceptions are potential | ||
// symptoms of internal state corruption and should be fatal. | ||
func (e *Engine) processQueuedBlocks() error { | ||
for { |
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.
should we add a termination check here? Like this:
flow-go/engine/collection/compliance/engine.go
Lines 171 to 175 in 00033d2
select { | |
case <-doneSignal: | |
return nil | |
default: | |
} |
Maybe best to append do PR #3512
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
…d follower engines
…thub.com/onflow/flow-go into yurii/6173-follower-engine-message-queue
bors merge |
https://github.com/dapperlabs/flow-go/issues/6173
Context
This PR implements message queues and message processing on dedicated worker for follower engine. It also updates
FollowerLoop
to usecomponent.ComponentManager
instead ofSingleRunner
.This PR is required to implement https://github.com/dapperlabs/flow-go/issues/6424 because follower engine and compliance engine need to implement the same interface.
Note on updating follower engine. This PR introduces message queues, proper implementation of
component.Component
interface and standalone startup of follower loop. Other changes will be handled in separate PR(like extracting interface for compliance engine).Originally I didn't plan to implement standalone startup as part of this PR but observing that some nodes already start follower loop separately I've decided to implement this for other nodes otherwise it was resulting in double
Start
which is a panic.Notable changes
ComponentManager
FollowerLoop
to work with buffered queuesSingleRunner