feat(block manager): rotation graceful role rotation #1154
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This was
linked to
issues
Oct 21, 2024
keruch
requested changes
Oct 22, 2024
| go uevent.MustSubscribe(ctx, m.Pubsub, "syncTargetLoop", settlement.EventQueryNewSettlementBatchAccepted, m.onNewStateUpdate, m.logger) | ||
| } | ||
|
|
||
| func (m *Manager) runProducerLoops(ctx context.Context) { |
There was a problem hiding this comment.
maybe it's better to call it runProposerLoops and runNonProposerLoops respectively? not to confuse the terminology
| @@ -54,6 +54,8 @@ type Manager struct { | |||
| DAClient da.DataAvailabilityLayerClient | |||
| SLClient settlement.ClientI | |||
|
|
|||
| isProposer bool // is the local node the proposer | |||
| roleSwitchC chan bool // channel to receive role switch signal | |||
There was a problem hiding this comment.
so true is this chan means a proposer and false is not a proposer, right? seems a bit intransparent for me, i'd use an enum here or at least add a comment with explanations
type Role bool
const (
Proposer Role = true
FullNode Role = false
)| @@ -133,7 +135,8 @@ func NewManager( | |||
| blockCache: &Cache{ | |||
| cache: make(map[uint64]types.CachedBlock), | |||
| }, | |||
| pruningC: make(chan int64, 10), // use of buffered channel to avoid blocking applyBlock thread. In case channel is full, pruning will be skipped, but the retain height can be pruned in the next iteration. | |||
| pruningC: make(chan int64, 10), // use of buffered channel to avoid blocking applyBlock thread. In case channel is full, pruning will be skipped, but the retain height can be pruned in the next iteration. | |||
| roleSwitchC: make(chan bool, 1), // channel to be used to signal role switch | |||
Comment on lines
+117
to
+119
| // signal the role switch, in case where this node is the new proposer | ||
| // the other direction is handled elsewhere | ||
| if isNewProposer && block.Header.Height == m.TargetHeight.Load() { |
There was a problem hiding this comment.
let's add a comment (or maybe even a condition) that the code in if can be called only in a full node mode. meaning, if the node is a proposer, then it can't switch to the proposer mode once again.
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.
This PR allows nodes to switch role (proposer -> non-proposer, and vice versa) without complete reboot
It splits the background loops to
when a role change happens, the old loops shutdown, and new role loops starts
how role switch triggered?
proposer -> non-proposer change triggered by monitoring the SL
non-proposer -> proposer is triggered when applying a block that sets the node as the next proposer
This PR closes: #1008 , #1135 , #1148