-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat(block manager): rotation graceful role rotation #1154
base: main
Are you sure you want to change the base?
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
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.
why is it buffered?
// 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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