Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required

From: Shawn Pearce <spearce@spearce.org>
Date: Jul 3, 2015, 18:43 UTC
Message-ID: <CAJo=hJsoKSB23KNNF9Xr-NUAqUXvq+O-ZYOSyb4wAC1TZKZ2-g@mail.gmail.com>
In-Reply-To: <20150703180718.GB9223@peff.net>

On Fri, Jul 3, 2015 at 11:07 AM, Jeff King <peff@peff.net> wrote:

Show 6 quoted lines

> I wondered briefly whether this impacted the keepalives we added to
> `upload-pack` in 05e9515; those are implemented as 0-byte data packets,
> which we send during the potentially long counting/delta-compression
> phase before we send out pack data. It works there because the packets
> actually contain a single sideband byte, so they are never mistaken for
> a flush packet.

Interesting. I didn't know about 05e9515. This is a great hack. We have similar issues in our server-server system at $DAY_JOB, they use --quiet as no human is watching. So we did a different hack for the same reason.

Show 13 quoted lines

> Related, I recently ran into a case where I think we should do the same
> for pushes. After receiving the pack, `index-pack` may chew on the
> result for literally minutes (try pushing up the entire linux.git
> history sometime). We say nothing at all on the wire until we've
> finished that, run check_everything_connected, and run all hooks.  Some
> clients (or intermediates on the connection) may give up after a few
> minutes of silence.
>
> I think we should have:
>
>   1. Some progress eye-candy from the server to tell us that something
>      is happening, and how close we are to finishing (basically
>      "index-pack -v").

JGit receive-pack has done this for years. We output a progress monitor for the resolving delta phase, and the counting during the graph connectivity check, as JGit being in Java is slow as snot and cannot digest the linux kernel instantly.

>   2. When progress is disabled, similar keepalive packets saying "nope,
>      no output yet".

Yea this is a problem so I think JGit ignores the client's request for "quiet" here and shovels progress messages anyway as a hack to force keep-alive. Never considered the empty side-band message that 05e9515 introduces.

> For (2), hopefully we can implement it in the same way, and rely on
> empty sideband-0 packets. I haven't tested it in practice, though (I
> have some very rough patches for (1) already).

sideband-0 is not going to work for JGit clients.

JGit clients are strict about the sideband stream being 1,2,3 and fail hard if they get any other stream from the server[1].

[1] https://eclipse.googlesource.com/jgit/jgit/+/master/org.eclipse.jgit/src/org/eclipse/jgit/transport/SideBandInputStream.java#169