Re: [RFC/PATCH 0/3] protocol v2

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

Junio C Hamano <gitster@pobox.com> writes:

> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Junio pointed out in private that I didn't address the packet length
>> limit (64k). I thought I could get away with a new capability
>> (i.e. not worry about it now) but I finally admit that was a bad
>> hack. So perhaps this on top.
>
> No, I didn't ;-) but I tend to agree that "perhaps 4GB huge packet?"
> is a bad idea.
> ...

I realize that I responded with "No, I did not complain about X, I
had trouble about Y and here is why" and talked mostly about Y
without talking much about X.  So let's touch X a bit.

As to the packet length, I think it is a good idea to give us an
escape hatch to bust 64k limit.  Refs may not be the reason to do
so, but as I said, we cannot forsee the future needs.

Having X behind us, now back to Y, and then I'll remind us of Z ;-)
[*1*]

> My recollection is that the consensus from the last time we
> discussed protocol revamping was to list one capability per packet
> ...

And the above is "the right thing" from the protocol point of view.
The only reason the current protocol says "capabilities go on a
single line separated by SP" is because the hole we found to add to
the protocol was to piggyback after the ref advertisement lines, and
there was no guarantee that we have more than one ref advertised, so
we needed to be able to stuff everything on a single line.

Stepping back and thinking about what a "packet" in pkt-line
protocol is, we realize that it is the smallest logical unit of
transferring information.  The state of a single ref in a series of
ref advertisement.  The fact that receiving end has all the history
leading up to a single commit.  The request to obtain all history
leading up to a single commit.

That is why I say that one-cap-per-packet is the right thing.

These individual logical units are grouped into a larger logical
unit by (1) being at a specific point in the protocol exchange, (2)
being adjacent to each other and (3) terminated by a "flush
packet".  Examples:

 - A bunch of individual ref states that is at the beginning of the
   upload-pack to fetch-pack commniucation that ends with a flush
   constitutes a ref advertisement.  

 - A series of "want" packets at the beginning of the fetch-pack to
   upload-pack communiucation that ends with a flush constitutes a
   fetch request.

Another thing I didn't find in the updated documentation was a
proposal to define what a "flush" exactly means.  

In my above writing, it should be clear that a "flush" is merely
"the end of a group".  It does not mean (and it never meant, until
smart HTTP) "I am finished talking, now it is your turn."  If a
requestor needs to give two groups of items before the responder can
process the request, we would want to be able to say "A1, A2, ...,
now I am done with As; B1, B2, B3, ..., now I am done with Bs; this
concludes my request, and it is your turn to process and respond to
me".  But you cannot easily do so without affecting smart HTTP, as
it is written in such a way that it assumes "flush" is "I am done,
it is your turn".

I am perfectly OK if v2 redefined "flush" to mean "I am done, it is
yoru turn."  But then the protocol should have another way to allow
packets into larger groups.  A sequence of packets "begin A", "A1",
"A2", ..., "end", "begin B", "B1", "B2", "B3", "end", "flush" may be
a way to do so, and if we continue to rely on the order of packets
to help determine the semantics (aka "being at a specific point in
the protocol exchange" above), we may even be able to omit "begin A"
and "begin B" packets (i.e. the "end" is the new "end of a logical
group", which is what "flush" originally was).


[Footnote]

 *1* For those who haven't been following the discussion:

     X: maximum packet length being 64kB might be problematic.

     Y: requiring capability advertisement and request in a single
        packet is wrong.

     Z: the meaning of "flush" needs to be clarified.