Skip to content
Snippets Groups Projects
Commit 89187229 authored by Monty Brandenberg's avatar Monty Brandenberg
Browse files

Refactoring of the request completion thread and removal of 206/content-range hack in xport.

Retry/response handling is decided in policy so moved that there.  Removed special case
206-without-content-range response in transport.  Have this sitation recognizable in the
API and let callers deal with it as needed.
parent f4a59854
No related branches found
No related tags found
No related merge requests found
......@@ -113,8 +113,11 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
CURL * handle(msg->easy_handle);
CURLcode result(msg->data.result);
HttpService::ELoopSpeed speed(completeRequest(mMultiHandles[policy_class], handle, result));
ret = (std::min)(ret, speed);
if (completeRequest(mMultiHandles[policy_class], handle, result))
{
// Request is still active, don't get too sleepy
ret = (std::min)(ret, HttpService::NORMAL);
}
handle = NULL; // No longer valid on return
}
else if (CURLMSG_NONE == msg->msg)
......@@ -161,12 +164,8 @@ void HttpLibcurl::addOp(HttpOpRequest * op)
}
HttpService::ELoopSpeed HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status)
bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status)
{
static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
HttpOpRequest * op(NULL);
curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op);
// *FIXME: check the pointer
......@@ -209,43 +208,11 @@ HttpService::ELoopSpeed HttpLibcurl::completeRequest(CURLM * multi_handle, CURL
curl_multi_remove_handle(multi_handle, handle);
curl_easy_cleanup(handle);
op->mCurlHandle = NULL;
// Retry or finalize
if (! op->mStatus)
{
// If this failed, we might want to retry. Have to inspect
// the status a little more deeply for those reasons worth retrying...
if (op->mPolicyRetries < op->mPolicyRetryLimit &&
((op->mStatus.isHttpStatus() && op->mStatus.mType >= 499 && op->mStatus.mType <= 599) ||
cant_connect == op->mStatus ||
cant_res_proxy == op->mStatus ||
cant_res_host == op->mStatus))
{
// Okay, worth a retry. We include 499 in this test as
// it's the old 'who knows?' error from many grid services...
HttpPolicy & policy(mService->getPolicy());
policy.retryOp(op);
return HttpService::NORMAL; // Having pushed to retry, keep things running
}
}
// This op is done, finalize it delivering it to the reply queue...
if (! op->mStatus)
{
LL_WARNS("CoreHttp") << "URL op failed after " << op->mPolicyRetries
<< " retries. Reason: " << op->mStatus.toString()
<< LL_ENDL;
}
else if (op->mPolicyRetries)
{
LL_WARNS("CoreHttp") << "URL op succeeded after " << op->mPolicyRetries << " retries."
<< LL_ENDL;
}
HttpPolicy & policy(mService->getPolicy());
bool still_active(policy.stageAfterCompletion(op));
op->stageFromActive(mService);
op->release();
return HttpService::REQUEST_SLEEP;
return still_active;
}
......
......@@ -83,9 +83,7 @@ class HttpLibcurl
protected:
/// Invoked when libcurl has indicated a request has been processed
/// to completion and we need to move the request to a new state.
HttpService::ELoopSpeed completeRequest(CURLM * multi_handle,
CURL * handle,
CURLcode status);
bool completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status);
protected:
typedef std::set<HttpOpRequest *> active_set_t;
......
......@@ -144,6 +144,8 @@ HttpOpRequest::~HttpOpRequest()
mCurlHeaders = NULL;
}
mReplyOffset = 0;
mReplyLength = 0;
if (mReplyBody)
{
mReplyBody->release();
......@@ -211,26 +213,11 @@ void HttpOpRequest::visitNotifier(HttpRequest * request)
response->setStatus(mStatus);
response->setBody(mReplyBody);
response->setHeaders(mReplyHeaders);
unsigned int offset(0), length(0);
if (mReplyOffset || mReplyLength)
{
// Got an explicit offset/length in response
offset = mReplyOffset;
length = mReplyLength;
}
else if (mReplyBody && partial_content == mStatus)
{
// Legacy grid services did not provide a 'Content-Range'
// header in responses to full- or partly-satisfyiable
// 'Range' requests. For these, we have to hope that
// the data starts where requested and the length is simply
// whatever we received. A bit of sanity could be provided
// by overlapping ranged requests and verifying that the
// overlap matches.
offset = mReqOffset;
length = mReplyBody->size();
response->setRange(mReplyOffset, mReplyLength);
}
response->setRange(offset, length);
mLibraryHandler->onCompleted(static_cast<HttpHandle>(this), response);
......
......@@ -185,5 +185,47 @@ bool HttpPolicy::changePriority(HttpHandle handle, HttpRequest::priority_t prior
return false;
}
bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
{
static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
// Retry or finalize
if (! op->mStatus)
{
// If this failed, we might want to retry. Have to inspect
// the status a little more deeply for those reasons worth retrying...
if (op->mPolicyRetries < op->mPolicyRetryLimit &&
((op->mStatus.isHttpStatus() && op->mStatus.mType >= 499 && op->mStatus.mType <= 599) ||
cant_connect == op->mStatus ||
cant_res_proxy == op->mStatus ||
cant_res_host == op->mStatus))
{
// Okay, worth a retry. We include 499 in this test as
// it's the old 'who knows?' error from many grid services...
retryOp(op);
return true; // still active/ready
}
}
// This op is done, finalize it delivering it to the reply queue...
if (! op->mStatus)
{
LL_WARNS("CoreHttp") << "URL op failed after " << op->mPolicyRetries
<< " retries. Reason: " << op->mStatus.toString()
<< LL_ENDL;
}
else if (op->mPolicyRetries)
{
LL_WARNS("CoreHttp") << "URL op succeeded after " << op->mPolicyRetries << " retries."
<< LL_ENDL;
}
op->stageFromActive(mService);
op->release();
return false; // not active
}
} // end namespace LLCore
......@@ -79,6 +79,16 @@ class HttpPolicy
// Shadows HttpService's method
bool changePriority(HttpHandle handle, HttpRequest::priority_t priority);
/// When transport is finished with an op and takes it off the
/// active queue, it is delivered here for dispatch. Policy
/// may send it back to the ready/retry queues if it needs another
/// go or we may finalize it and send it on to the reply queue.
///
/// @return Returns true of the request is still active
/// or ready after staging, false if has been
/// sent on to the reply queue.
bool stageAfterCompletion(HttpOpRequest * op);
// Get pointer to global policy options. Caller is expected
// to do context checks like no setting once running.
HttpPolicyGlobal & getGlobalOptions()
......
......@@ -1692,8 +1692,8 @@ void LLTextureFetchWorker::onCompleted(LLCore::HttpHandle handle, LLCore::HttpRe
<< " status: " << status.toHex()
<< " '" << status.toString() << "'"
<< llendl;
unsigned int offset(0), length(0);
response->getRange(&offset, &length);
// unsigned int offset(0), length(0);
// response->getRange(&offset, &length);
// llwarns << "HTTP COMPLETE: " << mID << " handle: " << handle
// << " status: " << status.toULong() << " '" << status.toString() << "'"
// << " req offset: " << mRequestedOffset << " req length: " << mRequestedSize
......@@ -1710,6 +1710,11 @@ void LLTextureFetchWorker::onCompleted(LLCore::HttpHandle handle, LLCore::HttpRe
}
else
{
// A warning about partial (HTTP 206) data. Some grid services
// do *not* return a 'Content-Range' header in the response to
// Range requests with a 206 status. We're forced to assume
// we get what we asked for in these cases until we can fix
// the services.
static const LLCore::HttpStatus par_status(HTTP_PARTIAL_CONTENT);
partial = (par_status == status);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment