Support proposervm historical block deletion #1929

StephenButtolph · 2023-08-25T22:56:58Z

Why this should be merged

High performance blockchains issue large blocks and issue blocks frequently. Even if such blockchain has a robust state bloat prevention mechanism, the block data also needs to be maintained. This allows node operators to restrict the number of historical blocks that will be kept on disk.

It's important for a network maintainer to be aware however that historical block state may be lost if every node is configured to delete historical state. Thankfully historical block state isn't required for validators, so specialized storage solutions can be readily applied here.

How this works

Added proposerNumHistoricalBlocks option to subnet configs.
Added block pruner that runs on startup to cleanup stale blocks.
Added historical block deletion that runs as part of block acceptance.
Required that the height index is fully repaired for any chain attempting to prune old blocks.
Should guaranteed that the indexed heights are continuous in the range [lastAcceptedHeight - proposerNumHistoricalBlocks, lastAcceptedHeight]
- ex: lastAcceptedHeight=10 proposerNumHistoricalBlocks=2 indexed heights should be [8,10] because 10 is not historical, 9 and 8 are the 2 most recent historical blocks.

How this was tested

Unit test
Local testing (see below)

- Start new network with proposerNumHistoricalBlocks = 0 (infinite default)

[08-25|19:23:05.869] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "2Qpky4XexDu2dDhWbvYmNJ97SUWr1SKhyFKwnnMcqZadfZumsS", "height": 1}
[08-25|19:23:23.392] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "269AoJMCD9jGwjhRmQDqbtoMTFDXwDjcxMoKsoyDVdrdXr6tNQ", "height": 2}
[08-25|19:23:29.030] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "2anCvpG5waoVNC3DbMMgAExqDPXFDcgWwsFumvoYiskYvKxXrw", "height": 3}
[08-25|19:23:34.191] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "i16mGnEHTzniergZNXS38igngeLBY3AtwoowFj7t2vdD2TeLm", "height": 4}
[08-25|19:23:39.273] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "MdKWNyXTanP1gkeYUevsnJAqvWGs3SDpjC6Q2Z4dnTDbBmQhy", "height": 5}

- Restart setting proposerNumHistoricalBlocks = 2

[08-25|19:27:56.258] DEBUG <X Chain> proposervm/height_indexed_vm.go:205 deleted block {"blkID": "2Qpky4XexDu2dDhWbvYmNJ97SUWr1SKhyFKwnnMcqZadfZumsS", "height": 1}
[08-25|19:27:56.258] DEBUG <X Chain> proposervm/height_indexed_vm.go:205 deleted block {"blkID": "269AoJMCD9jGwjhRmQDqbtoMTFDXwDjcxMoKsoyDVdrdXr6tNQ", "height": 2}
[08-25|19:30:31.626] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "ct964fFALeNqzV7tjEtJ7HMnQhBkfgX6tKnNYcvCbWLjcq1sG", "height": 6}
[08-25|19:30:31.626] DEBUG <X Chain> proposervm/height_indexed_vm.go:172 deleted block {"blkID": "2anCvpG5waoVNC3DbMMgAExqDPXFDcgWwsFumvoYiskYvKxXrw", "height": 3}
[08-25|19:30:56.282] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "KCv9wv9kHwSN3iVTVeN4cU4zyrNGPJNpwpr6SEGqi4R1A7jaU", "height": 7}
[08-25|19:30:56.282] DEBUG <X Chain> proposervm/height_indexed_vm.go:172 deleted block {"blkID": "i16mGnEHTzniergZNXS38igngeLBY3AtwoowFj7t2vdD2TeLm", "height": 4}

- Restart setting proposerNumHistoricalBlocks = 4

[08-25|19:32:02.101] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "2oEHCZHJf34m6LrUwyHX4ui7oAbG39rRNbibZVChRYmSiRC4RK", "height": 8}
[08-25|19:33:00.551] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "2UhLwuwAyZDVTXsEiwjiaV55HhyxXciBkxcQaTd94kc4UQYkYb", "height": 9}
[08-25|19:33:17.375] DEBUG <X Chain> proposervm/height_indexed_vm.go:132 indexed block {"blkID": "2TqxFYspbjLFdVgBaENaLSe83z2vLkxTmB3dD6yVkRE2oqB2pJ", "height": 10}
[08-25|19:33:17.375] DEBUG <X Chain> proposervm/height_indexed_vm.go:172 deleted block {"blkID": "MdKWNyXTanP1gkeYUevsnJAqvWGs3SDpjC6Q2Z4dnTDbBmQhy", "height": 5}

StephenButtolph · 2023-08-25T23:01:45Z

vms/proposervm/state/block_state.go

@@ -134,3 +135,8 @@ func (s *blockState) PutBlock(blk block.Block, status choices.Status) error {
 	s.blkCache.Put(blkID, &blkWrapper)
 	return s.db.Put(blkID[:], bytes)
 }
