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

2. Cache unsolicited address messages, and use them as responses #3294

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Dec 24, 2021

Motivation

Zebra's address crawler is fragile - small changes can cause significant regressions.

This PR caches unsolicited address messages, so that new addresses are always available when Zebra wants them.

Solution

  • Cache unsolicited address messages
  • Prefer multi-address messages to single-address messages
  • Provide cached addresses to Zebra in response to address requests

Closes #3110

Review

This PR is ready for review.

@dconnolly can review this PR.

Reviewer Checklist

  • Code implements Specs and Designs

I've tested this PR locally, and the regression in the address book metrics is resolved. (We see thousands of addresses on mainnet, and over a hundred on testnet.) 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.

It's hard to test this PR, because the timing issues are subtle, and don't show up in some network environments. We should have better tests after we implement #1592.

@teor2345 teor2345 added P-Medium C-security Category: Security issues I-hang A Zebra component stops responding to requests I-slow Problems with performance or responsiveness I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data A-network Area: Network protocol updates or fixes labels Dec 24, 2021
@teor2345 teor2345 self-assigned this Dec 24, 2021
@teor2345 teor2345 marked this pull request as draft December 24, 2021 02:35
@codecov
Copy link

codecov bot commented Dec 24, 2021

Codecov Report

Merging #3294 (1153aa2) into main (469fa6b) will increase coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #3294      +/-   ##
==========================================
+ Coverage   77.19%   77.25%   +0.05%     
==========================================
  Files         265      265              
  Lines       31445    31460      +15     
==========================================
+ Hits        24275    24303      +28     
+ Misses       7170     7157      -13     

@teor2345
Copy link
Contributor Author

Bumping this up to high priority, because this fixes some occasional CI failures, and it might conflict with other PRs.

@teor2345 teor2345 added the I-integration-fail Continuous integration fails, including build and test failures label Dec 28, 2021
@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-crawler-timing branch from efd6bb0 to fa44edf Compare December 30, 2021 03:30
@teor2345 teor2345 force-pushed the cache-unsolicited-addresses branch from 09c749b to c28fd01 Compare December 30, 2021 03:31
Base automatically changed from fix-address-crawler-timing to main January 4, 2022 23:43
@teor2345 teor2345 marked this pull request as ready for review January 4, 2022 23:54
@teor2345 teor2345 force-pushed the cache-unsolicited-addresses branch from c28fd01 to 1153aa2 Compare January 4, 2022 23:56
@teor2345 teor2345 requested a review from dconnolly January 4, 2022 23:57
@teor2345 teor2345 enabled auto-merge (squash) January 4, 2022 23:59
@teor2345 teor2345 merged commit 144c532 into main Jan 5, 2022
@teor2345 teor2345 deleted the cache-unsolicited-addresses branch January 5, 2022 20:56
@dconnolly
Copy link
Contributor

A pretty tight narrow addr cache 👍

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-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-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data I-slow Problems with performance or responsiveness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache unsolicited Peers responses until needed
2 participants