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

Implement splitstore cold object reification #6914

Closed
wants to merge 11 commits into from

Conversation

vyzo
Copy link
Contributor

@vyzo vyzo commented Jul 28, 2021

See #6726 for context.

This implements a specialized view for the splitstore which reifies cold objects on hotstore misses.

The view is hooked to the VM through the constructor for now but I am not sure that's the right place to do it.

One alternative would be to change Chainstore.StateBlockstore() to take a hotview parameter, but that would touch too many call sites.
But maybe that's the right thing to do, open to discussion about this.

Another alternative is to make a Chainstore.StateBlockstoreHotView() and call that when we want to use the hotview.
We'd have to go through the vm constructor call sites and see whether we want to use it.

Edit: the hotview is presented through the StateBlockstore accessor in Chainstore. This ensure that state accesses are reifying. Maybe that's too broad though and we need to introduce and alternate accessor which uses the reifying view, and hunt down callsites to decide what's appropriate.

@vyzo vyzo requested a review from Stebalien July 28, 2021 16:47
@vyzo vyzo force-pushed the feat/splitstore-cold-object-reification branch from 89afd63 to 6335d03 Compare July 28, 2021 16:53
@vyzo
Copy link
Contributor Author

vyzo commented Jul 28, 2021

rebased on master.

@vyzo vyzo added epic/splitstore team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs labels Jul 28, 2021
@vyzo vyzo force-pushed the feat/splitstore-cold-object-reification branch from 73d4075 to 0220f29 Compare July 29, 2021 06:00
@vyzo
Copy link
Contributor Author

vyzo commented Jul 29, 2021

rebased on master.

@vyzo vyzo force-pushed the feat/splitstore-cold-object-reification branch 2 times, most recently from 1c677eb to 4af2426 Compare July 29, 2021 08:05
@vyzo
Copy link
Contributor Author

vyzo commented Jul 29, 2021

did a rebase and some commit management; there is no need to change the VM, the view should be provided by Chainstore.StateBlockstore.

@vyzo vyzo force-pushed the feat/splitstore-cold-object-reification branch 2 times, most recently from ef58a19 to 89c26ba Compare July 29, 2021 17:41
@vyzo
Copy link
Contributor Author

vyzo commented Jul 29, 2021

rebased on master.

@jennijuju jennijuju added this to the v1.13.1 milestone Oct 12, 2021
@vyzo vyzo force-pushed the feat/splitstore-cold-object-reification branch from 89c26ba to 455fd85 Compare October 20, 2021 08:44
@vyzo
Copy link
Contributor Author

vyzo commented Oct 20, 2021

rebased on master.

@codecov
Copy link

codecov bot commented Oct 20, 2021

Codecov Report

Merging #6914 (ce558b8) into master (d910098) will increase coverage by 0.08%.
The diff coverage is 69.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6914      +/-   ##
==========================================
+ Coverage   39.84%   39.93%   +0.08%     
==========================================
  Files         632      634       +2     
  Lines       66955    67085     +130     
==========================================
+ Hits        26681    26793     +112     
+ Misses      35683    35668      -15     
- Partials     4591     4624      +33     
Impacted Files Coverage Δ
blockstore/blockstore.go 62.96% <ø> (ø)
blockstore/splitstore/splitstore_hotview.go 47.61% <47.61%> (ø)
chain/store/store.go 63.34% <50.00%> (+0.34%) ⬆️
blockstore/splitstore/splitstore_reify.go 64.28% <64.28%> (ø)
blockstore/splitstore/splitstore.go 50.56% <92.50%> (+26.69%) ⬆️
chain/actors/builtin/miner/diff.go 48.52% <0.00%> (-10.30%) ⬇️
extern/sector-storage/manager_calltracker.go 57.70% <0.00%> (-4.85%) ⬇️
chain/exchange/client.go 52.96% <0.00%> (-4.67%) ⬇️
chain/exchange/peer_tracker.go 66.66% <0.00%> (-4.31%) ⬇️
chain/stmgr/call.go 71.51% <0.00%> (-3.64%) ⬇️
... and 18 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 d910098...ce558b8. Read the comment docs.

@vyzo vyzo requested a review from raulk October 20, 2021 09:21
@vyzo vyzo marked this pull request as ready for review October 20, 2021 09:22
@vyzo vyzo requested a review from a team as a code owner October 20, 2021 09:22
@jennijuju jennijuju added the P2 P2: Should be resolved label Oct 22, 2021
@BigLep BigLep linked an issue Oct 28, 2021 that may be closed by this pull request
@Kubuxu Kubuxu closed this Nov 25, 2021
@Kubuxu Kubuxu deleted the feat/splitstore-cold-object-reification branch November 25, 2021 19:01
@Kubuxu Kubuxu restored the feat/splitstore-cold-object-reification branch November 25, 2021 19:08
@Kubuxu Kubuxu reopened this Nov 25, 2021
@vyzo
Copy link
Contributor Author

vyzo commented Dec 1, 2021

following sync discussion with @magik6k we decided to use a context option for reification and only do it for block validation.

Will rework this after the blockstore upgrade work is done.

@vyzo
Copy link
Contributor Author

vyzo commented Feb 4, 2022

Closing to cherry pick and rework on top of #8008.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 P2: Should be resolved team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Splitstore: reification of cold objects
3 participants