feat:chain:splitstore chain prune #9056

ZenGround0 · 2022-07-19T03:00:30Z

Related Issues

Proposed Changes

Because many changes from the existing coldstore GC PR (#6728) have made it into lotus master and have been heavily modified with the last year of changes this PR is meant to replace #6728.

The scope of this PR is adding correct splitstore coldstore GC, integration tests of the splitstore, and the chain prune command to trigger this manually. Auto cold GC will happen in another PR

Additional Info

There are a few tricky changes to the splitstore itself

splitstore correctly serves transaction requests with respect to the right store during the critical section. compactType field and splitstore.go functions are modified to cover this.
purge is generalized to delete from the correct store (again compactType achieves this)
markset now generally contains both cold and hot store live cids in order to handle transaction protection uniformly. I've optimized puts to not be protected during cold compaction. Reification still needs work and I might do a similar optimization but cold reified objects might show up in hotstore compaction markset to keep things simple.
new paths and startup code so coldstore compaction can benefit from checkpoints and recovery

Testing also has some nuanced challenges. The splitstore / itest combined complex relies on the coldstore being the universal blockstore for genesis or any other setup state to be loaded. Coldstore can only be gced if it is backed by a badger blockstore as map blockstores don't implement the BlockstoreGC interface. This means that we need to add an option to itest setup to instantiate a fs repo to get a universal badger blockstore. We don't actually need this if we don't test coldstore gc.

Testing

framework writing small amount of hash linked state and then allowing to overwrite it to test whether hot and cold store moved around or deleted that state
One hotstore discard happy path test
One badger coldstore and chain prune happy path test

Checklist

Before you mark the PR ready for review, please make sure that:

All commits have a clear commit message.
The PR title is in the form of of <PR type>: <area>: <change being made>
- example: fix: mempool: Introduce a cache for valid signatures
- PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
- area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
This PR has tests for new functionality or change in behaviour
If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
CI is green

ZenGround0 · 2022-08-03T19:55:56Z

itests/kit/blockminer.go

@@ -255,6 +258,15 @@ func (bm *BlockMiner) MineBlocks(ctx context.Context, blocktime time.Duration) {
 		defer bm.wg.Done()

 		for {
+			if paused := atomic.LoadInt64(&bm.pause); paused == 1 {


I'm an atomic n00b so I might be getting the usage quite wrong

ZenGround0 · 2022-08-03T19:56:39Z

itests/kit/blockminer.go

@@ -283,6 +295,18 @@ func (bm *BlockMiner) InjectNulls(rounds abi.ChainEpoch) {
 	atomic.AddInt64(&bm.nextNulls, int64(rounds))
 }

+// Pause compels the miner to wait for a signal to restart
+func (bm *BlockMiner) Pause() {


These helped me avoid races in test and seem generally useful. Worth checking carefully to make sure I'm not opening up deadlocks.

itests/kit/ensemble.go

ZenGround0 · 2022-08-03T19:59:44Z

blockstore/idstore.go

@@ -102,6 +102,14 @@ func (b *idstore) Put(ctx context.Context, blk blocks.Block) error {
 	return b.bs.Put(ctx, blk)
 }

+func (b *idstore) ForEachKey(f func(cid.Cid) error) error {


Needed so that badger backed cold store satisfies the interface needed for pruning

ZenGround0 · 2022-08-03T20:01:25Z

blockstore/mem.go

@@ -14,35 +14,36 @@ func NewMemory() MemBlockstore {
 }

 // MemBlockstore is a terminal blockstore that keeps blocks in memory.
-type MemBlockstore map[cid.Cid]blocks.Block
+// To match behavior of badger blockstore we index by multihash only.
+type MemBlockstore map[string]blocks.Block


Something I learned during this work is that the badger backed blockstores store mh only as keys to blocks. Until now the MemBlockstore has had different behavior, only matching when codec also matches. When using a coldstore backed by this MemBlockstore this led to a byzantine failure case. I think this is more correct across the board.

ZenGround0 · 2022-08-03T20:34:07Z

I'll add a chain prune discussion tutorial before merging

magik6k

First pass, couple of (mostly nitpicky) comments. I feel like I have a decent idea of what's going on here, and the core logic makes sense.

Would be nice to setup some chaos-style tests for both this and hotstore compaction, crashing the process in the middle of compacting, and seeing if we resume correctly (not as a part of this PR, but before we call splitstore stable)

blockstore/mem.go

api/api_full.go

itests/kit/blockminer.go

blockstore/splitstore/splitstore.go

blockstore/splitstore/splitstore_prune.go

magik6k

Nothing serious to complain about.

Definitely needs lots of testing, and maybe measuring perf in a bunch of scenarios, but that seems to be planned already.

blockstore/splitstore/splitstore.go

blockstore/splitstore/splitstore_prune.go

magik6k · 2022-08-05T10:30:41Z

Nothing serious to complain about.

Well, other than a bunch of itests hanging for whatever reason

LesnyRumcajs mentioned this pull request Jul 22, 2022

Spike: garbage collection in Forest ChainSafe/forest#1708

Closed

ZenGround0 force-pushed the feat/chain-prune-reborn branch from 638f2f6 to 699f315 Compare August 3, 2022 19:33

ZenGround0 marked this pull request as ready for review August 3, 2022 19:34

ZenGround0 requested a review from a team as a code owner August 3, 2022 19:34

ZenGround0 requested review from vyzo, magik6k and arajasek August 3, 2022 19:46

ZenGround0 commented Aug 3, 2022

View reviewed changes

itests/kit/ensemble.go Outdated Show resolved Hide resolved

ZenGround0 commented Aug 3, 2022

View reviewed changes

ZenGround0 removed the request for review from vyzo August 4, 2022 03:34

magik6k reviewed Aug 4, 2022

View reviewed changes

ZenGround0 requested a review from magik6k August 5, 2022 04:34

magik6k approved these changes Aug 5, 2022

View reviewed changes

blockstore/splitstore/splitstore.go Show resolved Hide resolved

blockstore/splitstore/splitstore_prune.go Show resolved Hide resolved

ZenGround0 force-pushed the feat/chain-prune-reborn branch from aed3aea to 231135b Compare August 5, 2022 19:35

ZenGround0 added 11 commits August 5, 2022 16:07

Splitstore chain prune

736ba82

WIP

fd61271

Protect on reification for simpler logic and sound cold compact protect

ccc8dc3

Recovery from checkpoint

c00861b

Lint

1494fad

Get splitstore (discard mode) running in itests

e04ed7d

First splitstore itest

82f9f0e

Separate out to own file

ee5134a

Completed first e2e integration test of splitstore

21c7364

Dirty from debug but tests almost done and passing

44c76f0

More testing support

f191ad4

ZenGround0 added 13 commits August 5, 2022 16:07

fix coldstore test

3adcafe

Beginning cleanup for review

0f8aa8a

Cleanup

9f77597

Remove debug prints

802357d

Remove major debugging refactors from compaction

085e3a8

better unpause channel shutdown

45c65dc

make jen

60d6dd8

Review Response

b18372e

Fix MineBlocks

665c3d6

Init channel

42ea13d

Better input type for chain prune

84a9f79

More fix

d152108

make gen again :/

94a348f

ZenGround0 force-pushed the feat/chain-prune-reborn branch from 71343b9 to 94a348f Compare August 5, 2022 20:15

ZenGround0 merged commit 0c91b0d into master Aug 5, 2022

ZenGround0 deleted the feat/chain-prune-reborn branch August 5, 2022 20:34

TippyFlitsUK mentioned this pull request Aug 11, 2022

1.17.1 Release Planning #9158

Closed

magik6k mentioned this pull request Dec 5, 2022

Splitstore Garbage Collection #6728

Closed

aakoshh mentioned this pull request Dec 19, 2023

State tree / app state garbage collection consensus-shipyard/ipc#154

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:chain:splitstore chain prune #9056

feat:chain:splitstore chain prune #9056

ZenGround0 commented Jul 19, 2022 •

edited

Loading

ZenGround0 Aug 3, 2022

ZenGround0 Aug 3, 2022

ZenGround0 Aug 3, 2022 •

edited

Loading

ZenGround0 Aug 3, 2022

ZenGround0 commented Aug 3, 2022

magik6k left a comment

magik6k left a comment

magik6k commented Aug 5, 2022

feat:chain:splitstore chain prune #9056

feat:chain:splitstore chain prune #9056

Conversation

ZenGround0 commented Jul 19, 2022 • edited Loading

Related Issues

Proposed Changes

Additional Info

Checklist

ZenGround0 Aug 3, 2022

Choose a reason for hiding this comment

ZenGround0 Aug 3, 2022

Choose a reason for hiding this comment

ZenGround0 Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

ZenGround0 Aug 3, 2022

Choose a reason for hiding this comment

ZenGround0 commented Aug 3, 2022

magik6k left a comment

Choose a reason for hiding this comment

magik6k left a comment

Choose a reason for hiding this comment

magik6k commented Aug 5, 2022

ZenGround0 commented Jul 19, 2022 •

edited

Loading

ZenGround0 Aug 3, 2022 •

edited

Loading