From 0c20beda6800149ee71a307ca4e943b5bba56908 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Thu, 4 Sep 2014 16:57:44 -0400 Subject: Pipelining work. Extend transfer timeout by the pipeline depth as transfers can appear delayed with deep pipelining and more requests in the pool. Added bad HTTP status error (typically getting a 0 back as HTTP status from libcurl) to the list of retryable errors. There's a response stream problem with libcurl and pipelining that induces this problem. Retrying helps but may not be entirely safe. Watch bug 1420 on the libcurl sourceforge bug tracker. Extend options of test/example program to include un-ranged requests. Document the excessive data transfer induced when ranged requests are disabled. This is an abnormal mode for very rare users so we'll just eat that for now. --- indra/llcorehttp/httpcommon.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'indra/llcorehttp/httpcommon.cpp') diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp index c2f15155ac..9bcf7ac5e3 100755 --- a/indra/llcorehttp/httpcommon.cpp +++ b/indra/llcorehttp/httpcommon.cpp @@ -4,7 +4,7 @@ * * $LicenseInfo:firstyear=2012&license=viewerlgpl$ * Second Life Viewer Source Code - * Copyright (C) 2012-2013, Linden Research, Inc. + * Copyright (C) 2012-2014, Linden Research, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -219,6 +219,13 @@ std::string HttpStatus::toTerseString() const // Pass true on statuses that might actually be cleared by a // retry. Library failures, calling problems, etc. aren't // going to be fixed by squirting bits all over the Net. +// +// HE_INVALID_HTTP_STATUS is special. As of 7.37.0, there are +// some scenarios where response processing in libcurl appear +// to go wrong and response data is corrupted. A side-effect +// of this is that the HTTP status is read as 0 from the library. +// See libcurl bug report 1420 (https://sourceforge.net/p/curl/bugs/1420/) +// for details. bool HttpStatus::isRetryable() const { static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT); @@ -231,6 +238,7 @@ bool HttpStatus::isRetryable() const static const HttpStatus post_error(HttpStatus::EXT_CURL_EASY, CURLE_HTTP_POST_ERROR); static const HttpStatus partial_file(HttpStatus::EXT_CURL_EASY, CURLE_PARTIAL_FILE); static const HttpStatus inv_cont_range(HttpStatus::LLCORE, HE_INV_CONTENT_RANGE_HDR); + static const HttpStatus inv_status(HttpStatus::LLCORE, HE_INVALID_HTTP_STATUS); return ((isHttpStatus() && mType >= 499 && mType <= 599) || // Include special 499 in retryables *this == cant_connect || // Connection reset/endpoint problems @@ -242,7 +250,8 @@ bool HttpStatus::isRetryable() const *this == op_timedout || // Timer expired *this == post_error || // Transport problem *this == partial_file || // Data inconsistency in response - *this == inv_cont_range); // Short data read disagrees with content-range + *this == inv_cont_range || // Short data read disagrees with content-range + *this == inv_status); // Inv status can reflect internal state problem in libcurl } } // end namespace LLCore -- cgit v1.3 From 3057d246f0c63be1b3015313b9ad9824a66f4e0f Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Thu, 18 Sep 2014 18:42:30 -0400 Subject: Documentation. Describe curl bug 1420 testing and how to reproduce data corruption via timeouts. --- indra/llcorehttp/_httpoprequest.cpp | 15 +++++++++++++++ indra/llcorehttp/httpcommon.cpp | 4 ++++ 2 files changed, 19 insertions(+) (limited to 'indra/llcorehttp/httpcommon.cpp') diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index 38c1f1e78a..4453bf2922 100755 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.cpp @@ -47,6 +47,19 @@ #include "llhttpconstants.h" #include "llproxy.h" +// *DEBUG: "[curl:bugs] #1420" problem and testing. +// +// A pipelining problem, https://sourceforge.net/p/curl/bugs/1420/, +// was a source of Core_9 failures. Code related to this can be +// identified and tested by: +// * Looking for '[curl:bugs]' strings in source and following +// instructions there. +// * Set 'QAModeHttpTrace' to 2 or 3 in settings.xml and look for +// 'timed out' events in the log. +// * Enable the HttpRangeRequestsDisable debug setting which causes +// full asset fetches. These slow the pipelines down a bit. +// + namespace { @@ -610,6 +623,8 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) // *TODO: Find a better scheme than timeouts to guarantee liveness. xfer_timeout *= cpolicy.mPipelining; } + // *DEBUG: Useful for timeout handling and "[curl:bugs] #1420" tests + // xfer_timeout = 3L; code = curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, xfer_timeout); check_curl_easy_code(code, CURLOPT_TIMEOUT); code = curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, timeout); diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp index 9bcf7ac5e3..8714915fa2 100755 --- a/indra/llcorehttp/httpcommon.cpp +++ b/indra/llcorehttp/httpcommon.cpp @@ -240,6 +240,10 @@ bool HttpStatus::isRetryable() const static const HttpStatus inv_cont_range(HttpStatus::LLCORE, HE_INV_CONTENT_RANGE_HDR); static const HttpStatus inv_status(HttpStatus::LLCORE, HE_INVALID_HTTP_STATUS); + // *DEBUG: For "[curl:bugs] #1420" tests. + // Disable the '*this == inv_status' test and look for 'Core_9' + // failures in log files. + return ((isHttpStatus() && mType >= 499 && mType <= 599) || // Include special 499 in retryables *this == cant_connect || // Connection reset/endpoint problems *this == cant_res_proxy || // DNS problems -- cgit v1.3 From ec4fd2f0e226bb2cae5982760317e1d6ea2d2d69 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Fri, 10 Oct 2014 16:43:04 -0400 Subject: MAINT-4564 HTTP Pipelining is not happening in Drano HTTP Phase 4 Incorporate the new libcurl 7.38.0 build with curl bug 1420 workaround. Add developer-centric testing code to evaluate the workaround or a future fix for 1420. --- autobuild.xml | 12 ++++++------ indra/llcorehttp/_httpoprequest.cpp | 4 ++-- indra/llcorehttp/_httppolicy.cpp | 12 ++++++++++++ indra/llcorehttp/httpcommon.cpp | 5 +++-- 4 files changed, 23 insertions(+), 10 deletions(-) (limited to 'indra/llcorehttp/httpcommon.cpp') diff --git a/autobuild.xml b/autobuild.xml index b9db04c28b..1045e11b6d 100755 --- a/autobuild.xml +++ b/autobuild.xml @@ -282,9 +282,9 @@ archive hash - ed283b163e8f74d2c9d6ea5874fcca54 + 40b1c6b3727ebedafc2f1a172797ccd1 url - http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/294562/arch/Darwin/installer/curl-7.38.0-darwin-20140923.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/295367/arch/Darwin/installer/curl-7.38.0-darwin-20141010.tar.bz2 name darwin @@ -294,9 +294,9 @@ archive hash - 6a0a62b6c026fa0b33c0978f4afd152e + 06149da3d7a34adf40853f813ae55328 url - http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/294562/arch/Linux/installer/curl-7.38.0-linux-20140923.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/295367/arch/Linux/installer/curl-7.38.0-linux-20141010.tar.bz2 name linux @@ -306,9 +306,9 @@ archive hash - 56ff4698d5b39a37994f4cc8acba19f0 + e4280eae792a5f13bc9d01d8cfb7c557 url - http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/294562/arch/CYGWIN/installer/curl-7.38.0-windows-20140923.tar.bz2 + http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/295367/arch/CYGWIN/installer/curl-7.38.0-windows-20141010.tar.bz2 name windows diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index bbda0b82fd..fbbb1614fb 100755 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.cpp @@ -626,8 +626,8 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) // *TODO: Find a better scheme than timeouts to guarantee liveness. xfer_timeout *= cpolicy.mPipelining; } - // *DEBUG: Useful for timeout handling and "[curl:bugs] #1420" tests - // xfer_timeout = 3L; + // *DEBUG: Enable following override for timeout handling and "[curl:bugs] #1420" tests + // xfer_timeout = 1L; code = curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, xfer_timeout); check_curl_easy_code(code, CURLOPT_TIMEOUT); code = curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, timeout); diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp index 09b9206f63..e5d6321401 100755 --- a/indra/llcorehttp/_httppolicy.cpp +++ b/indra/llcorehttp/_httppolicy.cpp @@ -414,6 +414,18 @@ bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op) // Retry or finalize if (! op->mStatus) { + // *DEBUG: For "[curl:bugs] #1420" tests. This will interfere + // with unit tests due to allocation retention by logging code. + // But you won't be checking this in enabled. +#if 0 + if (op->mStatus == HttpStatus(HttpStatus::EXT_CURL_EASY, CURLE_OPERATION_TIMEDOUT)) + { + LL_WARNS(LOG_CORE) << "HTTP request " << static_cast(op) + << " timed out." + << LL_ENDL; + } +#endif + // If this failed, we might want to retry. if (op->mPolicyRetries < op->mPolicyRetryLimit && op->mStatus.isRetryable()) { diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp index 8714915fa2..7907e958a4 100755 --- a/indra/llcorehttp/httpcommon.cpp +++ b/indra/llcorehttp/httpcommon.cpp @@ -254,8 +254,9 @@ bool HttpStatus::isRetryable() const *this == op_timedout || // Timer expired *this == post_error || // Transport problem *this == partial_file || // Data inconsistency in response - *this == inv_cont_range || // Short data read disagrees with content-range - *this == inv_status); // Inv status can reflect internal state problem in libcurl + // *DEBUG: Comment out 'inv_status' test for [curl:bugs] #1420 testing. + *this == inv_status || // Inv status can reflect internal state problem in libcurl + *this == inv_cont_range); // Short data read disagrees with content-range } } // end namespace LLCore -- cgit v1.3