-
Notifications
You must be signed in to change notification settings - Fork 219
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
add pausable uptime manager #1340
base: validator-state
Are you sure you want to change the base?
Conversation
e156f9b
to
549d319
Compare
1520c18
to
0f16af2
Compare
|
||
var _ validators.StateCallbackListener = &pausableManager{} | ||
|
||
var ErrPausedDc = errors.New("paused node cannot be disconnected") |
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 avoid abbreviation of Disconnected to Dc
// We should never see this case | ||
return ErrPausedDc |
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.
This error should not be triggered by calling exported functions if I understand correctly. (we can unexport it)
|
||
// pause pauses uptime tracking for the node with the given ID | ||
// pause can disconnect the node from the uptime.Manager if it is connected. | ||
// Returns an error if the node is already paused. |
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.
seems this is missing?
|
||
// resume resumes uptime tracking for the node with the given ID | ||
// resume can connect the node to the uptime.Manager if it was connected. | ||
// Returns an error if the node is not paused. |
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.
seems this is missing?
currentTime = addTime(clk, 8*time.Second) | ||
// Uptime should have increased by 8 seconds since the node was connected | ||
checkUptime(t, up, nodeID0, 21*time.Second, currentTime) | ||
} |
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 think we could improve this test using a table, may require some moving items around.
One approach could have callback fns like beforePause
, afterPause
etc
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.
some test cases require restart/reinitializing of manager. so not sure converting it to a table is a easy one.
Why this should be merged
Adds pausable uptime manager interface for uptime tracking
How this works
Pausable uptime manager extends the avalanchego uptime manager by introducing callback listeners for validators state (under
plugin/evm/validators
). The idea is to pause a node's uptime and then can continue whenever it's resumed. It also modifiesConnect
,Disconnect,
StartTracking` accordingly.How this was tested
Added tests
How is this documented
Comments in code