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

Turn the CandidateSet and AddressBook into tower Services #1976

Closed
9 tasks
teor2345 opened this issue Apr 5, 2021 · 1 comment
Closed
9 tasks

Turn the CandidateSet and AddressBook into tower Services #1976

teor2345 opened this issue Apr 5, 2021 · 1 comment
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup C-design Category: Software design work S-needs-design Status: Needs a design decision

Comments

@teor2345
Copy link
Contributor

teor2345 commented Apr 5, 2021

Is your feature request related to a problem? Please describe.

In Zebra, we use tower Services for most of our internal APIs.

The AddressBook and CandidateSet act like services, but they are actually custom structs with their own APIs. This makes Zebra harder to maintain and debug.

Describe the solution you'd like

  • Finish the refactors and security fixes for zebra-network, so the API is stable
  • Write a design RFC for the AddressBook and CandidateSet as tower Services
    • Work out how to test for deadlocks and livelocks
    • Share the AddressBook using a Buffer, rather than Arc<Mutex<_>>
    • Move the CandidateSet::next rate-limit to the outbound handshaker, using a tower::RateLimitLayer
    • Continue to ignore repeated CandidateSet::update requests within the update time limit
    • Continue to mark peers as AttemptPending before their addresses are returned
  • Implement the design
  • Write tests for deadlocks and livelocks

Describe alternatives you've considered

We could do nothing, and Zebra will continue to work well.

We could use a futures-aware mutex for the address book: https://docs.rs/futures/0.3.13/futures/lock/struct.Mutex.html

Additional context

We're still working on a bunch of security fixes that will change the APIs for these services.

This change risks re-introducing deadlocks and livelocks, so it needs to have tests for various edge cases.

@teor2345 teor2345 added C-design Category: Software design work A-rust Area: Updates to Rust code S-needs-design Status: Needs a design decision C-cleanup Category: This is a cleanup S-needs-triage Status: A bug report needs triage P-Optional labels Apr 5, 2021
@teor2345 teor2345 changed the title Turn the CandidateSet and AddressBook into tower Services Turn the CandidateSet into a tower Service Apr 9, 2021
@teor2345 teor2345 changed the title Turn the CandidateSet into a tower Service Turn the CandidateSet and AddressBook into tower Services Apr 9, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Apr 9, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Mar 1, 2022

This refactor would be useful, but it's not needed right now.

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-cleanup Category: This is a cleanup C-design Category: Software design work S-needs-design Status: Needs a design decision
Projects
None yet
Development

No branches or pull requests

2 participants