Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Limit the number of leaves per level #1544

Closed
wants to merge 12 commits into from
Closed

Conversation

davxy
Copy link
Member

@davxy davxy commented Aug 12, 2022

Closes #432


⚠ DEPENDS ON: paritytech/substrate#12005
(for correct blocks removal from the backend)


This PR prevents the StateDbError::TooManySiblingBlocks error from being triggered by eagerly removing leaves from the backend on block import and before the error condition is met.


The strategy is to remove the older block to make space for a new fresh block when we reach a configurable upper bound.

To detect which is the older block at the given level currently we are relying on the implementation of the Backend::leaves() method that ships with substrate:

https://github.com/paritytech/substrate/blob/6b8553511112afd5ae7e8e6877dc2f467850f155/client/api/src/leaves.rs#L204-L206

That is, our implementation already returns the leaves sorted by insertion time (older first).

(Before this, as an alternative - more portable but less elegant - implementation, I was maintaining for every leaf its import order, e.g. its timestamp).

In the end I've preferred to rely on the assumption that the leaves are already given ordered by insertion time to keep it simple.


If this strategy is good, then I suggest to modify the blockchain Backend trait leaves() method documentation to better fit our ordering assumptions.

https://github.com/paritytech/substrate/blob/6b8553511112afd5ae7e8e6877dc2f467850f155/primitives/blockchain/src/backend.rs#L98-L101

From:

Results must be ordered best (longest, highest) chain first.

To:

Results must be ordered best (longest,highest) chain first and within each level ordered by import time (older first).

@davxy davxy changed the title Davxy/remove leaves Limit the number of leaves per level Aug 12, 2022
@davxy davxy self-assigned this Aug 12, 2022
@davxy davxy added B0-silent Changes should not be mentioned in any release notes A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. labels Aug 12, 2022
@davxy davxy requested review from skunert, bkchr and a team August 12, 2022 14:47
@davxy davxy closed this Aug 19, 2022
@bkchr bkchr deleted the davxy/remove-leaves branch August 19, 2022 17:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove "dead" leaves
2 participants