neutrino: Added ResetRanking method to PeerRanking. #288
Conversation
|
@Crypt-iQ, this is the PR. |
| @@ -67,6 +67,8 @@ type PeerRanking interface { | |||
|
|
|||
| // Order sorst the slice of peers according to their ranking. | |||
| Order(peers []string) | |||
|
|
|||
| ResetRanking(peerAddr string) | |||
| @@ -377,6 +379,11 @@ Loop: | |||
| result.job.timeout = newTimeout | |||
| } | |||
|
|
|||
| // Refresh peer rank on disconnect. | |||
| if result.err == ErrPeerDisconnected { | |||
| w.cfg.Ranking.ResetRanking(result.peer.Addr()) | |||
There was a problem hiding this comment.
Line should fold to 80 char columns.
There was a problem hiding this comment.
Please I checked this in my IDE and it is less than 80 char column
There was a problem hiding this comment.
@Chinwendu20 did you configure your IDE to use 8 spaces for a tab character? If not, this will probably pop up in future PRs as well (in lnd the line length is even enforced by a linter).
There was a problem hiding this comment.
Thanks for pointing this out, this was one of my earlier PRs. I have now configured my IDE to this. Does it still appear as going beyond the limit?
There was a problem hiding this comment.
This particular line as it was merged, yes. But no big deal, just wanted to let you know for future PRs.
|
Thank you guys but I am.not sure if this PR is still valid. Since there is no need to reset ranking? |
What makes you think that? IIUC, the original intent was the a peer's ranking isn't necessarily the same between reconnects (changes in network, implementation, hardware, etc, etc). I'd consider the fact that they were never reset to be a bug. |
Oh okay, this comment: #280 (comment), I was just wondering since work would still be distributed, there would be no need for fetch from that from that particular peer which disconnected. But I guess the current setup makes for a bug in itself, thanks for pointing it out. |
Signed-off-by: Maureen Ononiwu <[email protected]>
|
@Chinwendu20, remember to re-request review from reviewers when ready |
This PR addresses #280
Fixes #280