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

Warn on low connectivity. #3408

Merged
merged 3 commits into from
Jul 5, 2021
Merged

Warn on low connectivity. #3408

merged 3 commits into from
Jul 5, 2021

Conversation

eskimor
Copy link
Member

@eskimor eskimor commented Jul 5, 2021

Low connectivity between validators is an issue one should be aware of.

@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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jul 5, 2021
@eskimor eskimor requested a review from ordian July 5, 2021 05:45
/// First time we did not reach our connectivity treshold.
///
/// Will be cleared once we reached it.
first_failure: Option<Instant>,
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the reason for this value. Especially as you reset it below, meaning this isn't always the first failure.

Wouldn't just a start: Instant be enough to check that we are above the threshold and start with the printing of the warning?

Copy link
Member Author

Choose a reason for hiding this comment

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

first failure in a sequence of failures. So the first time we did not connect to enough validators of potentially many attempts. This is most important at startup, but I kept it more generic: Whenever we are not connected to at least 2/3rds of all validators for at least a minute, a warning is printed. I'll clarify the comment.

node/network/gossip-support/src/lib.rs Outdated Show resolved Hide resolved
node/network/gossip-support/src/lib.rs Show resolved Hide resolved
node/network/gossip-support/src/lib.rs Outdated Show resolved Hide resolved
node/network/gossip-support/src/lib.rs Show resolved Hide resolved
@eskimor eskimor merged commit 0236a38 into master Jul 5, 2021
@eskimor eskimor deleted the rk-connectivity-warn branch July 5, 2021 19:22
ordian added a commit that referenced this pull request Jul 6, 2021
* master: (33 commits)
  Update all weights, add run_all_benches.sh script (#3400)
  Enable over-bridge-messaging in Rococo/Wococo runtime (#3377)
  paras.rs to FRAME V2 (#3403)
  Add XCM Tracing (#3353)
  Use MaxEncodedLen trait from new parity-scale-codec v2.2 (#3412)
  bump a bunch of deps in parity-common (#3402)
  Warn on low connectivity. (#3408)
  origin to frame v2 (#3405)
  Enable logging in the puppet worker (#3411)
  make it easier to dbg stalls (#3351)
  XCM `canonicalize` + `prepend_with` fix (#3269)
  cleanup stream polls (#3397)
  Staking Miner (#3141)
  Companion for Substrate#8953 (#3140)
  Bump version, specs & substrate in prep for v0.9.8 (#3387)
  Fix busy loops. (#3392)
  Minor refactor (#3386)
  add simnet tests (#3381)
  BEEFY: adjust gossip (#3372)
  Companion for #9193 (#3376)
  ...
eskimor added a commit that referenced this pull request Aug 18, 2021
* Warn on low connectivity.

* Better timeout and docs.

* Review remarks.
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.

4 participants