Subject: Re: [PATCH 3/5] git push: verify refs early

From: Clemens Buchacher <[email-masked]>
Date: Tue, 14 Feb 2012 09:59:19 +0100

On Mon, Feb 13, 2012 at 02:16:13PM -0800, Junio C Hamano wrote:
> Clemens Buchacher <[email-masked]> writes:
> 
> > I suppose with some effort, this could be done for smart HTTP as well.
> > But I am not sure if we actually want the overhead of the additional
> > ping-pong for HTTP.
> 
> Hrm, I am confused.
> 
> The updated protocol exchange, if I am reading your patch correctly, would
> go like this (S stands for the sender, R for the receiver):
> 
>         R: Here are the tips of my refs
>         ----
>         S: I'd like to update your refs this way
>         ----
>       + R: No you cannot because all updates will fail, go away
>         or
>       + R: You may proceed, as some updates may succeed
>         ----
>         S: Here is the packfile
>         ----
>         R: Here is how I processed your request
> 
> Given that this makes the sender stall for both smart HTTP and native
> protocol, don't your worries about the additional ping-pong apply equally
> to both transports?

That is true. However, my assumption was that the overhead is greater
for HTTP, because the native protocol is full-duplex, while HTTP tears
down the connection and starts from scratch with each request. But to be
honest, I am not confident that this assumption is correct.

So, the stall might be an issue for both the native and the HTTP
protocol, or for neither. We should probably find out and then decide
whether to make this change for both protocols or not at all.

> If it is not worth doing for smart HTTP, I wonder if it is worth doing for
> native transport.  After all, "all updates will fail" is hopefully the
> less likely case, and with this protocol extension, we end up penalizing
> the common case with an extra stall for everybody, regardless of the
> transport.

Indeed. I wish we could make the ref validation asynchronous. The client
would start sending object data right away, while listening for an
"abort" command on the side-band. But if I understand correctly, that is
not possible for HTTP.