Re: [PATCHv2 0/10] pkt-line and remote-curl cleanups server

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Mon, Feb 18, 2013 at 01:29:16AM -0800, Junio C Hamano wrote:
>
>> > I just checked, and GitHub also does not send flush packets after ERR.
>> > Which makes sense; ERR is supposed to end the conversation.
>> 
>> Hmph.  A flush packet was supposed to be a mark to say "all the
>> packets before this one can be buffered and kept without getting
>> passed to write(2), but this and all such buffered data _must_ go on
>> the wire _now_".  So in the sense, ERR not followed by a flush may
>> not even have a chance to be seen on the other end, no?  That is
>> what the comment before the implementation of packet_flush() is all
>> about.
>
> Despite the name, I always thought of packet_flush as more of a signal
> for the _receiver_, who then knows that they have gotten all of a
> particular list. In other words, we seem to use it as a sequence point
> as much as anything (mostly because we immediately write() out any other
> packets anyway, so there is no flushing to be done; but perhaps there
> were originally thoughts that we could do more buffering on the writing
> side).

Exactly.

The logic behind the comment "we do not buffer, but we should" is
that flush tells the receiver that the sending end is "done" feeding
it a class of data, and the data before flush do not have to reach
the receiver immediately, hence we can afford to buffer on the
sending end if we can measure that doing so would give us better
performance. We haven't measure and turned buffering on yet.

But when dying, we may of course have data before flushing. We may
disconnect (by dying) without showing flush (or preceding ERR) to
the other side, and for that reason, not relying on the flush packet
on the receiving end is a good change. Once we turn buffering on, we
probably need to be careful when sending an ERR indicator by making
it always drain any buffered data and show the ERR indicator without
buffering, or something.