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

Move storage backend initialization to storage worker #3373

Merged
merged 2 commits into from
Oct 8, 2020

Conversation

jberci
Copy link
Contributor

@jberci jberci commented Oct 5, 2020

Closes #3323

@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #3373 into master will increase coverage by 0.15%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3373      +/-   ##
==========================================
+ Coverage   66.04%   66.20%   +0.15%     
==========================================
  Files         371      371              
  Lines       33284    33293       +9     
==========================================
+ Hits        21982    22041      +59     
+ Misses       8075     8019      -56     
- Partials     3227     3233       +6     
Impacted Files Coverage Δ
go/worker/storage/committee/node.go 73.88% <ø> (+0.26%) ⬆️
go/worker/storage/crashing.go 100.00% <ø> (ø)
go/worker/storage/worker.go 87.09% <44.44%> (-4.31%) ⬇️
go/runtime/registry/registry.go 71.97% <67.74%> (+0.94%) ⬆️
go/storage/api/metrics.go 90.76% <80.00%> (ø)
go/oasis-node/cmd/node/node.go 53.53% <84.61%> (-0.16%) ⬇️
go/roothash/tests/tester.go 88.43% <100.00%> (ø)
go/runtime/tagindexer/tagindexer.go 66.27% <100.00%> (-0.39%) ⬇️
go/storage/client/init.go 72.22% <100.00%> (ø)
go/worker/common/worker.go 89.47% <100.00%> (+0.09%) ⬆️
... and 37 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 20c2b93...1cc252d. Read the comment docs.

@jberci jberci force-pushed the jberci/feature/storage branch from 0ba016a to cb7f369 Compare October 5, 2020 15:46
@jberci jberci marked this pull request as ready for review October 5, 2020 15:46
@jberci jberci force-pushed the jberci/feature/storage branch 3 times, most recently from ce15ec4 to 5255024 Compare October 5, 2020 16:52
@jberci jberci mentioned this pull request Oct 5, 2020
go: Move storage backend initialization to storage worker

Notably, this also means that node command line options for storage
configuration have been renamed (`--storage.*` to `--worker.storage.*`).
Copy link
Member

Choose a reason for hiding this comment

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

Add a cfg changelog fragment and list the changes please.

Copy link
Member

@ptrus ptrus Oct 6, 2020

Choose a reason for hiding this comment

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

Also there's an additional configuration change that should also be listed: clients do not need to configure storage backed anymore

docs/oasis-node/metrics.md Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Outdated Show resolved Hide resolved
@jberci jberci force-pushed the jberci/feature/storage branch 2 times, most recently from bc5f233 to dd5c1c8 Compare October 6, 2020 16:48
.changelog/3323.cfg.2.md Outdated Show resolved Hide resolved
// Close tag indexer service.
r.tagIndexer.Stop()
<-r.tagIndexer.Quit()
if !r.finished {
Copy link
Member

Choose a reason for hiding this comment

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

Does this require holding a mutex?

go/runtime/registry/registry.go Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Show resolved Hide resolved
go/runtime/registry/registry.go Show resolved Hide resolved
go/runtime/tagindexer/tagindexer.go Outdated Show resolved Hide resolved
go/runtime/registry/registry.go Outdated Show resolved Hide resolved
@jberci jberci force-pushed the jberci/feature/storage branch 3 times, most recently from 79eade2 to b3af377 Compare October 7, 2020 13:50
@jberci jberci force-pushed the jberci/feature/storage branch from b3af377 to 1eea9f7 Compare October 8, 2020 11:11
@jberci jberci force-pushed the jberci/feature/storage branch from 1eea9f7 to 1cc252d Compare October 8, 2020 11:28
@jberci jberci merged commit d9d1229 into master Oct 8, 2020
@jberci jberci deleted the jberci/feature/storage branch October 8, 2020 12:01
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.

Client storage backend should be removed as a configurable backend
3 participants