refresh peer's rank on reconnect

[Crypt-iQ]

In peer_rank.go we never refresh a peer's rank if we reconnect. It will use the old ranking we had for it. If we try to query a tor node and penalize it due to high latency, we should allow reconnecting to refresh the rank:

neutrino/query/peer_rank.go

Lines 57 to 61 in cb61d7f

	func (p *peerRanking) AddPeer(peer string) {
	if _, ok := p.rank[peer]; ok {
	return
	}
	p.rank[peer] = defaultScore

[Chinwendu20]

I guess we can delete the peer address from.the map after it has been disconnected but disconnecting leads to punishment in the current design.

[Crypt-iQ]

I think it should just get refreshed on a reconnect, some people had problems with connecting to tor since the old ranking was used

[Chinwendu20]

Old Ranking? Was there a different ranking before this?
Just checked this PR: #179
and it seems this current ranking was introduced same time as the work dispatcher.

[ellemouton]

@Chinwendu20 - he means the rank value

[Chinwendu20]

Thanks @ellemouton, we can fix this by deleting the peer from the ranking map when it disconnects, changing the current design. I can work on this but I do not know if the issue needs further triaging.

[Crypt-iQ]

That would work, but I think just resetting the value to defaultScore in AddPeer is a smaller patch. Maybe not as correct, but still should work

[Chinwendu20]

Yeah that sounds great.

[Chinwendu20]

Please I have a question about this, why is it important to fetch from this particular peer. If a peer has a bad score and other peers are present they can take on that task otherwise, the workmanager would still schedule the task to the bad scored peer if that peer is the only one on the list or all peers present have same score.

[Crypt-iQ]

Hmm, I don't remember anymore. @Roasbeef do you think this should be addressed given #280 (comment)

[Roasbeef]

. If a peer has a bad score and other peers are present they can take on that task otherwise, the workmanager would still schedule the task to the bad scored peer if that peer is the only one on the list or all peers present have same score.

Yeah @Chinwendu20 is right here imo, as is we still uniformly schedule tasks to all peers when we have a batch. If it's a series of one-off requests, then the highest ranking node will get the task. However if we have a batch of items, due to the way the initial allocation logic is setup, the load will be distributed to all peers.

Making a more specific issue with an implementation plan for improving the state of the art here.