Re: [PATCH/RFC] Allow curl to rewind the RPC read buffer

From: Martin Storsjö <martin@martin.st>
Date: Dec 2, 2009, 07:45 UTC
Message-ID: <alpine.DEB.2.00.0912020931560.5582@cone.home.martin.st>
In-Reply-To: <be6fef0d0912011832k12eaa093o73b057ddf4ab866@mail.gmail.com>

On Wed, 2 Dec 2009, Tay Ray Chuan wrote:

Show 25 quoted lines

> > What will this result in?  A failed request, then the user increases
> > http.postBuffer, and re-runs the entire command?  I am not suggesting the
> > code should do it differently (e.g.  retry with a larger buffer without
> > having the user to help it).  At least not yet.  That is why my first
> > question above was "what do we do?" and not "what should we do?".
> 
> I guess that by "we" you're referring to the "normal" users of git?
> 
> > I am primarily interested in _documenting_ the expected user experience in
> > the failure case, so that people can notice the message, run "git grep" to
> > find the above line and then run "git blame" to find the commit to read
> > its log message to understand what is going on.
> 
> Yes, the code will just fail. As you might suspect, the code won't
> attempt to mitigate the failure by doing anything, and would require
> intervention on the part of the user.
> 
> What the user could do to make this work:
> 
> 1. Turn off multi-pass authentication and just go with Basic.
> 
> 2. Allow for persistent curl sessions. In theory, we get a 401 the
> first time when we send a GET for info/refs; subsequently, curl knows
> what authentication to use, so the POST request *should* take place
> without the need for rewinding. In theory.

I'd actually put this as number 1 - if this error message pops up for some reason, the first thing would be to find out why reusing the previous curl sessions didn't work.

Other options are:

- Switch to a HTTP server that handles Expect: 100-continue properly
- Try pushing the data in smaller chunks, e.g. if populating a new repo 
from scratch, don't push the whole history in one single run, or populate 
through some other mechanism and just do the incremental pushs over HTTP.

And possibly: Update curl to a version post 7.19.7, which detects the Expect header set by git and tries to await a response from the server before proceeding. (The problem that would solve is if we start sending and manage to send the whole initial 1 MB buffer before the 401 reply from the server is received. But it doesn't solve the case if the server doesn't understand the Expect header at all.)

> 3. Increase http.postBuffer size in the config.

As Shawn pointed out, if the whole request would have to be buffered, the needed size may be prohibitively large, so I guess this isn't a good hint to include in the error message after all. But if the request is sensibly sized (e.g. on the order of tens of MBs), this may be a stopgap solution.

So, should we change the error message to something a bit more descriptive, and add this discussion into the commit message?

// Martin