Page MenuHomePhabricator

[avalanche] Protect from eviction the best node for each peer
Needs RevisionPublic

Authored by Fabien on Aug 9 2021, 14:58.


Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task

For each avalanche peer, protect the node with the best availability score from eviction.
This occurs before we protect the 16 best nodes (independently of the peer).

Note that there is no filter on the avalanche peers to consider in this diff. A threshold on the proof score can be added later but is outside the scope of this diff.

Depends on D9853 and D9857.

Ref T1855.

Test Plan
ninja all check-all

Diff Detail

rABC Bitcoin ABC
Lint Passed
No Test Coverage
Build Status
Buildable 16311
Build 32487: Build Difflint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 32486: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Aug 9 2021, 14:58
deadalnix requested changes to this revision.Aug 10 2021, 12:42
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
967 ↗(On Diff #29369)

The datastructure you are building here already exists in the PeerManager, no? It seems like this is calling for refactoring to leverage this.

1143 ↗(On Diff #29369)

These lambdas do not outlive the call, so using a generic & is fine, IMO.

This revision now requires changes to proceed.Aug 10 2021, 12:42
967 ↗(On Diff #29369)

There is no such structure, the peerid is managed by the avalanche::PeerManager and the score is part of the AvalancheState of each CNode.

Rebase, use [&] capture in lamda, don't protect nodes with score < 0.

Don't protect nodes with score of = 0, only > 0


And this is why you use trailing comas :)


yes I missed it in D9857...

This comment was removed by deadalnix.
deadalnix requested changes to this revision.Aug 14 2021, 01:08

I don't really like the design of this.

There is a lot of bookkeeping that is being done and datastructures suited for this kinda exist int he peer manager. Isn't it possible to run a similar algorithm in the peer manager and tag one node per peer there? There need to be a routine in there to cleanup outdated nodes anyways, so that seems appropriate to do it as part of that maintenance process.


This is int he right order.

This revision now requires changes to proceed.Aug 14 2021, 01:08
Mengerian edited tasks, added Restricted Maniphest Task; removed Restricted Maniphest Task.Sep 14 2021, 18:24
Mengerian edited the summary of this revision. (Show Details)