+
+func (s *blockState) DeleteBlock(blkID ids.ID) error {
+	s.blkCache.Evict(blkID)


Should we s.blkCache.Put(blkID, nil) here rather than evicting?

Delete seems fine with me.
IIRC we peek historical blocks only via GetAncestors call which should stop at the first database.ErrNotFound. I'd rather use the cache for fresh blocks, rather than historical ones

StephenButtolph · 2023-08-26T00:30:19Z

Still need to write some unit tests - but I think this is ready for at least a first pass review

abi87

questions and minor requests for comments

subnets/config.go

vms/proposervm/height_indexed_vm.go

vms/proposervm/state/block_height_index.go

abi87 · 2023-08-28T16:09:39Z

vms/proposervm/state/block_state.go

@@ -134,3 +135,8 @@ func (s *blockState) PutBlock(blk block.Block, status choices.Status) error {
 	s.blkCache.Put(blkID, &blkWrapper)
 	return s.db.Put(blkID[:], bytes)
 }
+
+func (s *blockState) DeleteBlock(blkID ids.ID) error {
+	s.blkCache.Evict(blkID)


Delete seems fine with me.
IIRC we peek historical blocks only via GetAncestors call which should stop at the first database.ErrNotFound. I'd rather use the cache for fresh blocks, rather than historical ones

abi87 · 2023-08-28T16:11:30Z

vms/proposervm/vm.go

@@ -599,7 +619,10 @@ func (vm *VM) repairAcceptedChainByHeight(ctx context.Context) error {

 	newProLastAcceptedID, err := vm.State.GetBlockIDAtHeight(innerLastAcceptedHeight)
 	if err != nil {
-		return err
+		// This fatal error can happen if NumHistoricalBlocks is set too


q: again this shouldn't really happen unless innerVM is being clever right?

That's correct - this should not happen unless the inner VM had a rollback more than the proposervm is configured to support.

vms/proposervm/vm_test.go

vms/proposervm/height_indexed_vm.go

subnets/config.go

vms/proposervm/height_indexed_vm.go

rarepepi · 2023-08-29T17:34:54Z

LGTM

StephenButtolph added 2 commits August 25, 2023 18:48

Support proposervm historical block deletion

5e0d4fd

Add TODOs

eb7fa86

StephenButtolph commented Aug 25, 2023

View reviewed changes

StephenButtolph added this to the v1.10.10 milestone Aug 25, 2023

StephenButtolph self-assigned this Aug 25, 2023

StephenButtolph added 3 commits August 25, 2023 19:35

Populate default value for primary network

7bf53bd

Add invariant comment to config

69b12e0

idk grammar - be consistent

9eb82f0

StephenButtolph requested a review from abi87 August 26, 2023 00:29

StephenButtolph added 3 commits August 27, 2023 14:18

Improve error + comments

9b48986

nit

d82a041

Merge branch 'dev' into proposervm-block-deletion

fbc5101

StephenButtolph marked this pull request as ready for review August 27, 2023 18:28

Add test

915a3b5

abi87 requested changes Aug 28, 2023

View reviewed changes

StephenButtolph added 6 commits August 28, 2023 12:42

cleanup test

15e73dd

Add comment

8efd994

Merge branch 'dev' into proposervm-block-deletion

62e5cad

Merge branch 'dev' into proposervm-block-deletion

0868049

Add comment

9fe8e8e

Merge branch 'dev' into proposervm-block-deletion

2be9520

patrick-ogrady reviewed Aug 29, 2023

View reviewed changes

vms/proposervm/height_indexed_vm.go Show resolved Hide resolved

patrick-ogrady reviewed Aug 29, 2023

View reviewed changes

subnets/config.go Outdated Show resolved Hide resolved

subnets/config.go Show resolved Hide resolved

vms/proposervm/height_indexed_vm.go Show resolved Hide resolved

Address comments

9817f3b

patrick-ogrady approved these changes Aug 29, 2023

View reviewed changes

StephenButtolph merged commit dd78602 into dev Aug 29, 2023
16 checks passed

StephenButtolph deleted the proposervm-block-deletion branch August 29, 2023 17:47

patrick-ogrady mentioned this pull request Aug 29, 2023

[experiment] Do we need to store blocks ourself? ava-labs/hypersdk#302

Closed

This was referenced Aug 29, 2023

[storage + vm] Allow VM to delete proposervm blocks (or make blocks smaller) ava-labs/hypersdk#130

Closed

[vm] Block Pruning (Default to Only Keeping Last 768 Blocks) ava-labs/hypersdk#436

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support proposervm historical block deletion #1929

Support proposervm historical block deletion #1929

StephenButtolph commented Aug 25, 2023 •

edited

StephenButtolph Aug 25, 2023

abi87 Aug 28, 2023

StephenButtolph commented Aug 26, 2023 •

edited

abi87 left a comment

abi87 Aug 28, 2023

abi87 Aug 28, 2023

StephenButtolph Aug 28, 2023

rarepepi commented Aug 29, 2023

Support proposervm historical block deletion #1929

Support proposervm historical block deletion #1929

Conversation

StephenButtolph commented Aug 25, 2023 • edited

Why this should be merged

How this works

How this was tested

StephenButtolph Aug 25, 2023

Choose a reason for hiding this comment

abi87 Aug 28, 2023

Choose a reason for hiding this comment

StephenButtolph commented Aug 26, 2023 • edited

abi87 left a comment

Choose a reason for hiding this comment

abi87 Aug 28, 2023

Choose a reason for hiding this comment

abi87 Aug 28, 2023

Choose a reason for hiding this comment

StephenButtolph Aug 28, 2023

Choose a reason for hiding this comment

rarepepi commented Aug 29, 2023

StephenButtolph commented Aug 25, 2023 •

edited

StephenButtolph commented Aug 26, 2023 •

edited