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

Don't err on deactivated leaf during valiation. #3708

Merged
merged 2 commits into from
Aug 25, 2021
Merged

Conversation

eskimor
Copy link
Member

@eskimor eskimor commented Aug 24, 2021

This seems to be an expected event and should not trigger an error.

@eskimor eskimor added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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. labels Aug 24, 2021
"Mpsc background validation mpsc died during validation- leaf no longer active?"
);
} else {
tracing::debug!(
Copy link
Member

Choose a reason for hiding this comment

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

any reason to downgrade this to debug?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep: https://matrix.to/#/!JZkrzbabXFGsaweNTt:matrix.parity.io/$R_p9_Cna8w4I1335zW0VX6jO8OBhOScg2mHhNdskkTo?via=matrix.parity.io&via=web3.foundation

I think that the closed mpsc is really an expected event, in case the leaf gets deactivated before the validation was able to finish. Which I guess can easily happen in practice (network delays and such) and should not trigger an error and alerting people. That's at least my understanding, if I am wrong - let's fix the bug instead 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it should not happen too often though ... if we want to keep an eye on that, a metric and alerts when it happens too often, in contrast to a higher logging level, would probably be more effective?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh - I just realized what you meant - no that still should be error, that was a copy paste error - thank you!

@ordian ordian added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. label Aug 24, 2021
@eskimor eskimor merged commit 1e6cddb into master Aug 25, 2021
@eskimor eskimor deleted the rk-error-debug branch August 25, 2021 07:25
ordian added a commit that referenced this pull request Aug 27, 2021
* master:
  staking-miner: remove need of a file to pass the seed (#3680)
  Companion for 9619 (Private Events) (#3712)
  Fix Try-Runtime  (#3725)
  XCM v2: Scripting, Query responses, Exception handling and Error reporting (#3629)
  Bump async-trait from 0.1.50 to 0.1.51 (#3721)
  allow some overhead in MERKLE_NODE_MAX_SIZE (#3724)
  Bump itertools from 0.10.0 to 0.10.1 (#3719)
  Bump tokio from 1.10.0 to 1.10.1 (#3717)
  Bump trybuild from 1.0.43 to 1.0.45 (#3713)
  Don't err on deactivated leaf during valiation. (#3708)
  Bump libc from 0.2.99 to 0.2.100 (#3703)
chevdor pushed a commit that referenced this pull request Sep 23, 2021
* Don't err on deactivated leaf during valiation.

* Fix error level + formatting of unrelated code.
@chevdor chevdor mentioned this pull request Sep 23, 2021
ordian pushed a commit that referenced this pull request Sep 24, 2021
* Don't err on deactivated leaf during valiation. (#3708)

* Don't err on deactivated leaf during valiation.

* Fix error level + formatting of unrelated code.

* Substrate update

* Update node/core/backing/src/lib.rs

Co-authored-by: David <dvdplm@gmail.com>

Co-authored-by: Robert Klotzner <eskimor@users.noreply.github.com>
Co-authored-by: David <dvdplm@gmail.com>
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. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants