-
Notifications
You must be signed in to change notification settings - Fork 16
feat: simple leader election #1140
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
base: will/verifier-process
Are you sure you want to change the base?
Conversation
// LeaderElection provides a way to determine if an executor should be currently executing a given message. | ||
// This is used to prevent multiple executors from executing the same message concurrently. | ||
type LeaderElection interface { | ||
IsLeader(msgId [32]byte, destChainSelector uint64, offset uint8) bool |
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.
Is that the offset
of current node's place in the sorted nodes that can participate?
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.
Lets say message comes in at 0s unix, after 5m unix we want to swap over to the next executor. Offset is how many times we've swapped over (should loop back to zero, don't know if we want to handle it inside this code though, good point)
for _, offset := range tt.offsets { | ||
result := le.IsLeader(msgId, tt.destChainSelector, offset) | ||
if result != tt.expectedResult { | ||
t.Errorf("expected %v, got %v", tt.expectedResult, result) |
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.
that's a nit for all tests, use require.Equal
or similar instead of that pattern of adding a condition and then do t.Errorf
. I know it's only for illustrations purposes now so it doesn't matter much.
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.
good point!
|
||
// LeaderElection provides a way to determine if an executor should be currently executing a given message. | ||
// This is used to prevent multiple executors from executing the same message concurrently. | ||
type LeaderElection interface { |
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.
The way I was thinking about this Leader interface is that we'll have something like
GetMessagesSchedules(allMsgs) [][]msgs
Where msgs[0] are the ones that needs executing now by this leader, msgs[1] are next in line once all msgs[0] are executed,...etc. WDYT about that approach?
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.
I'm just trying to think about how it would be used. If the result of this election is some messages getting enqueue()
to the executor. i don't think theres a need to know what messages to execute after this batch
|
This PR adds in a simple leader election algorithm and an interface we can expand on if we want something more robust.
Open Questions: