On Mar 19, 2008, at 9:51 AM, Matthew Toseland wrote: > On Wednesday 19 March 2008 13:32, Michael Rogers wrote: >> On Mar 19 2008, Matthew Toseland wrote: >>> Actually, I'm not sure we're finished here. What we really want to >>> do, to >>> minimise the chance of a timeout, is to alternate between the active >>> transfers. With the code you implemented, if a block is in store, >>> all of >>> its packet transmits will be queued at the same time and therefore >>> all >>> other transfers to the same peer will wait until they have all >>> been sent, >>> which given bandwidth limiting could be a long time. >>> >>> Suggestions? > > I'm wrong. PacketThrottle.sendThrottledMessage blocks, so we don't > need to > worry about this. Robert's fix is correct.
That's good, 'cause I've read that comment several times and I still don't understand it. :-) >> I think you and I discussed this problem WRT the new transport >> layer. One >> possibility is to have an array of queues for each peer - the first >> queue >> is for small messages and each block transfer gets a separate >> queue. You >> keep a round-robin pointer into the array, and each time you build >> a packet >> you start with the queue pointed to by the round-robin, then >> consider the >> other queues. After building the packet you increment the round- >> robin. >> >> (If you want to give small messages priority over transfers then >> don't >> include their queue in the array and always check it before >> checking the >> array.) > > Hmmm. I think that needs some modification: > > For the current code, we don't want to round robin between block > transfers and > small messages. We want to put in one block transfer packet, and as > many > small messages as will fit within a reasonable message size. Or we > want to > just use the latter. We do already round robin in that we send the > throttled > packets in submit-time order, and each stream only has one packet in > flight > at once (if bandwidth is limiting) because sendThrottledPacket is > blocking. Right, but sendThrottledPacket puts a packet on the send queue, not actually prompting an immediate sending of a packet; probably little effective difference if a packet can only hold one block/bulk packet, but your wording makes it sound the other way around. > For the proposed streams changes, we want to handle this > differently: each > peer will have a queue of small messages, and a list of streams. > When we > build a packet, we put in small messages first, then we use one or > more of > the streams to fill it up to the desired total packet size (using a > few bytes > of random data at the end if we need to). We would alternate between > the > streams in order to ensure that none starved. It sounds good. The transport layer then has effectively two types. Messages and streams. One for latency and one for throughput. Presumably this would fundamentally change how block transfers are implemented. e.g. transfers[i].getNextPacket().... in fact... it seems like it's just a prb with a completion callback. -- Robert Hailey > > The proposed streams changes can be implemented without the New > Transport > Layer, but should be kept when it comes in. >> >> Cheers, >> Michael
