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: Add initial sync from checkpoints #3181

Merged
merged 3 commits into from
Sep 9, 2020

Conversation

jberci
Copy link
Contributor

@jberci jberci commented Aug 10, 2020

Closes #2479

go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/node.go Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be cool to also have some failure case tests, e.g., something like a Byzantine storage node that we could test against. Maybe for another issue.

go/oasis-test-runner/scenario/e2e/runtime/storage_sync.go Outdated Show resolved Hide resolved
@jberci jberci force-pushed the jberci/feature/checkpoints branch 4 times, most recently from 595397c to 7a68cf5 Compare August 26, 2020 14:20
@codecov
Copy link

codecov bot commented Aug 26, 2020

Codecov Report

Merging #3181 into master will increase coverage by 0.41%.
The diff coverage is 75.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3181      +/-   ##
==========================================
+ Coverage   67.43%   67.84%   +0.41%     
==========================================
  Files         370      371       +1     
  Lines       36541    36864     +323     
==========================================
+ Hits        24640    25009     +369     
+ Misses       8702     8679      -23     
+ Partials     3199     3176      -23     
Impacted Files Coverage Δ
go/registry/api/runtime.go 44.37% <0.00%> (-0.63%) ⬇️
go/storage/mkvs/checkpoint/checkpoint.go 86.66% <ø> (ø)
go/storage/mkvs/checkpoint/restorer.go 82.45% <0.00%> (-13.47%) ⬇️
go/worker/storage/committee/checkpoint_sync.go 74.91% <74.91%> (ø)
go/worker/storage/committee/node.go 74.59% <91.66%> (+0.77%) ⬆️
go/runtime/committee/committee.go 62.74% <100.00%> (+7.18%) ⬆️
go/storage/api/grpc.go 70.28% <100.00%> (ø)
go/storage/client/init.go 72.22% <100.00%> (+0.79%) ⬆️
go/worker/storage/worker.go 92.52% <100.00%> (+0.77%) ⬆️
go/consensus/tendermint/api/errors.go 86.66% <0.00%> (-13.34%) ⬇️
... and 30 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 69ae325...29e5473. Read the comment docs.

@jberci jberci force-pushed the jberci/feature/checkpoints branch 3 times, most recently from 0971968 to 67d5e63 Compare August 26, 2020 16:21
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests seem to be failing?

go/oasis-test-runner/oasis/log.go Outdated Show resolved Hide resolved
@jberci jberci force-pushed the jberci/feature/checkpoints branch 5 times, most recently from 2e5bda6 to 199beeb Compare September 3, 2020 16:00
go/storage/api/grpc.go Show resolved Hide resolved
go/worker/storage/committee/node.go Outdated Show resolved Hide resolved
go/storage/mkvs/checkpoint/restorer.go Outdated Show resolved Hide resolved
go/worker/storage/committee/checkpoint_sync.go Outdated Show resolved Hide resolved
@jberci jberci marked this pull request as ready for review September 4, 2020 11:12
@jberci jberci force-pushed the jberci/feature/checkpoints branch 3 times, most recently from c727310 to 8478b21 Compare September 4, 2020 16:02
@jberci jberci force-pushed the jberci/feature/checkpoints branch 3 times, most recently from cafc40b to 5e67de8 Compare September 5, 2020 21:27
go/storage/client/init.go Outdated Show resolved Hide resolved
@kostko
Copy link
Member

kostko commented Sep 8, 2020

Could you add a configuration flag, e.g. worker.storage.checkpoint_sync (which defaults to true) so that we can disable checkpoint sync (e.g., in tests).

@jberci jberci force-pushed the jberci/feature/checkpoints branch 7 times, most recently from 2b5cbc9 to 247897e Compare September 8, 2020 16:40
@jberci jberci merged commit 8314132 into master Sep 9, 2020
@jberci jberci deleted the jberci/feature/checkpoints branch September 9, 2020 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storage node sync should use checkpoints
2 participants