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

feat(log): Show the current network upgrade in progress logs #4694

Merged
merged 7 commits into from
Jun 28, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jun 27, 2022

Motivation

It takes almost 6 hours for Zebra to sync all its checkpoints, so we need to be able to split CI jobs by network upgrade as well as checkpoint/full validation.

This is part of ticket #4661, it also includes a refactor I did as part of loading blocks from a file.

Solution

Features:

  • Show the network upgrade in progress logs
  • Log when Zebra verifies the final checkpoint

Fixes:

  • Log elapsed time using humantime truncated to whole seconds, rather than raw seconds and microseconds
  • Fix incorrect negative sign on elapsed time
  • Remove the space before the percentage sign when showing percent progress

Refactors:

  • Move the progress task into its own module

Review

This is a high priority because I need it for ticket #4661.

Reviewer Checklist

  • Progress logs contain network upgrade
  • Time is formatted nicely
  • Existing tests pass

@teor2345 teor2345 added C-enhancement Category: This is an improvement C-cleanup Category: This is a cleanup P-High 🔥 A-diagnostics Area: Diagnosing issues or monitoring performance labels Jun 27, 2022
@teor2345 teor2345 requested review from a team as code owners June 27, 2022 07:34
@teor2345 teor2345 self-assigned this Jun 27, 2022
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team June 27, 2022 07:34
@codecov
Copy link

codecov bot commented Jun 27, 2022

Codecov Report

Merging #4694 (09b04f9) into main (d37d8aa) will decrease coverage by 0.02%.
The diff coverage is 1.07%.

@@            Coverage Diff             @@
##             main    #4694      +/-   ##
==========================================
- Coverage   78.93%   78.90%   -0.03%     
==========================================
  Files         304      306       +2     
  Lines       37506    37496      -10     
==========================================
- Hits        29604    29585      -19     
- Misses       7902     7911       +9     

@teor2345 teor2345 requested review from upbqdn and removed request for a team and oxarbitrage June 27, 2022 21:26
Copy link
Member

@upbqdn upbqdn left a comment

Choose a reason for hiding this comment

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

All looks great.

By reviewing this PR, I learned to compare only parts of different files against each other so that I can see what's changed, and what's just moved.

@teor2345
Copy link
Contributor Author

All looks great.

By reviewing this PR, I learned to compare only parts of different files against each other so that I can see what's changed, and what's just moved.

Yeah I find git diff --color-moved really helpful, I wish GitHub would do it automatically.

mergify bot added a commit that referenced this pull request Jun 28, 2022
@mergify mergify bot merged commit d4b9353 into main Jun 28, 2022
@mergify mergify bot deleted the progress-cleanup branch June 28, 2022 02:51
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 C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants