All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Turner <David.Turner@twosigma.com>
To: Jeff King <peff@peff.net>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: RE: [PATCH v3] http.postbuffer: allow full range of ssize_t values
Date: Mon, 3 Apr 2017 17:03:49 +0000	[thread overview]
Message-ID: <34d444b673c64310baa275f821037b3e@exmbdft7.ad.twosigma.com> (raw)
In-Reply-To: <20170401060116.b2v7tyoi7fcxwbvo@sigill.intra.peff.net>



> -----Original Message-----
> From: Jeff King [mailto:peff@peff.net]
> Sent: Saturday, April 1, 2017 2:01 AM
> To: David Turner <David.Turner@twosigma.com>
> Cc: git@vger.kernel.org
> Subject: Re: [PATCH v3] http.postbuffer: allow full range of ssize_t values
> 
> On Fri, Mar 31, 2017 at 01:26:31PM -0400, David Turner wrote:
> 
> > Unfortunately, in order to push some large repos, the http postbuffer
> > must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:
> > we just malloc a larger buffer.
> 
> I'm still not sure why a 2GB post-buffer is necessary. It sounds like something
> is broken in your setup. Large pushes should be sent chunked.
> 
> I know broken setups are a fact of life, but this feels like a really hacky work-
> around.

I'm not sure what other workaround I should use.  I guess I could do multiple pushes, but only if individual objects are under the size limit, and I'm not sure all of mine are (maybe I'll get lucky tho).  I know that this is a configuration issue with gitlab: https://gitlab.com/gitlab-org/gitlab-ce/issues/30315 but I don't know when that will get fixed.  I could manually copy the repo to the server and do a local push, but I don't know that I have the necessary permissions to do that. Or I could do this, which would hopefully actually solve the problem.

> > diff --git a/cache.h b/cache.h
> > index fbdf7a815a..5e6747dbb4 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1900,6 +1900,7 @@ extern int git_parse_maybe_bool(const char *);
> > extern int git_config_int(const char *, const char *);  extern int64_t
> > git_config_int64(const char *, const char *);  extern unsigned long
> > git_config_ulong(const char *, const char *);
> > +extern ssize_t git_config_ssize_t(const char *, const char *);
> 
> For most of our other "big" values we use git_config_ulong(). E.g.,
> core.bigfilethreshold. I suspect that would be fine for your purposes here,
> though using size_t is more correct (on Windows "unsigned long" is still only
> 32 bits, even on 64-bit systems).
> 
> The ultimate fate of this number, though, is to be handed to:
> 
>   curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, rpc->len);
> 
> where the final argument is interpreted as a long. So I suspect that on 64-bit
> Windows, setting http.postbuffer to "3G" would cause some kind of weird
> error (either a truncated post or some internal curl error due to the negative
> size, depending on how curl handles it).

Ah, so we would need to use CURLOPT_POSTFIELDSIZE_LARGE.  Will re-roll.

> 
> I think a "git_config_long()" would probably do everything correctly.
> 
> > +static int git_parse_ssize_t(const char *value, ssize_t *ret) {
> > +	ssize_t tmp;
> > +	if (!git_parse_signed(value, &tmp,
> maximum_signed_value_of_type(ssize_t)))
> > +		return 0;
> > +	*ret = tmp;
> > +	return 1;
> > +}
> 
> I saw the earlier iteration used a size_t, but you switched it after the compiler
> (rightfully) complained about the signedness. But I'm not sure why we would
> want ssize_t here instead of just using git_parse_unsigned().

It was originally signed.  I'm not sure why that was, but I figured it would be simpler to save the extra bit just in case.

  parent reply	other threads:[~2017-04-03 17:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-31 17:26 David Turner
2017-03-31 19:51 ` Junio C Hamano
2017-04-01  6:01 ` Jeff King
2017-04-01 18:09   ` Junio C Hamano
2017-04-03 17:03   ` David Turner [this message]
2017-04-04  2:01     ` Jeff King
2017-04-04 18:42       ` David Turner
2017-04-04 20:40         ` Jeff King
2017-04-06 17:24           ` Christian Couder
2017-04-07  4:48             ` Jeff King

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=34d444b673c64310baa275f821037b3e@exmbdft7.ad.twosigma.com \
    --to=david.turner@twosigma.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --subject='RE: [PATCH v3] http.postbuffer: allow full range of ssize_t values' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.