Feat: Track Session Peer Latency More Accurately #149
Conversation
Return optimized peers in real latency order, weighted toward recent requests
When fetching optimized peers from the peer manager, return an optimization rating, and pass on to request splitter BREAKING CHANGE: interface change to GetOptimizedPeers and SplitRequests public package methods
Better estimate latency per peer by tracking cancellations
send duplicate responses to the session peer manager to track latencies
|
Note marked improvement on time for some benchmarks: |
There was a problem hiding this comment.
Overall, this looks awesome! My main comments are:
- Let's document the interfaces (when certain functions should be called).
- Can we write a benchmark that issues many parallel requests for unavailable content? This adds some complicated per-CID logic so I'm a bit worried about issues like #154.
| // OptimizedPeer describes a peer and its level of optimization from 0 to 1. | ||
| type OptimizedPeer struct { | ||
| Peer peer.ID | ||
| OptimizationRating float64 |
There was a problem hiding this comment.
Can we comment on what this rating means?
| request, ok := lt.requests[key] | ||
| var latency time.Duration | ||
| if ok { | ||
| latency = time.Now().Sub(request.startedAt) |
There was a problem hiding this comment.
nit: time.Since(request.startedAt)
|
|
||
| func (ptm *peerTimeoutMessage) handle(spm *SessionPeerManager) { | ||
| data, ok := spm.activePeers[ptm.p] | ||
| if !ok || !data.lt.WasCancelled(ptm.k) { |
There was a problem hiding this comment.
Should this be ok && !data.lt.....? That is, do we want to record timeouts for inactive peers?
There was a problem hiding this comment.
(that is, won't this add these peers to the active set?)
|
IMO, the constants are fine. Ideally, we'd some how "learn" them but I can't think of a simple way to do so. |
This ^^ really shows that it's working. That's exactly what I'd expect from latency tracking. This is going to be a really nice boost. ❤️ |
From my experience with digital signal processing and networking systems, the alpha parameter of 0.5 in exponential moving average seems high. Could you set up logging (probably one separate logger) with raw data so we can try tweaking these parameters? As an example, one lost packet over TCP will incur 2-3 RTTs of jitter over normal latency. From my simulations in matlab, with network connection defined by Rayleigh distribution, packet drop probability of 0.5% and EMA for latency tracking, 0.5 seems too high. Take a look: Especially that we track only blocks transferred and not latency in general. Matlab script for those interested: https://gist.github.com/Kubuxu/5c58022d7af6b1f3dfb66f0eae5a730c |
|
I'm going to merge this as strictly better than what we have. Given our new release process, I'm confident that we'll catch any regressions (if any) before they hit users. |
…cking Feat: Track Session Peer Latency More Accurately This commit was moved from ipfs/go-bitswap@8f0e4c6
Goals
Track the speeds between session peers more accurately
Implementation
The current SessionPeerManager uses a very simple algorithm for sorting peers by optimization -- it simply orders them by last block received.
This algorithm attempts to actually track peers length of time to respond to requests, and sort them not only by which is fastest, by provide useful information (and optimization rating from 1-0, with 1 being the fastest peer) about how they relate to each other.
The steps are as follows:
For Discussion