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

Chain Tip Estimate Test: Log chain progress while Zebra is syncing #3495

Merged
merged 2 commits into from
Feb 12, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Feb 9, 2022

Motivation

  1. This PR helps test if the chain tip estimate is accurate when it's used in zebrad. It will help diagnose problems during full sync tests.

  2. On PR Estimate network chain tip height based on local node time and current best tip #3492, Janito asked:

I'd especially like to know if the solution I've implemented is too generic and complicated, and if I should change it to a simpler one.

Solution

I added a very simple chain tip progress log to Zebra.

This helped me answer the questions above:

  1. I ran Zebra, the chain tip estimate seems to be accurate enough for both the full sync tests and lightwalletd.
  2. The external ChainTip API is very easy to use. (The internals are much less important, because we don't have any plans to use them directly, or change them.)

This PR contains diagnostics for:

It implements part of the zebrad side of:

Review

@jvff can review this PR.

It depends on PR #3492.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

@teor2345 teor2345 added P-Medium ⚡ A-network Area: Network protocol updates or fixes A-diagnostics Area: Diagnosing issues or monitoring performance C-testing Category: These are tests labels Feb 9, 2022
@teor2345 teor2345 requested a review from jvff February 9, 2022 04:58
@teor2345 teor2345 self-assigned this Feb 9, 2022
@teor2345 teor2345 force-pushed the log-chain-tip-estimate branch from 7dce780 to 2e70b4f Compare February 9, 2022 05:23
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #3495 (b5e0670) into main (499ae89) will increase coverage by 0.31%.
The diff coverage is 79.41%.

@@            Coverage Diff             @@
##             main    #3495      +/-   ##
==========================================
+ Coverage   78.34%   78.65%   +0.31%     
==========================================
  Files         267      274       +7     
  Lines       31526    32500     +974     
==========================================
+ Hits        24698    25562     +864     
- Misses       6828     6938     +110     

@conradoplg
Copy link
Collaborator

Haven't reviewed the code but I tested it and already love it

Feb 09 10:55:39.297  INFO {zebrad="2e70b4f" net="Main"}:sync: zebrad::components::sync: extending tips tips.len=1 in_flight=127 lookahead_limit=2000 state_tip=Some(Height(1559584))
Feb 09 10:55:39.843  INFO {zebrad="2e70b4f" net="Main"}:sync: zebrad::components::sync: exhausted prospective tip set
Feb 09 10:55:39.843  INFO {zebrad="2e70b4f" net="Main"}:sync: zebrad::components::sync: waiting to restart sync timeout=67s state_tip=Some(Height(1559589))
Feb 09 10:56:17.237  INFO {zebrad="2e70b4f" net="Main"}:crawl_and_dial: zebra_network::peer_set::inventory_registry: dropped lagged inventory advertisements count=35
Feb 09 10:56:17.299  INFO {zebrad="2e70b4f" net="Main"}: zebrad::commands::start: estimated progress to chain tip sync_percent=99.992 remaining_sync_blocks=117

@teor2345
Copy link
Contributor Author

teor2345 commented Feb 9, 2022

It's even more fun on my local testnet sync, where the estimate each minute jumps by 2% 🎉

@jvff jvff force-pushed the estimate-network-chain-tip-height branch from dbfaf34 to 9a5b634 Compare February 10, 2022 23:26
@teor2345 teor2345 force-pushed the log-chain-tip-estimate branch from 2e70b4f to b213680 Compare February 10, 2022 23:46
jvff
jvff previously approved these changes Feb 11, 2022
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Looks good! This is very useful :D

@jvff jvff changed the base branch from estimate-network-chain-tip-height to main February 12, 2022 22:09
@jvff jvff dismissed their stale review February 12, 2022 22:09

The base branch was changed.

This helps test if the chain tip estimate is accurate,
and helps diagnose problems during full sync tests.
@jvff jvff force-pushed the log-chain-tip-estimate branch from b213680 to b5e0670 Compare February 12, 2022 22:13
@mergify mergify bot merged commit 20ac7b1 into main Feb 12, 2022
@mergify mergify bot deleted the log-chain-tip-estimate branch February 12, 2022 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Diagnosing issues or monitoring performance A-network Area: Network protocol updates or fixes C-testing Category: These are tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants