Skip to content
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

go/worker/storage: Handle state sync before runtime is operational #4311

Merged
merged 2 commits into from
Oct 19, 2021

Conversation

kostko
Copy link
Member

@kostko kostko commented Oct 18, 2021

Previously there was an edge case that was not handled when the runtime was
registered in the consensus layer but not yet operational (e.g., there have been
no normal blocks yet). If a new node used state sync to quickly catch up with
the consensus layer and sync to a height after runtime's genesis, the node would
never register as it would keep waiting for storage checkpoints (which wouldn't
exist yet).

TODO

  • Add e2e test if possible.

@kostko kostko added c:storage Category: storage c:bug Category: bug labels Oct 18, 2021
@kostko kostko force-pushed the kostko/fix/rt-storage-state-sync branch 3 times, most recently from b3b2ddb to c5c63aa Compare October 18, 2021 16:48
@codecov
Copy link

codecov bot commented Oct 18, 2021

Codecov Report

Merging #4311 (23039a5) into master (1c2afad) will decrease coverage by 0.07%.
The diff coverage is 29.33%.

❗ Current head 23039a5 differs from pull request most recent head 0f7ccd3. Consider uploading reports for the commit 0f7ccd3 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4311      +/-   ##
==========================================
- Coverage   68.59%   68.51%   -0.08%     
==========================================
  Files         411      411              
  Lines       47271    47346      +75     
==========================================
+ Hits        32424    32441      +17     
- Misses      10805    10862      +57     
- Partials     4042     4043       +1     
Impacted Files Coverage Δ
go/worker/storage/committee/node.go 72.91% <13.72%> (-4.15%) ⬇️
go/runtime/history/history.go 66.66% <33.33%> (-2.44%) ⬇️
go/runtime/history/db.go 81.57% <80.00%> (-0.18%) ⬇️
go/worker/storage/service_external.go 55.95% <0.00%> (-4.77%) ⬇️
go/storage/api/root_cache.go 75.00% <0.00%> (-4.17%) ⬇️
go/consensus/tendermint/apps/beacon/beacon.go 73.77% <0.00%> (-3.28%) ⬇️
go/consensus/tendermint/apps/beacon/state/state.go 72.97% <0.00%> (-2.71%) ⬇️
go/worker/keymanager/worker.go 63.58% <0.00%> (-2.57%) ⬇️
go/consensus/tendermint/full/full.go 64.92% <0.00%> (-1.78%) ⬇️
go/oasis-node/cmd/node/node.go 55.69% <0.00%> (-1.70%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f6a6cf...0f7ccd3. Read the comment docs.

@kostko kostko force-pushed the kostko/fix/rt-storage-state-sync branch from c5c63aa to 23039a5 Compare October 18, 2021 16:53
@kostko kostko marked this pull request as ready for review October 18, 2021 16:59
Previously there was an edge case that was not handled when the runtime was
registered in the consensus layer but not yet operational (e.g., there have been
no normal blocks yet). If a new node used state sync to quickly catch up with
the consensus layer and sync to a height after runtime's genesis, the node would
never register as it would keep waiting for storage checkpoints (which wouldn't
exist yet).
Upstream dependencies must update and runtimes should never be querying
local time anyway.
@kostko kostko force-pushed the kostko/fix/rt-storage-state-sync branch from 23039a5 to 0f7ccd3 Compare October 19, 2021 09:32
@kostko kostko enabled auto-merge October 19, 2021 09:33
@kostko kostko merged commit 973cf48 into master Oct 19, 2021
@kostko kostko deleted the kostko/fix/rt-storage-state-sync branch October 19, 2021 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:bug Category: bug c:storage Category: storage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants