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

Only calculate tree route during finalization when there are multiple leaves #14067

Conversation

skunert
Copy link
Contributor

@skunert skunert commented May 3, 2023

Calculation of these tree routes is very expensive. When the finalized block is very far from the best block, it can take multiple seconds to calculate this tree route, blocking the import lock.

If there is only one leave, we can skip this calculation since the best block is guaranteed to be a descendant of the new finalized one.

Closes paritytech/cumulus#2494, paritytech/polkadot-sdk#13,

Could impact the parachain part of paritytech/polkadot-sdk#11

Edit: Do not close the above issues for now, freshly syncing node still has issues.

@skunert skunert added 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T0-node This PR/Issue is related to the topic “node”. labels May 3, 2023
@skunert skunert requested a review from a team May 3, 2023 09:50
client/service/src/client/client.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@altonen altonen left a comment

Choose a reason for hiding this comment

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

Edit: Do not close the above issues for now, freshly syncing node still has issues.

what kind of issues?

Co-authored-by: Bastian Köcher <git@kchr.de>
@skunert
Copy link
Contributor Author

skunert commented May 3, 2023

Edit: Do not close the above issues for now, freshly syncing node still has issues.

what kind of issues?

False alert, I was running off a db which was close to the tip of the chain, this PR has indeed the desired effect.

@skunert
Copy link
Contributor Author

skunert commented May 3, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@paritytech-processbot paritytech-processbot bot merged commit 5a81bad into paritytech:master May 3, 2023
@jasl
Copy link
Contributor

jasl commented May 3, 2023

Thank you for the work!
Could it be backported to polkadot-v0.9.42 branch?

chevdor pushed a commit that referenced this pull request May 3, 2023
… leaves (#14067)

* Only calculate tree route when there are multiple leaves

* Update client/service/src/client/client.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
chevdor added a commit that referenced this pull request May 3, 2023
… leaves (#14067) (#14069)

* Only calculate tree route when there are multiple leaves

* Update client/service/src/client/client.rs



---------

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
gpestana pushed a commit that referenced this pull request May 4, 2023
… leaves (#14067)

* Only calculate tree route when there are multiple leaves

* Update client/service/src/client/client.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
crystalin pushed a commit to moonbeam-foundation/substrate that referenced this pull request May 5, 2023
… leaves (paritytech#14067)

* Only calculate tree route when there are multiple leaves

* Update client/service/src/client/client.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
… leaves (paritytech#14067)

* Only calculate tree route when there are multiple leaves

* Update client/service/src/client/client.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
vanderian pushed a commit to gasp-xyz/substrate that referenced this pull request Jul 26, 2023
… leaves (paritytech#14067) (paritytech#14069)

* Only calculate tree route when there are multiple leaves

* Update client/service/src/client/client.rs



---------

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OOM Westmint
6 participants