Re: [RFC] multi_ack protocol v2

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 27, 2005, 09:37 UTC
Message-ID: <Pine.LNX.4.63.0510271107570.5970@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vzmov4elu.fsf@assigned-by-dhcp.cox.net>

Hi,

On Thu, 27 Oct 2005, Junio C Hamano wrote:

Show 9 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > after thinking about my earlier approach, I think there's a better, less 
> > intrusive, and all in all just simpler approach:
> >
> > - client asks for multi_ack protocol by sending " multi_ack" at the end
> >   of at least one "want" line
> > - server appends " continue" to the ACK message, but continues sending
> >   ACK (but not NAK) messages

Here must be a correction: server continues to send NAK messages. Else we must introduce some select() or poll() crud, or the client hangs forever.

> > - after receiving "done", server repeats last ACK message without 
> >   " continue" appended

In effect, an ACK message with "continue" switches into v2, which just means: "I'll keep sendin'", and an ACK *without* "continue" switches back to v1.

This way the patch looks almost trivial. (Will send soon).

> > After much fun with non-working, fragile code which had to be retracted 
> > from master, I hope that this approach is less prone to errors.
> 
> Sorry for a late reply -- real life interrupts ;-).

No problem (hope you enjoyed it ;-). I tried to get some Zs anyway.

Show 9 quoted lines

> Sending "have you found a common yet" every 32 "have" like the
> original protocol does has a nice batching property (I think we
> could even tweak pkt-line.c::packet_write() to actually buffer
> these "have"s and write them out in one-go, to put them in a
> single network packet) and I do not want to lose it, but other
> than that, since you have found a good way to extend the initial
> handshake to find out if both ends can do a v2 protocol without
> breaking older server nor clients, I have no objection against
> doing things drastically differently in the v2 protocol.

I think that we don't need to extend packet_write(). Let's leave it like that.

Show 24 quoted lines

> upload-pack (S) | fetch/clone-pack (C) protocol (v1):
> 
> 	# Tell the puller what commits we have and what their names are
> 	S: SHA1 name
> 	S: ...
> 	S: SHA1 name
> 	S: # flush -- it's your turn
> 	# Tell the pusher what commits we want, and what we have
> 	C: want SHA1
> 	C: ..
> 	C: want SHA1
> 	C: # flush -- done with "want" lines.
> 	C: have SHA1
> 	C: have SHA1
> 	C: ...
> 	C: # flush -- this occasionally asks "had enough?"
> 	S: NAK
>         # and the server answers "notyet"
> 	C: have SHA1
> 	C: ...
> 	C: have SHA1
> 	S: ACK SHA1
> 	C: done
> 	S: XXXXXXX -- packfile contents.

alternatively, when no "ACK" was sent, the response to "done" is "NAK".

> I wrote as if "want" sends names, but it actually sends SHA1s.

You also ask if it is sensible in upload-pack.c to accept names. I think not. SHA1s are supposed to be unique, names not (master~10 is likely to denote different commits on server and client).

> Also I missed a place where "flush" was needed.

Between "want" and "have". It is sent so that the server can handle them in separate functions.

Show 28 quoted lines

> upload-pack (S) | fetch/clone-pack (C) protocol (v2):
> 
> 	# Tell the puller what commits we have and what their names are
> 	S: SHA1 name
> 	S: ...
> 	S: SHA1 name
> 	S: # flush -- it's your turn
> 	# Tell the pusher what commits we want, and what we have.
>         # In addition, we tell the other end that we support protocol
> 	# extensions, without breaking the old servers.
> 	C: want SHA1 extended
> 	C: ..
> 	C: want SHA1
> 	C: # flush -- done with "want" lines.
> 
> Notice that until we hear from the server, we cannot tell if our
> "extended" protocol wish will be granted, and in the original
> protocol, "NAK" will come in fixed length, and the only thing we
> could tack arbitrary garbage to was "ACK SHA1".  That's why your
> "ACK SHA1 continue" works nicely, but at the time you could not
> find out if you are talking with updated server until you get at
> least one ACK.
> 
> However, at this point, we *could* force the server to reveal
> what it supports, by doing an extra flush here, before sending
> *ANY* "have" lines yet:
> 
> 	C: # flush -- this is another one after "I'm done with wants".

We don't need a second flush. The first can do. The client just has to expect either "NAK" or "VER blabla blibli multi_ack". "VER" must be sent by the server only when the client requested an extension the server has, though.

> 	S: proto v2 v3 v5

As written above, I'd prefer something like "VER" in order to stay with the 3 capital letters convention. Also, I like to read what it is about, i.e. "VER multi_ack" instead of "VER v2".

> 	C: proto v2

I'd say that this is not necessary. Client can send that in the "want" lines.

> After this exchange, both ends know they understand and would
> want to talk at protocol level v2.  This leaves door open for
> future protocol extension, but more importantly, I think this
> arrangement would make things safer.

Yes. Nice.

I especially like that the client learns early that multi_ack protocol it is. In my tests, non-multi_ack would only be happy when fed with commits sorted by date, while multi_ack is much more efficient when fed commits sorted by distance-to-tip (I cannot yet explain why this is so).

Ciao, Dscho