Update AddressBook field types with concurrent collections

[arya2]

Motivation

This may reduce the likelihood of contention when the inbound service is responding to getpeers requests.

Possible Design

• Replace ordered_map with a SkipMap from crossbeam-skiplist.
• Replace most_recent_by_ip HashMap with a DashMap.
• Use an immutable reference in update() method and take AddressBook out of the Mutex.

[teor2345]

by_addr and most_recent_by_ip have to stay consistent with each other. Currently we enforce that constraint by only allowing one thread to modify the address book at the same time.

I think this ticket needs a design that will ensure consistency during concurrent modification. This is a tricky problem to solve, which requires specialised testing. So in general we've used existing concurrency primitives in Zebra, rather than implementing our own.

So I'd suggest opening a ticket for the goal, and any evidence that will help us prioritise it. Then we can add this change as one of the alternatives.

For example, some alternatives are:

• replacing the Mutex with a RwLock
• replacing the thread-based concurrency primitives with async concurrency primitives, and changing all the tasks that use the address book into async tasks
• Turn the CandidateSet and AddressBook into tower Services #1976
• do nothing, because this issue does not have a significant impact

[arya2]

So I'd suggest opening a ticket for the goal, and any evidence that will help us prioritise it.

It would solve an issue that's introduced in the current form of #7041.

I'll take a different approach in that PR, and re-open this if I see an indication in tokio-console that it's blocking the async executor.

[teor2345]

So I'd suggest opening a ticket for the goal, and any evidence that will help us prioritise it.

It would solve an issue that's introduced in the current form of #7041.

Thanks, I think that's a useful motivation, feel free to re-open this ticket if it's needed.