git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Robin Rosenberg <robin.rosenberg@dewire.com>, git@vger.kernel.org
Subject: Re: [JGIT PATCH 2/2] Decrease the fetch pack client buffer to the lower minimum
Date: Sun, 10 May 2009 17:55:26 -0700	[thread overview]
Message-ID: <20090511005526.GI30527@spearce.org> (raw)
In-Reply-To: <7vfxfctqon.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > This is the lowest buffer size we actually require to keep the
> > client and server sides from deadlocking against each other.
> 
> Is this about the fetch-pack protocol where

Yes.
 
>  (1) upload-pack shows what it has; fetch-pack keeps reading until it sees
>      a flush; then
> 
>  (2) fetch-pack shows what it wants; upload-pack keeps reading; then
> 
>  (3) fetch-pack sends a bunch of have's, followed by a flush; upload-pack
>      keeps reading and then responds with an ACK-continue or NAK, which
>      fetch-pack reads; this step continues zero or more times; and then
>      finally
> 
>  (4) fetch-pack sends a bunch of have's, followed by a flush; upload-pack
>      keeps reading and then responds with an ACK, fetch-pack says done.
> 
> Where do you need "enough buffer"?  The conversation looks very much "it's
> my turn to talk", "now it's your turn to talk and I'll wait until I hear
> from you", to me.  I am puzzled.

In step 3 during the first round the client can send up to 2 blocks
worth of data, with 32 haves per block.  This means the client
writes 2952 bytes of data before it reads.

C Git doesn't run into this sort of problem because a normal pipe
would get 1 page (~4096 bytes) in the kernel for the FIFO buffer.

In SSH transport, we still have 4096 between us and the ssh client
process, plus that has its own buffering.

In TCP transport, we have the kernel TX buffer on this side, and the
kernel RX buffer on the remote side, plus network switch buffers in
the middle.  2952 bytes nicely fits into just over 2 IP packets,
and the TCP window is sufficiently large enough to allow these to
be sent without blocking the writer.

We need to be able to shove 2952 bytes down at the other guy before
we start listening to him.  The upload-pack side of the system can
(at worst) send us 64 "ACK %s continue" lines.  We must be able
to enter into the receive mode before the upload-pack side fills
their outgoing buffer.

In the Sun JVMs a pure in-memory pipe only has room for 1024 bytes
in the FIFO before it blocks.  Though the technique I am using to
boost the FIFO from 1024 to 2952 bytes isn't necessarily going to
be portable to other JVMs.  If both sides only have 1024 bytes of
buffer available and both sides can possibly write more than that,
we deadlock.

> > +	/**
> > +	 * Amount of data the client sends before starting to read.
> > +	 * <p>
> > +	 * Any output stream given to the client must be able to buffer this many
> > +	 * bytes before the client will stop writing and start reading from the
> > +	 * input stream. If the output stream blocks before this many bytes are in
> > +	 * the send queue, the system will deadlock.
> > +	 */
> > +	protected static final int MIN_CLIENT_BUFFER = 2 * 32 * 46 + 4;

And this should be + 8 here.  F@!*!

Robin, can you amend?  It should be + 8 because we send to end()
packets in that initial burst, each packet is 4 bytes in size.

-- 
Shawn.

  reply	other threads:[~2009-05-11  0:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-10 22:48 [JGIT PATCH 1/2] Fix deadlock in native git protocol client for upload-pack Shawn O. Pearce
2009-05-10 22:48 ` [JGIT PATCH 2/2] Decrease the fetch pack client buffer to the lower minimum Shawn O. Pearce
2009-05-11  0:43   ` Junio C Hamano
2009-05-11  0:55     ` Shawn O. Pearce [this message]
2009-05-11  3:51       ` Junio C Hamano
2009-05-11 14:10         ` Shawn O. Pearce
2009-05-11 14:23           ` (unknown), Carl Mercier

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=20090511005526.GI30527@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=robin.rosenberg@dewire.com \
    /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).