Singularity-Viewer/SingularityViewer
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Possible upload timeout improvement.

Browse Source 
When uploading finishes, but is not detected, the timeout should be for
"reply delay", the time that the server takes before it replies, and not
CurlTimeoutLowSpeedTime. This patch adds code that takes this failure
into account (which happened only ONCE for me on Metropolis while flying
around and using trickle (not sure if that is relevant), so it's not
that likely to improvement anything in practise. Note that it is
detected by an assertion when it happens, so that we can safely assume
it normally never happened on SL).

* Generalized PUT / POST configuration by adding
  CurlEasyRequest::setPut, which now also supports keep-alive (which
  still isn't used).
* Upload content length is now stored in CurlEasyRequest::mContentLength
* CurlEasyRequest::has_stalled() now return false if it was possbile
  that the 'upload finished' detect failed AND calls upload_finished()
  itself in that case, so it is no longer 'const'.
* If low speed is detect exactly when the last bytes are being attempted
  to be sent (unlikely scenario), then the upload gets 4 more seconds
  after which is switches to CurlTimeoutReplyDelay.
* Added EDoesAuthentication and EAllowCompressedReply to replace
  booleans, for readability and type-safety, as did EKeepAlive. Note
  that this change inverts the meaning of the compression related parameter.
* Unrelated: removed an unnecessary #include "llurlrequest.h" from
  llxmlrpcresponder.h
This commit is contained in:
Aleric Inglewood
2013-03-10 16:43:15 +01:00
parent 96d302aa42
commit 7dfef31469
10 changed files with 106 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
indra/llmessage/aicurl.cpp
View File

@@ -771,6 +771,22 @@ void CurlEasyRequest::setoptString(CURLoption option, std::string const& value)
setopt(option, value.c_str());
}
void CurlEasyRequest::setPut(U32 size, bool keepalive)
{
DoutCurl("PUT size is " << size << " bytes.");
mContentLength = size;
// The server never replies with 100-continue, so suppress the "Expect: 100-continue" header that libcurl adds by default.
addHeader("Expect:");
if (size > 0 && keepalive)
{
addHeader("Connection: keep-alive");
addHeader("Keep-alive: 300");
}
setopt(CURLOPT_UPLOAD, 1);
setopt(CURLOPT_INFILESIZE, size);
}
void CurlEasyRequest::setPost(AIPostFieldPtr const& postdata, U32 size, bool keepalive)
{
llassert_always(postdata->data());
@@ -788,6 +804,7 @@ void CurlEasyRequest::setPost_raw(U32 size, char const* data, bool keepalive)
// data == NULL when we're going to read the data using CURLOPT_READFUNCTION.
DoutCurl("POST size is " << size << " bytes.");
}
mContentLength = size;
// The server never replies with 100-continue, so suppress the "Expect: 100-continue" header that libcurl adds by default.
addHeader("Expect:");

6
indra/llmessage/aicurlprivate.h
View File

