From: Shawn Pearce <spearce@spearce.org>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
Dave Borowitz <dborowitz@google.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
Date: Fri, 3 Jul 2015 11:43:33 -0700 [thread overview]
Message-ID: <CAJo=hJsoKSB23KNNF9Xr-NUAqUXvq+O-ZYOSyb4wAC1TZKZ2-g@mail.gmail.com> (raw)
In-Reply-To: <20150703180718.GB9223@peff.net>
On Fri, Jul 3, 2015 at 11:07 AM, Jeff King <peff@peff.net> wrote:
> 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.
> 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
next prev parent reply other threads:[~2015-07-03 18:44 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-01 18:08 [PATCH 0/7] Clarify signed push protocol documentation Dave Borowitz
2015-07-01 18:08 ` [PATCH 1/7] pack-protocol.txt: Add warning about protocol inaccuracies Dave Borowitz
2015-07-01 19:39 ` Jonathan Nieder
2015-07-01 19:52 ` Junio C Hamano
2015-07-01 19:56 ` Dave Borowitz
2015-07-01 18:08 ` [PATCH 2/7] pack-protocol.txt: Mark LF in command-list as optional Dave Borowitz
2015-07-01 18:21 ` Stefan Beller
2015-07-01 18:46 ` Dave Borowitz
2015-07-01 18:08 ` [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required Dave Borowitz
2015-07-01 20:00 ` Junio C Hamano
2015-07-01 20:07 ` Dave Borowitz
2015-07-01 20:49 ` Junio C Hamano
2015-07-06 14:46 ` Dave Borowitz
2015-07-06 15:22 ` Dave Borowitz
2015-07-06 15:27 ` Dave Borowitz
2015-07-06 15:29 ` Dave Borowitz
2015-07-06 15:35 ` Dave Borowitz
2015-07-06 16:12 ` Junio C Hamano
2015-07-06 15:46 ` Shawn Pearce
2015-07-06 16:28 ` Junio C Hamano
2015-07-06 16:28 ` Junio C Hamano
2015-07-06 16:38 ` Dave Borowitz
2015-07-06 16:57 ` Junio C Hamano
2015-07-06 17:11 ` Dave Borowitz
2015-07-06 17:18 ` Dave Borowitz
2015-07-06 17:34 ` Junio C Hamano
2015-07-06 17:38 ` Dave Borowitz
2015-07-06 18:06 ` Junio C Hamano
2015-07-06 18:08 ` Dave Borowitz
2015-07-06 18:23 ` Junio C Hamano
2015-07-06 17:30 ` Junio C Hamano
2015-07-06 17:35 ` Dave Borowitz
2015-07-06 17:59 ` Junio C Hamano
2015-07-01 20:36 ` Junio C Hamano
2015-07-01 20:39 ` Junio C Hamano
2015-07-02 13:53 ` Jeff King
2015-07-03 17:45 ` Junio C Hamano
2015-07-03 18:07 ` Jeff King
2015-07-03 18:43 ` Shawn Pearce [this message]
2015-07-03 18:46 ` Jeff King
2015-07-01 18:08 ` [PATCH 4/7] pack-protocol.txt: Elaborate on pusher identity Dave Borowitz
2015-07-01 18:58 ` Junio C Hamano
2015-07-01 18:08 ` [PATCH 5/7] pack-protocol.txt: Be more precise about pusher-key relationship Dave Borowitz
2015-07-01 18:08 ` [PATCH 6/7] pack-protocol.txt: Mark pushee field as optional Dave Borowitz
2015-07-01 18:56 ` Junio C Hamano
2015-07-01 19:06 ` Dave Borowitz
2015-07-01 19:07 ` Junio C Hamano
2015-07-01 19:08 ` Junio C Hamano
2015-07-01 19:31 ` Dave Borowitz
2015-07-01 18:08 ` [PATCH 7/7] send-pack.c: Die if the nonce is empty Dave Borowitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJo=hJsoKSB23KNNF9Xr-NUAqUXvq+O-ZYOSyb4wAC1TZKZ2-g@mail.gmail.com' \
--to=spearce@spearce.org \
--cc=dborowitz@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).