From e79a88c8ccfadcd260892000d4dec2ae921b26de Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Tue, 12 Aug 2014 18:21:26 -0400 Subject: Better support for dynamic option changes in llcorehttp. Libcurl has some problems disabling pipelining on a multi handle with outstanding requests so build a more conservative system that allows requests to drain before setting curl multi options. Would rather not have this but it is significantly safer. "HttpPipelining" debug setting is now fully dynamic. Connection limits can also be made dynamic in the near future. Upped the default connection count back to 8 for now but will revisit this in the tuning phase. It might be time to combine mesh and textures into a single asset class. For normal server operations that would be a clear path, but for server under load, the current scheme may be better. Minor cleanup in logging to elminate some redundant strings. Might add some more tracing to the stall logic 'just in case'. --- indra/llcorehttp/_httpoprequest.cpp | 46 +++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 22 deletions(-) (limited to 'indra/llcorehttp/_httpoprequest.cpp') diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index 43dd069bc6..eb664fdced 100755 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.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 @@ -94,6 +94,8 @@ void os_strlower(char * str); void check_curl_easy_code(CURLcode code); void check_curl_easy_code(CURLcode code, int curl_setopt_option); +static const char * const LOG_CORE("CoreHttp"); + } // end anonymous namespace @@ -416,8 +418,8 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) if (! mCurlHandle) { // We're in trouble. We'll continue but it won't go well. - LL_WARNS("CoreHttp") << "Failed to allocate libcurl easy handle. Continuing." - << LL_ENDL; + LL_WARNS(LOG_CORE) << "Failed to allocate libcurl easy handle. Continuing." + << LL_ENDL; return HttpStatus(HttpStatus::LLCORE, HE_BAD_ALLOC); } code = curl_easy_setopt(mCurlHandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4); @@ -538,9 +540,9 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) break; default: - LL_ERRS("CoreHttp") << "Invalid HTTP method in request: " - << int(mReqMethod) << ". Can't recover." - << LL_ENDL; + LL_ERRS(LOG_CORE) << "Invalid HTTP method in request: " + << int(mReqMethod) << ". Can't recover." + << LL_ENDL; break; } @@ -652,8 +654,8 @@ size_t HttpOpRequest::readCallback(void * data, size_t size, size_t nmemb, void { // Warn but continue if the read position moves beyond end-of-body // for some reason. - LL_WARNS("CoreHttp") << "Request body position beyond body size. Truncating request body." - << LL_ENDL; + LL_WARNS(LOG_CORE) << "Request body position beyond body size. Truncating request body." + << LL_ENDL; } return 0; } @@ -790,10 +792,10 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi else { // Ignore the unparsable. - LL_INFOS_ONCE("CoreHttp") << "Problem parsing odd Content-Range header: '" - << std::string(hdr_data, wanted_hdr_size) - << "'. Ignoring." - << LL_ENDL; + LL_INFOS_ONCE(LOG_CORE) << "Problem parsing odd Content-Range header: '" + << std::string(hdr_data, wanted_hdr_size) + << "'. Ignoring." + << LL_ENDL; } } @@ -895,11 +897,11 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe if (logit) { - LL_INFOS("CoreHttp") << "TRACE, LibcurlDebug, Handle: " - << static_cast(op) - << ", Type: " << tag - << ", Data: " << safe_line - << LL_ENDL; + LL_INFOS(LOG_CORE) << "TRACE, LibcurlDebug, Handle: " + << static_cast(op) + << ", Type: " << tag + << ", Data: " << safe_line + << LL_ENDL; } return 0; @@ -1094,9 +1096,9 @@ void check_curl_easy_code(CURLcode code, int curl_setopt_option) // // linux appears to throw a curl error once per session for a bad initialization // at a pretty random time (when enabling cookies). - LL_WARNS("CoreHttp") << "libcurl error detected: " << curl_easy_strerror(code) - << ", curl_easy_setopt option: " << curl_setopt_option - << LL_ENDL; + LL_WARNS(LOG_CORE) << "libcurl error detected: " << curl_easy_strerror(code) + << ", curl_easy_setopt option: " << curl_setopt_option + << LL_ENDL; } } @@ -1109,8 +1111,8 @@ void check_curl_easy_code(CURLcode code) // // linux appears to throw a curl error once per session for a bad initialization // at a pretty random time (when enabling cookies). - LL_WARNS("CoreHttp") << "libcurl error detected: " << curl_easy_strerror(code) - << LL_ENDL; + LL_WARNS(LOG_CORE) << "libcurl error detected: " << curl_easy_strerror(code) + << LL_ENDL; } } -- cgit v1.2.3 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/_httpoprequest.cpp | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) (limited to 'indra/llcorehttp/_httpoprequest.cpp') diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index eb664fdced..38c1f1e78a 100755 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.cpp @@ -378,6 +378,7 @@ void HttpOpRequest::setupCommon(HttpRequest::policy_t policy_id, // Junk may be left around from a failed request and that // needs to be cleaned out. // +// *TODO: Move this to _httplibcurl where it belongs. HttpStatus HttpOpRequest::prepareRequest(HttpService * service) { CURLcode code; @@ -411,8 +412,9 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) // *FIXME: better error handling later HttpStatus status; - // Get global policy options - HttpPolicyGlobal & policy(service->getPolicy().getGlobalOptions()); + // Get global and class policy options + HttpPolicyGlobal & gpolicy(service->getPolicy().getGlobalOptions()); + HttpPolicyClass & cpolicy(service->getPolicy().getClassOptions(mReqPolicy)); mCurlHandle = LLCurl::createStandardCurlHandle(); if (! mCurlHandle) @@ -462,30 +464,30 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) code = curl_easy_setopt(mCurlHandle, CURLOPT_SSL_VERIFYHOST, 0); check_curl_easy_code(code, CURLOPT_SSL_VERIFYHOST); - if (policy.mUseLLProxy) + if (gpolicy.mUseLLProxy) { // Use the viewer-based thread-safe API which has a // fast/safe check for proxy enable. Would like to // encapsulate this someway... LLProxy::getInstance()->applyProxySettings(mCurlHandle); } - else if (policy.mHttpProxy.size()) + else if (gpolicy.mHttpProxy.size()) { // *TODO: This is fine for now but get fuller socks5/ // authentication thing going later.... - code = curl_easy_setopt(mCurlHandle, CURLOPT_PROXY, policy.mHttpProxy.c_str()); + code = curl_easy_setopt(mCurlHandle, CURLOPT_PROXY, gpolicy.mHttpProxy.c_str()); check_curl_easy_code(code, CURLOPT_PROXY); code = curl_easy_setopt(mCurlHandle, CURLOPT_PROXYTYPE, CURLPROXY_HTTP); check_curl_easy_code(code, CURLOPT_PROXYTYPE); } - if (policy.mCAPath.size()) + if (gpolicy.mCAPath.size()) { - code = curl_easy_setopt(mCurlHandle, CURLOPT_CAPATH, policy.mCAPath.c_str()); + code = curl_easy_setopt(mCurlHandle, CURLOPT_CAPATH, gpolicy.mCAPath.c_str()); check_curl_easy_code(code, CURLOPT_CAPATH); } - if (policy.mCAFile.size()) + if (gpolicy.mCAFile.size()) { - code = curl_easy_setopt(mCurlHandle, CURLOPT_CAINFO, policy.mCAFile.c_str()); + code = curl_easy_setopt(mCurlHandle, CURLOPT_CAINFO, gpolicy.mCAFile.c_str()); check_curl_easy_code(code, CURLOPT_CAINFO); } @@ -594,6 +596,20 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) { xfer_timeout = timeout; } + if (cpolicy.mPipelining > 1L) + { + // Pipelining affects both connection and transfer timeout values. + // Requests that are added to a pipeling immediately have completed + // their connection so the connection delay tends to be less than + // the non-pipelined value. Transfers are the opposite. Transfer + // timeout starts once the connection is established and completion + // can be delayed due to the pipelined requests ahead. So, it's + // a handwave but bump the transfer timeout up by the pipelining + // depth to give some room. + // + // *TODO: Find a better scheme than timeouts to guarantee liveness. + xfer_timeout *= cpolicy.mPipelining; + } code = curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, xfer_timeout); check_curl_easy_code(code, CURLOPT_TIMEOUT); code = curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, timeout); -- cgit v1.2.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 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'indra/llcorehttp/_httpoprequest.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); -- cgit v1.2.3 From 79ab7c20703c092a4416a4f9a885e0246fc17ee0 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Fri, 19 Sep 2014 15:34:09 -0400 Subject: Introduce libcurl handle cache. Create a private cache of used handles and a fast handle factory that's thread- correct. --- indra/llcorehttp/_httpoprequest.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'indra/llcorehttp/_httpoprequest.cpp') diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index 4453bf2922..bbda0b82fd 100755 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.cpp @@ -170,6 +170,8 @@ HttpOpRequest::~HttpOpRequest() if (mCurlHandle) { + // Uncertain of thread context so free using + // safest method. curl_easy_cleanup(mCurlHandle); mCurlHandle = NULL; } @@ -429,7 +431,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) HttpPolicyGlobal & gpolicy(service->getPolicy().getGlobalOptions()); HttpPolicyClass & cpolicy(service->getPolicy().getClassOptions(mReqPolicy)); - mCurlHandle = LLCurl::createStandardCurlHandle(); + mCurlHandle = service->getTransport().getHandle(); if (! mCurlHandle) { // We're in trouble. We'll continue but it won't go well. @@ -437,6 +439,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) << LL_ENDL; return HttpStatus(HttpStatus::LLCORE, HE_BAD_ALLOC); } + code = curl_easy_setopt(mCurlHandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4); check_curl_easy_code(code, CURLOPT_IPRESOLVE); code = curl_easy_setopt(mCurlHandle, CURLOPT_NOSIGNAL, 1); -- cgit v1.2.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. --- indra/llcorehttp/_httpoprequest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcorehttp/_httpoprequest.cpp') 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); -- cgit v1.2.3