@@ -214,6 +214,7 @@ class CurlEasyRequest : public CurlEasyHandle {
private:
void setPost_raw(U32 size, char const* data, bool keepalive);
public:
void setPut(U32 size, bool keepalive = true);
void setPost(U32 size, bool keepalive = true) { setPost_raw(size, NULL, keepalive); }
void setPost(AIPostFieldPtr const& postdata, U32 size, bool keepalive = true);
void setPost(char const* data, U32 size, bool keepalive = true) { setPost(new AIPostField(data), size, keepalive); }
@@ -299,6 +300,7 @@ class CurlEasyRequest : public CurlEasyHandle {
protected:
curl_slist* mHeaders;
AICurlEasyHandleEvents* mHandleEventsTarget;
U32 mContentLength; // Non-zero if known (only set for PUT and POST).
CURLcode mResult; //AIFIXME: this does not belong in the request object, but belongs in the response object.
AIHTTPTimeoutPolicy const* mTimeoutPolicy;
@@ -321,12 +323,12 @@ class CurlEasyRequest : public CurlEasyHandle {
// Accessor for mTimeout with optional creation of orphaned object (if lockobj != NULL).
LLPointer<curlthread::HTTPTimeout>& httptimeout(void) { if (!mTimeout) { create_timeout_object(); mTimeoutIsOrphan = true; } return mTimeout; }
// Return true if no data has been received on the latest socket (if any) for too long.
bool has_stalled(void) const { return mTimeout && mTimeout->has_stalled(); }
bool has_stalled(void) { return mTimeout && mTimeout->has_stalled(); }
protected:
// This class may only be created as base class of BufferedCurlEasyRequest.
// Throws AICurlNoEasyHandle.
CurlEasyRequest(void) : mHeaders(NULL), mHandleEventsTarget(NULL), mResult(CURLE_FAILED_INIT), mTimeoutPolicy(NULL), mTimeoutIsOrphan(false)
CurlEasyRequest(void) : mHeaders(NULL), mHandleEventsTarget(NULL), mContentLength(0), mResult(CURLE_FAILED_INIT), mTimeoutPolicy(NULL), mTimeoutIsOrphan(false)
#if defined(CWDEBUG) || defined(DEBUG_CURLIO)
, mDebugIsHeadOrGetMethod(false)
#endif

3
indra/llmessage/aicurlthread.cpp
View File

@@ -2058,8 +2058,9 @@ size_t BufferedCurlEasyRequest::curlReadCallback(char* data, size_t size, size_t
S32 bytes = size * nmemb; // The maximum amount to read.
self_w->mLastRead = self_w->getInput()->readAfter(sChannels.out(), self_w->mLastRead, (U8*)data, bytes);
self_w->mRequestTransferedBytes += bytes; // Accumulate data sent to the server.
llassert(self_w->mRequestTransferedBytes <= self_w->mContentLength); // Content-Length should always be known, and we should never be sending more.
// Timeout administration.
if (self_w->httptimeout()->data_sent(bytes))
if (self_w->httptimeout()->data_sent(bytes, self_w->mRequestTransferedBytes >= self_w->mContentLength))
{
// Transfer timed out. Return CURL_READFUNC_ABORT which will abort with error CURLE_ABORTED_BY_CALLBACK.
return CURL_READFUNC_ABORT;

46
indra/llmessage/aihttptimeout.cpp
View File

@@ -105,7 +105,7 @@ U64 HTTPTimeout::sClockCount; // Clock count, set once per select() exi
// queued--><--DNS lookup + connect + send headers-->[<--send body (if any)-->]<--replydelay--><--receive headers + body--><--done
// ^ ^ ^ ^ ^ ^
// | | | | | |
bool HTTPTimeout::data_sent(size_t n)
bool HTTPTimeout::data_sent(size_t n, bool finished)
{
// Generate events.
if (!mLowSpeedOn)
@@ -114,7 +114,7 @@ bool HTTPTimeout::data_sent(size_t n)
reset_lowspeed();
}
// Detect low speed.
return lowspeed(n);
return lowspeed(n, finished);
}
// CURL-THREAD
@@ -127,6 +127,7 @@ void HTTPTimeout::reset_lowspeed(void)
{
mLowSpeedClock = sClockCount;
mLowSpeedOn = true;
mLastBytesSent = false; // We're just starting!
mLastSecond = -1; // This causes lowspeed to initialize the rest.
mStalled = (U64)-1; // Stop reply delay timer.
DoutCurl("reset_lowspeed: mLowSpeedClock = " << mLowSpeedClock << "; mStalled = -1");
@@ -198,9 +199,9 @@ bool HTTPTimeout::data_received(size_t n/*,*/
// queued--><--DNS lookup + connect + send headers-->[<--send body (if any)-->]<--replydelay--><--receive headers + body--><--done
// ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^
// | | | | | | | | | | | | | |
bool HTTPTimeout::lowspeed(size_t bytes)
bool HTTPTimeout::lowspeed(size_t bytes, bool finished)
{
//DoutCurlEntering("HTTPTimeout::lowspeed(" << bytes << ")"); commented out... too spammy for normal use.
//DoutCurlEntering("HTTPTimeout::lowspeed(" << bytes << ", " << finished << ")"); commented out... too spammy for normal use.
// The algorithm to determine if we timed out if different from how libcurls CURLOPT_LOW_SPEED_TIME works.
//
@@ -224,6 +225,9 @@ bool HTTPTimeout::lowspeed(size_t bytes)
// and caused something so evil and hard to find that... NEVER AGAIN!
llassert(second >= 0);
// finished should be false until the very last call to this function.
mLastBytesSent = finished;
// If this is the same second as last time, just add the number of bytes to the current bucket.
if (second == mLastSecond)
{
@@ -287,6 +291,23 @@ bool HTTPTimeout::lowspeed(size_t bytes)
DoutCurl("Average transfer rate is " << (mTotalBytes / low_speed_time) << " bytes/s (low speed limit is " << low_speed_limit << " bytes/s)");
if (mTotalBytes < mintotalbytes)
{
if (finished)
{
llwarns <<
#ifdef CWDEBUG
(void*)get_lockobj() << ": "
#endif
"Transfer rate timeout (average transfer rate below " << low_speed_limit <<
" for more than " << low_speed_time << " second" << ((low_speed_time == 1) ? "" : "s") <<
") but we just sent the LAST bytes! Waiting an additional 4 seconds." << llendl;
// Lets hope these last bytes will make it and do not time out on transfer speed anymore.
// Just give these bytes 4 more seconds to be written to the socket (after which we'll
// assume that the 'upload finished' detection failed and we'll wait another ReplyDelay
// seconds before finally, actually timing out.
mStalled = sClockCount + 4 / sClockWidth;
DoutCurl("mStalled set to sClockCount (" << sClockCount << ") + " << (mStalled - sClockCount) << " (4 seconds)");
return false;
}
// The average transfer rate over the passed low_speed_time seconds is too low. Abort the transfer.
llwarns <<
#ifdef CWDEBUG
@@ -383,6 +404,19 @@ void HTTPTimeout::done(AICurlEasyRequest_wat const& curlEasyRequest_w, CURLcode
DoutCurl("done: mStalled set to -1");
}
bool HTTPTimeout::maybe_upload_finished(void)
{
if (!mUploadFinished && mLastBytesSent)
{
// Assume that 'upload finished' detection failed and the server is slow with a reply.
// Switch to waiting for a reply.
upload_finished();
return true;
}
// The upload certainly finished or certainly did not finish.
return false;
}
// Libcurl uses GetTickCount on windows, with a resolution of 10 to 16 ms.
// As a result, we can not assume that namelookup_time == 0 has a special meaning.
#define LOWRESTIMER LL_WINDOWS
@@ -499,6 +533,10 @@ void HTTPTimeout::print_diagnostics(CurlEasyRequest const* curl_easy_request, ch
{
llinfos << "The request upload finished successfully." << llendl;
}
else if (mLastBytesSent)
{
llinfos << "All bytes where sent to libcurl for upload." << llendl;
}
if (mLastSecond > 0 && mLowSpeedOn)
{
llinfos << "The " << (mNothingReceivedYet ? "upload" : "download") << " did last " << mLastSecond << " second" << ((mLastSecond == 1) ? "" : "s") << ", before it timed out." << llendl;

15
indra/llmessage/aihttptimeout.h
View File

@@ -79,6 +79,7 @@ class HTTPTimeout : public LLRefCount {
U16 mBucket; // The bucket corresponding to mLastSecond.
bool mNothingReceivedYet; // Set when created, reset when the HTML reply header from the server is received.
bool mLowSpeedOn; // Set while uploading or downloading data.
bool mLastBytesSent; // Set when the last bytes were sent to libcurl to be uploaded.
bool mUploadFinished; // Used to keep track of whether upload_finished was called yet.
S32 mLastSecond; // The time at which lowspeed() was last called, in seconds since mLowSpeedClock.
S32 mOverwriteSecond; // The second at which the first bucket of this transfer will be overwritten.
@@ -94,7 +95,7 @@ class HTTPTimeout : public LLRefCount {
public:
HTTPTimeout(AIHTTPTimeoutPolicy const* policy, ThreadSafeBufferedCurlEasyRequest* lock_obj) :
mPolicy(policy), mNothingReceivedYet(true), mLowSpeedOn(false), mUploadFinished(false), mStalled((U64)-1)
mPolicy(policy), mNothingReceivedYet(true), mLowSpeedOn(false), mLastBytesSent(false), mUploadFinished(false), mStalled((U64)-1)
#if defined(CWDEBUG) || defined(DEBUG_CURLIO)
, mLockObj(lock_obj)
#endif
@@ -104,7 +105,7 @@ class HTTPTimeout : public LLRefCount {
void upload_finished(void);
// Called when data is sent. Returns true if transfer timed out.
bool data_sent(size_t n);
bool data_sent(size_t n, bool finished);
// Called when data is received. Returns true if transfer timed out.
bool data_received(size_t n/*,*/ ASSERT_ONLY_COMMA(bool upload_error_status = false));
@@ -112,8 +113,8 @@ class HTTPTimeout : public LLRefCount {
// Called immediately before done() after curl finished, with code.
void done(AICurlEasyRequest_wat const& curlEasyRequest_w, CURLcode code);
// Accessor.
bool has_stalled(void) const { return mStalled < sClockCount; }
// Returns true when we REALLY timed out. Might call upload_finished heuristically.
bool has_stalled(void) { return mStalled < sClockCount && !maybe_upload_finished(); }
// Called from BufferedCurlEasyRequest::processOutput if a timeout occurred.
void print_diagnostics(CurlEasyRequest const* curl_easy_request, char const* eff_url);
@@ -127,7 +128,11 @@ class HTTPTimeout : public LLRefCount {
void reset_lowspeed(void);
// Common low speed detection, Called from data_sent or data_received.
bool lowspeed(size_t bytes);
bool lowspeed(size_t bytes, bool finished = false);
// Return false when we timed out on reply delay, or didn't sent all bytes yet.
// Otherwise calls upload_finished() and return true;
bool maybe_upload_finished(void);
};
} // namespace curlthread

14
indra/llmessage/llhttpclient.cpp
View File

@@ -204,8 +204,8 @@ void LLHTTPClient::request(
AIHTTPHeaders& headers/*,*/
DEBUG_CURLIO_PARAM(EDebugCurl debug),
EKeepAlive keepalive,
bool is_auth,
bool no_compression,
EDoesAuthentication does_auth,
EAllowCompressedReply allow_compression,
AIStateMachine* parent,
AIStateMachine::state_type new_parent_state,
AIEngine* default_engine)
@@ -221,7 +221,7 @@ void LLHTTPClient::request(
LLURLRequest* req;
try
{
req = new LLURLRequest(method, url, body_injector, responder, headers, keepalive, is_auth, no_compression);
req = new LLURLRequest(method, url, body_injector, responder, headers, keepalive, does_auth, allow_compression);
#ifdef DEBUG_CURLIO
req->mCurlEasyRequest.debug(debug);
#endif
@@ -687,17 +687,17 @@ U32 LLHTTPClient::blockingGetRaw(const std::string& url, std::string& body/*,*/
void LLHTTPClient::put(std::string const& url, LLSD const& body, ResponderPtr responder, AIHTTPHeaders& headers/*,*/ DEBUG_CURLIO_PARAM(EDebugCurl debug))
{
request(url, HTTP_PUT, new LLSDInjector(body), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug));
request(url, HTTP_PUT, new LLSDInjector(body), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), no_keep_alive, no_does_authentication, no_allow_compressed_reply);
}
void LLHTTPClient::post(std::string const& url, LLSD const& body, ResponderPtr responder, AIHTTPHeaders& headers/*,*/ DEBUG_CURLIO_PARAM(EDebugCurl debug), EKeepAlive keepalive, AIStateMachine* parent, AIStateMachine::state_type new_parent_state)
{
request(url, HTTP_POST, new LLSDInjector(body), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), keepalive, false, false, parent, new_parent_state);
request(url, HTTP_POST, new LLSDInjector(body), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), keepalive, no_does_authentication, allow_compressed_reply, parent, new_parent_state);
}
void LLHTTPClient::postXMLRPC(std::string const& url, XMLRPC_REQUEST xmlrpc_request, ResponderPtr responder, AIHTTPHeaders& headers/*,*/ DEBUG_CURLIO_PARAM(EDebugCurl debug), EKeepAlive keepalive)
{
request(url, HTTP_POST, new XMLRPCInjector(xmlrpc_request), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), keepalive, true, false); // Does use compression.
request(url, HTTP_POST, new XMLRPCInjector(xmlrpc_request), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), keepalive, does_authentication, allow_compressed_reply);
}
void LLHTTPClient::postXMLRPC(std::string const& url, char const* method, XMLRPC_VALUE value, ResponderPtr responder, AIHTTPHeaders& headers/*,*/ DEBUG_CURLIO_PARAM(EDebugCurl debug), EKeepAlive keepalive)
@@ -708,7 +708,7 @@ void LLHTTPClient::postXMLRPC(std::string const& url, char const* method, XMLRPC
XMLRPC_RequestSetData(xmlrpc_request, value);
// XMLRPCInjector takes ownership of xmlrpc_request and will free it when done.
// LLURLRequest takes ownership of the XMLRPCInjector object and will free it when done.
request(url, HTTP_POST, new XMLRPCInjector(xmlrpc_request), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), keepalive, true, true); // Does not use compression.
request(url, HTTP_POST, new XMLRPCInjector(xmlrpc_request), responder, headers/*,*/ DEBUG_CURLIO_PARAM(debug), keepalive, does_authentication, no_allow_compressed_reply);
}
void LLHTTPClient::postRaw(std::string const& url, char const* data, S32 size, ResponderPtr responder, AIHTTPHeaders& headers/*,*/ DEBUG_CURLIO_PARAM(EDebugCurl debug), EKeepAlive keepalive)

14
indra/llmessage/llhttpclient.h
View File

@@ -75,6 +75,16 @@ enum EKeepAlive {
keep_alive
};
enum EDoesAuthentication {
no_does_authentication = 0,
does_authentication
};
enum EAllowCompressedReply {
no_allow_compressed_reply = 0,
allow_compressed_reply
};
#ifdef DEBUG_CURLIO
enum EDebugCurl {
debug_off = 0,
@@ -419,8 +429,8 @@ public:
AIHTTPHeaders& headers/*,*/
DEBUG_CURLIO_PARAM(EDebugCurl debug),
EKeepAlive keepalive = keep_alive,
bool is_auth = false,
bool no_compression = false,
EDoesAuthentication does_auth = no_does_authentication,
EAllowCompressedReply allow_compression = allow_compressed_reply,
AIStateMachine* parent = NULL,
/*AIStateMachine::state_type*/ U32 new_parent_state = 0,
AIEngine* default_engine = &gMainThreadEngine);

22
indra/llmessage/llurlrequest.cpp
View File

@@ -76,8 +76,8 @@ std::string LLURLRequest::actionAsVerb(LLURLRequest::ERequestAction action)
// This might throw AICurlNoEasyHandle.
LLURLRequest::LLURLRequest(LLURLRequest::ERequestAction action, std::string const& url, Injector* body,
LLHTTPClient::ResponderPtr responder, AIHTTPHeaders& headers, bool keepalive, bool is_auth, bool no_compression) :
mAction(action), mURL(url), mKeepAlive(keepalive), mIsAuth(is_auth), mNoCompression(no_compression),
LLHTTPClient::ResponderPtr responder, AIHTTPHeaders& headers, bool keepalive, bool is_auth, bool compression) :
mAction(action), mURL(url), mKeepAlive(keepalive), mIsAuth(is_auth), mNoCompression(!compression),
mBody(body), mResponder(responder), mHeaders(headers), mResponderNameCache(responder ? responder->getName() : "<uninitialized>")
{
}
@@ -213,17 +213,17 @@ bool LLURLRequest::configure(AICurlEasyRequest_wat const& curlEasyRequest_w)
break;
case LLHTTPClient::HTTP_PUT:
{
// Disable the expect http 1.1 extension. POST and PUT default
// to using this, causing the broken server to get confused.
curlEasyRequest_w->addHeader("Expect:");
curlEasyRequest_w->setopt(CURLOPT_UPLOAD, 1);
curlEasyRequest_w->setopt(CURLOPT_INFILESIZE, mBodySize);
// Set the handle for an http put
curlEasyRequest_w->setPut(mBodySize, mKeepAlive);
// Set Accept-Encoding to allow response compression
curlEasyRequest_w->setoptString(CURLOPT_ENCODING, mNoCompression ? "identity" : "");
rv = true;
break;
}
case LLHTTPClient::HTTP_POST:
{
// Set the handle for an http post
curlEasyRequest_w->setPost(mBodySize, mKeepAlive);
@@ -231,7 +231,7 @@ bool LLURLRequest::configure(AICurlEasyRequest_wat const& curlEasyRequest_w)
curlEasyRequest_w->setoptString(CURLOPT_ENCODING, mNoCompression ? "identity" : "");
rv = true;
break;
}
case LLHTTPClient::HTTP_DELETE:
// Set the handle for an http post
curlEasyRequest_w->setoptString(CURLOPT_CUSTOMREQUEST, "DELETE");

2
indra/newview/lltexturefetch.cpp
View File

@@ -1343,7 +1343,7 @@ bool LLTextureFetchWorker::doWork(S32 param)
}
LLHTTPClient::request(mUrl, LLHTTPClient::HTTP_GET, NULL,
new HTTPGetResponder(mFetcher, mID, LLTimer::getTotalTime(), mRequestedSize, mRequestedOffset, true),
headers/*,*/ DEBUG_CURLIO_PARAM(false), keep_alive, false, false, NULL, 0, NULL);
headers/*,*/ DEBUG_CURLIO_PARAM(false), keep_alive, no_does_authentication, allow_compressed_reply, NULL, 0, NULL);
res = true;
}
if (!res)

1
indra/newview/llxmlrpcresponder.h
View File

@@ -35,7 +35,6 @@
#define LLXMLRPCRESPONDER_H
#include <string>
#include "llurlrequest.h" // Injector
#include "llcurl.h"
#include "llhttpstatuscodes.h"