-
Notifications
You must be signed in to change notification settings - Fork 182
node: call CreateSnapshot explicitly when node starts #1829
Conversation
312000e
to
b48e27b
Compare
71080f4
to
f3605ce
Compare
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 sorry, but I cannot understand some points. Can you explain more?
if no such snapshot function is called until the next governance block is processed,
It seems that a node calls snapshot()
on every block to verifyHeader.
this problem is caused by #1706
The PR disables "write" in some cases, but you are saying that there is an issue on "read" (restoration of vote data). So, I am confused.
In a syncing situation, a snapshot is called, governance state data is writable, so governance state data is written(e.g. changeSet, currentSet). But the problem is that if the snapshot already exists, it will be hit and returned directly from the cache, and the code that writes the governance state data is not executed. (When I say that the snapshot already exists, I am assuming that the code has been run once when creating the snapshot.) On second thought, I realized that �it is good to change the implementation, because the fundamental solution is to decouple the governance state data recovery (specifically changeSet) from the snapshot call.
|
- latest synced blocks are the blocks from lastGovernanceStateBlock to currentBlock - in-memory snapshots and related gov states are missed when node stops, so restore it when node starts
f3605ce
to
605ca57
Compare
Dear reviewers, please review this PR one more time. I'll go as #1829 (comment). |
Proposed changes
snapshot
call.snapshot
is called by getValidator, it creates the snapshot caches without restoring gov.changeSet (because the snapshot called by getValidator is not able to write governance data)snapshot
call skip the restoration part always whether it is able to write governance data or not because there's already snapshot cache.snapshot
explictly before sync starts to ensure the firstsnapshot
call restores gov.changeSet always.dev
PR
Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test
)Related issues
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...