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

Stop ignoring some peers when updating the address book #3292

Merged
merged 6 commits into from
Jan 5, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Dec 24, 2021

Motivation

Zebra was ignoring some peers when updating the address book.

This could be the cause of issues like #3042.

Solution

  • Correctly apply MetaAddrChanges when there is no existing address book entry
  • Add all initial peers to the address book (not just the unused ones)

Review

This PR is ready for review.

I think @dconnolly can review this PR.

Reviewer Checklist

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

I've tested this PR locally, and the crawler and address book metrics look a lot better. The number of peer addresses rises rapidly, and gets full after a few hours. And Zebra doesn't hang any more, even after a few days.

@teor2345 teor2345 added C-bug Category: This is a bug P-Medium C-security Category: Security issues I-hang A Zebra component stops responding to requests I-slow Problems with performance or responsiveness A-network Area: Network protocol updates or fixes labels Dec 24, 2021
@teor2345 teor2345 self-assigned this Dec 24, 2021
@codecov
Copy link

codecov bot commented Dec 24, 2021

Codecov Report

Merging #3292 (bf569a6) into main (144c532) will increase coverage by 0.11%.
The diff coverage is 95.45%.

@@            Coverage Diff             @@
##             main    #3292      +/-   ##
==========================================
+ Coverage   77.10%   77.21%   +0.11%     
==========================================
  Files         265      265              
  Lines       31460    31462       +2     
==========================================
+ Hits        24256    24293      +37     
+ Misses       7204     7169      -35     

@teor2345
Copy link
Contributor Author

I've tested this PR locally, and the crawler and address book metrics look a lot better. The number of peer addresses rises rapidly, and gets full after a few hours. And Zebra doesn't hang any more, even after a few days.

@teor2345 teor2345 force-pushed the fix-address-book-changes branch from e997b3f to 28c8a58 Compare December 30, 2021 03:21
@teor2345 teor2345 enabled auto-merge (squash) January 3, 2022 07:57
@teor2345 teor2345 added the I-integration-fail Continuous integration fails, including build and test failures label Jan 3, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Jan 3, 2022

Marking as a high priority, because this bug sometimes makes CI fail.

Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

lgtm!

@teor2345 teor2345 merged commit 7e63182 into main Jan 5, 2022
@teor2345 teor2345 deleted the fix-address-book-changes branch January 5, 2022 23:13
@mpguerra mpguerra mentioned this pull request Jan 27, 2022
49 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network Area: Network protocol updates or fixes C-bug Category: This is a bug C-security Category: Security issues I-hang A Zebra component stops responding to requests I-integration-fail Continuous integration fails, including build and test failures I-slow Problems with performance or responsiveness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants