Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log address book metrics when PeerSet or CandidateSet get in a bad state #1906

Merged
merged 3 commits into from
Mar 17, 2021

Conversation

teor2345
Copy link
Contributor

Motivation

To diagnose issues like #1905, we need to know the state of the addresses in the address book.

While we're fixing issues like #1848, and other network security issues, we need good diagnostics, so we can:

  • fix bugs
  • monitor performance and peer set impacts
  • tune any security parameters that have too much of an impact

Solution

  • log address metrics every time an address changes state, at trace level
  • if there are no Responded addresses, log address metrics, at most once per minute, at info level
  • if all addresses have Failed, log address metrics, at most once per minute, at warn level
  • log address metrics in the existing peer set info and warn level logs

The code in this pull request:

  • Has Documentation Comments
  • Is Covered By Existing Acceptance Tests

Review

These diagnostics will be useful for fixing security issues, but they aren't blocking them.

Follow Up Work

Fix any bugs revealed by these logs.

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium I-integration-fail Continuous integration fails, including build and test failures I-usability Zebra is hard to understand or use labels Mar 15, 2021
@teor2345 teor2345 added this to the 2021 Sprint 5 milestone Mar 15, 2021
@teor2345 teor2345 requested a review from a team March 15, 2021 12:40
@teor2345 teor2345 self-assigned this Mar 15, 2021
@teor2345 teor2345 changed the title Log address book metrics Log address book metrics when PeerSet or CandidateSet get in a bad state Mar 17, 2021
@teor2345 teor2345 merged commit 4f923b9 into ZcashFoundation:main Mar 17, 2021
@dconnolly dconnolly mentioned this pull request Mar 23, 2021
23 tasks
dconnolly added a commit that referenced this pull request Mar 23, 2021
Zebra's latest alpha checkpoints on Canopy activation, continues our work on NU5, and fixes a security issue.

Some notable changes include:

## Added
- Log address book metrics when PeerSet or CandidateSet don't have many peers (#1906)
- Document test coverage workflow (#1919)
- Add a final job to CI, so we can easily require all the CI jobs to pass (#1927)

## Changed
- Zebra has moved its mandatory checkpoint from Sapling to Canopy (#1898, #1926)
  - This is a breaking change for users that depend on the exact height of the mandatory checkpoint.

## Fixed
- tower-batch: wake waiting workers on close to avoid hangs (#1908)
- Assert that pre-Canopy blocks use checkpointing (#1909)
- Fix CI disk space usage by disabling incremental compilation in coverage builds (#1923)

## Security
- Stop relying on unchecked length fields when preallocating vectors (#1925)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement I-integration-fail Continuous integration fails, including build and test failures I-usability Zebra is hard to understand or use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants