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

From: Jeff King <peff@peff.net>
Date: Feb 18, 2013, 09:55 UTC
Message-ID: <20130218095537.GL5096@sigill.intra.peff.net>
In-Reply-To: <7v8v6mar4e.fsf@alter.siamese.dyndns.org>

On Mon, Feb 18, 2013 at 01:49:37AM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> 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.

Yeah, I'd agree. An ERR packet should always be accompanied by a flush (whether an actual flush packet or not doesn't really matter, but definitely a buffer flush on the sender). But I think that is really the sender's problem, and we can deal with it there if and when we buffer.

>From the receiver's end, they simply want to be liberal in interpreting

an ERR as the end of the data stream. They do not have to be picky about whether it is followed by more data, or by a flush packet, or whatever. But that is not a change I am introducing; that is how get_remote_heads has always worked. I am merely correcting the proposed change from v1 of the series that did not correctly take that into account (and therefore regressed the error message in the ERR case).

-Peff