From: Drew Northup <n1xim.email@gmail.com>
To: Shawn Pearce <spearce@spearce.org>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org
Subject: Re: [PATCH] Retry HTTP requests on SSL connect failures
Date: Tue, 2 Oct 2012 09:57:29 -0400 [thread overview]
Message-ID: <CAM9Z-nmWdM1g5B+M7sSV3jkAJsUEkHUGPxbUi-r4x7zAwD29qA@mail.gmail.com> (raw)
In-Reply-To: <CAJo=hJtxNq_DYNkok7D29G1takRCp6mw85zsa49XetZNVNEm8g@mail.gmail.com>
On Mon, Oct 1, 2012 at 10:38 PM, Shawn Pearce <spearce@spearce.org> wrote:
> On Mon, Oct 1, 2012 at 3:18 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Oct 01, 2012 at 02:23:06PM -0700, Shawn O. Pearce wrote:
>>>
>>> When libcurl fails to connect to an SSL server always retry the
>>> request once. Since the connection failed before the HTTP headers
>>> can be sent, no data has exchanged hands, so the remote side has
>>> not learned of the request and will not perform it twice.
>>
>> I find this a little distasteful just because we haven't figured out the
>> actual _reason_ for the failure.
>
> No. I didn't try because I reproduced the issue on the initial "GET
> /.../info/refs?service=git-upload-pack" request with no authentication
> required. So the very first thing the remote-https process did was
> fail on an SSL error. During this run I was using a patched Git that
> had a different version of the retry logic, but it immediately retried
> and the retry was successful. At that point I decided the SSL session
> cache wasn't possibly relevant since the first request failed and the
> immediate retry was OK.
>
>> Have you tried running your fails-after-a-few-hours request with other
>> clients that don't have the problem and seeing what they do
>
> This is harder to reproduce than you think. It took me about 5 days of
> continuous polling to reproduce the error. And I have thus far only
> reproduced it against our production servers. This makes it very hard
> to test anything. Or to prove that any given patch is better than a
> different version.
The only sure way to make sure your patch works is to get your load
balancers Slashdotted first (reason noted in my previous mail on this
subject). For the sake of your relationship with your networking crew
I'd not advise doing that intentionally.
>> which means it shouldn't really be affecting the general populace. So
>> even though it feels like a dirty hack, at least it is self-contained,
>> and it does fix a real-world problem. If your answer to the above
>> questions is "hunting this further is just not worth the effort", I can
>> live with that.
>
> I am sort of at that point, but the hack is so ugly... yea, we
> shouldn't have to do this. Or pollute our code with it. I'm willing to
> go back and iterate on this further, but its going to be a while
> before I can provide any more information.
>> How come the first hunk gets a nice for-loop and this one doesn't?
>
> Both hunks retry exactly once after an SSL connect error. I just tried
> to pick something reasonably clean to implement. This hunk seemed
> simple with the if, the other was uglier and a loop seemed the most
> simple way to get a retry in there.
If indeed the problem you are having is with a load balanced setup
then applying TCP/IP like back-off semantics is the right way to go.
The only reason the network stack isn't doing it for you is because
the load balancers wait for the SSL/TLS start before dumping the
"excess" (exceeding of license) SSL connections.
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
next prev parent reply other threads:[~2012-10-02 13:57 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-20 2:55 [PATCH] Disable dumb HTTP fallback with GIT_CURL_FALLBACK=0 Shawn O. Pearce
2012-09-20 3:22 ` Shawn Pearce
2012-09-20 3:52 ` Jeff King
2012-09-20 3:48 ` Jeff King
2012-09-20 5:57 ` Shawn Pearce
2012-09-20 5:58 ` [PATCH] Revert "retry request without query when info/refs?query fails" Shawn O. Pearce
2012-09-20 6:29 ` Junio C Hamano
2012-09-20 6:31 ` Junio C Hamano
2012-09-20 16:24 ` Jeff King
2012-09-20 16:59 ` [PATCH 0/2] smart http toggle switch fails" Jeff King
2012-09-20 17:00 ` [PATCH 1/2] remote-curl: rename is_http variable Jeff King
2012-09-20 17:05 ` [PATCH 2/2] remote-curl: let users turn off smart http Jeff King
2012-09-20 17:53 ` Junio C Hamano
2012-09-20 18:12 ` Jeff King
2012-09-20 18:36 ` Junio C Hamano
2012-09-20 20:51 ` Jeff King
2012-09-20 21:15 ` Junio C Hamano
2012-09-20 21:30 ` Jeff King
2012-09-21 17:34 ` Junio C Hamano
2012-09-21 17:41 ` Jeff King
2012-09-20 17:24 ` [PATCH] Disable dumb HTTP fallback with GIT_CURL_FALLBACK=0 Jeff King
2012-09-20 23:05 ` Shawn Pearce
2012-09-21 5:26 ` Jeff King
2012-09-21 14:19 ` Shawn Pearce
2012-10-01 21:23 ` [PATCH] Retry HTTP requests on SSL connect failures Shawn O. Pearce
2012-10-01 21:47 ` Junio C Hamano
2012-10-01 21:53 ` Junio C Hamano
2012-10-01 22:23 ` Jeff King
2012-10-01 23:20 ` Junio C Hamano
2012-10-01 22:18 ` Jeff King
2012-10-02 2:38 ` Shawn Pearce
2012-10-02 13:57 ` Drew Northup [this message]
2012-10-02 0:14 ` Drew Northup
2012-09-20 4:14 ` Re* [PATCH] Disable dumb HTTP fallback with GIT_CURL_FALLBACK=0 Junio C Hamano
2012-09-20 4:14 ` [PATCH 1/2] Disable dumb HTTP fallback with GIT_DUMB_HTTP_FALLBACK=false Junio C Hamano
2012-09-20 4:14 ` [PATCH 2/2] remote-curl: make dumb-http fallback configurable per URL Junio C Hamano
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAM9Z-nmWdM1g5B+M7sSV3jkAJsUEkHUGPxbUi-r4x7zAwD29qA@mail.gmail.com \
--to=n1xim.email@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=spearce@spearce.org \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).