[infinispan-dev] DIST.retrieveFromRemoteSource

[Dan Berindei]

On Sun, Feb 5, 2012 at 12:24 PM, Bela Ban <bban at redhat.com> wrote:
>
>
> On 2/5/12 9:44 AM, Dan Berindei wrote:
>> On Sat, Feb 4, 2012 at 5:23 PM, Bela Ban<bban at redhat.com>  wrote:
>>> No, Socket.send() is not a hotspot in the Java code.
>>>
>>> On the networking level, with UDP datagrams, a packet is placed into a
>>> buffer and send() returns after the packet has been placed into the
>>> buffer successfully. Some network stacks simply discard the packet if
>>> the buffer is full, I assume.
>>>
>>> With TCP, send() blocks until the packet has been placed into the buffer
>>> and an ack has been received. If the receiver can't keep up with the
>>> sending rate, it'll reduce the window to 0, so send() blocks until the
>>> receiver catches up.
>>>
>>> So if you send 2 requests in paralle over TCP, both threads would
>>> block() on the send.
>>>
>>> So I don't think parallelization would make sense here, unless of course
>>> you're doing something else, such as serialization, lock acquisition etc...
>>>
>>
>> You're probably right...
>>
>> When I sent the email I had only seen that DataGramSocket.send takes
>> 0.4% of the get worker's total time (which is huge considering that
>> 99.3% is spent waiting for the remote node's response). I didn't
>> notice that it's synchronized and the actual sending only takes half
>> of that - sending the messages in parallel would create more lock
>> contention on the socket *and* in JGroups.
>>
>> On the other hand, we're sending the messages to two different nodes,
>> on two different sockets, so sending in parallel *may* improve the
>> response time in scenarios with few worker threads. Certainly not
>> worth making our heavy-load performance worse, but I thought it was
>> worth mentioning.
>
>
> If you think this makes a difference, why don't you make a temporary
> code change and measure its effect on performance ?
>

I will give it a try, it's just that writing emails is a bit easier ;-)

>
>> Anyway, my primary motivation for this question was that I believe we
>> could use GroupRequest instead of our FutureCollator to send our
>> commands in parallel. They both do essentially the same thing, except
>> FutureCollator has and extra indirection layer because it uses
>> UnicastRequest.
>
>
> Ah, ok. Yes, I think that 1 GroupRequest of 2 might be more efficient
> than 2 UnicastRequests for sending an 'anycast' message to 2 members.
> Perhaps the marshalling is done only once (I'd have to confirm that
> though) and we're only creating 1 data structure and add it to a hashmap
> (request/response correlation)... Certainly worth giving a try...
>

We are only serializing once with the FutureCollator approach, and we
don't copy the buffer either.

>
>>  If there is any performance discrepancy at the moment
>> between GroupRequest and FutureCollator+UnicastRequest, I don't think
>> there is any fundamental reason why we can't "fix" GroupRequest to be
>> just as efficient as FutureCollator+UnicastRequest.
>
>
> You're assuming that FutureCollator+UnicastRequest is faster than
> GroupRequest, what are you basing your assumption on ? As I outlined
> above, I'd rather assume the opposite, although I don't know FutureCollator.
>

I did say *if*...

> Maybe we should have a chat next week to go through the code that sends
> an anycast to 2 members, wdyt ?
>

Sounds good, we should also also talk about how to implement staggered
get requests best.

>
>> I think I just saw one such fix: in RequestCorrelator.sendRequest, if
>> anycasting is enabled then it's making a copy of the buffer for each
>> target member. I don't think that is necessary at all, in fact I think
>> it should reuse both the buffer and the headers and only change the
>> destination address.
>
> No, this *is* necessary, I don't just make copies because I think this
> is fun !! :-)
>
> Remember that a message might be retransmitted, so it is placed into a
> retransmit buffer. If M1 has destination A and M2 has destination B, and
> we send M1 first (to A), then change M1's destination to B, and send it,
> everything is fine. However, if we later get a retransmit request from
> B, we'd resend the message to A instead ! This is just 1 example,
> modifications of headers is another one.
>
> Note that the copy does *not* copy the buffer (payload) itself, but only
> references it, so this is fast. Of course, nobody is supposed to modify
> the contents of the buffer itself...
>

I wasn't clear enough, but I didn't mean we should reuse the entire
Message object. I meant we should copy the Message but not the buffer
or the headers. I see now that protocols may be adding new headers, so
it wouldn't be safe to reuse the headers collection.

I think this line in
RequestCorrelator.sendRequest(RequestCorrelator.java:152) means that
the contents of the buffer is copied in the new message, not just the
buffer reference:

                Message copy=msg.copy(true);


Cheers
Dan