From bf004be1023347bcabaae6baa1624b2ed78d69fd Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Wed, 1 Aug 2012 12:38:28 -0400
Subject: [PATCH] SH-3308  Beef up retry messaging. Reformatted messages around
 request retry.  Successfully retried requests also message so you can see the
 cycle closed.  Added additional retryable error codes (timeout, other libcurl
 failures).  Commenting and removed some unnecessary std::min logic.

---
 indra/llcorehttp/_httplibcurl.cpp | 10 +++++++--
 indra/llcorehttp/_httppolicy.cpp  | 34 +++++++++++++++++++++++++------
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp
index 4e2e3f0e0ec..6fe0bfc7d1a 100644
--- a/indra/llcorehttp/_httplibcurl.cpp
+++ b/indra/llcorehttp/_httplibcurl.cpp
@@ -97,6 +97,12 @@ void HttpLibcurl::start(int policy_count)
 }
 
 
+// Give libcurl some cycles, invoke it's callbacks, process
+// completed requests finalizing or issuing retries as needed.
+//
+// If active list goes empty *and* we didn't queue any
+// requests for retry, we return a request for a hard
+// sleep otherwise ask for a normal polling interval.
 HttpService::ELoopSpeed HttpLibcurl::processTransport()
 {
 	HttpService::ELoopSpeed	ret(HttpService::REQUEST_SLEEP);
@@ -129,7 +135,7 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
 				if (completeRequest(mMultiHandles[policy_class], handle, result))
 				{
 					// Request is still active, don't get too sleepy
-					ret = (std::min)(ret, HttpService::NORMAL);
+					ret = HttpService::NORMAL;
 				}
 				handle = NULL;			// No longer valid on return
 			}
@@ -150,7 +156,7 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
 
 	if (! mActiveOps.empty())
 	{
-		ret = (std::min)(ret, HttpService::NORMAL);
+		ret = HttpService::NORMAL;
 	}
 	return ret;
 }
diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp
index c7a69ad133c..76c1e224316 100644
--- a/indra/llcorehttp/_httppolicy.cpp
+++ b/indra/llcorehttp/_httppolicy.cpp
@@ -162,8 +162,10 @@ void HttpPolicy::retryOp(HttpOpRequest * op)
 	const HttpTime delta(retry_deltas[llclamp(op->mPolicyRetries, 0, delta_max)]);
 	op->mPolicyRetryAt = now + delta;
 	++op->mPolicyRetries;
-	LL_WARNS("CoreHttp") << "URL op retry #" << op->mPolicyRetries
-						 << " being scheduled for " << delta << " uSecs from now."
+	LL_WARNS("CoreHttp") << "HTTP request " << static_cast<HttpHandle>(op)
+						 << " retry " << op->mPolicyRetries
+						 << " scheduled for +" << (delta / HttpTime(1000))
+						 << " mS.  Status:  " << op->mStatus.toHex()
 						 << LL_ENDL;
 	if (op->mTracing > 0)
 	{
@@ -175,6 +177,17 @@ void HttpPolicy::retryOp(HttpOpRequest * op)
 }
 
 
+// Attempt to deliver requests to the transport layer.
+//
+// Tries to find HTTP requests for each policy class with
+// available capacity.  Starts with the retry queue first
+// looking for requests that have waited long enough then
+// moves on to the ready queue.
+//
+// If all queues are empty, will return an indication that
+// the worker thread may sleep hard otherwise will ask for
+// normal polling frequency.
+//
 HttpService::ELoopSpeed HttpPolicy::processReadyQueue()
 {
 	const HttpTime now(totalTime());
@@ -311,6 +324,9 @@ bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
 	static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
 	static const HttpStatus send_error(HttpStatus::EXT_CURL_EASY, CURLE_SEND_ERROR);
 	static const HttpStatus recv_error(HttpStatus::EXT_CURL_EASY, CURLE_RECV_ERROR);
+	static const HttpStatus upload_failed(HttpStatus::EXT_CURL_EASY, CURLE_UPLOAD_FAILED);
+	static const HttpStatus op_timedout(HttpStatus::EXT_CURL_EASY, CURLE_OPERATION_TIMEDOUT);
+	static const HttpStatus post_error(HttpStatus::EXT_CURL_EASY, CURLE_HTTP_POST_ERROR);
 
 	// Retry or finalize
 	if (! op->mStatus)
@@ -323,7 +339,10 @@ bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
 			 cant_res_proxy == op->mStatus ||
 			 cant_res_host == op->mStatus ||
 			 send_error == op->mStatus ||
-			 recv_error == op->mStatus))
+			 recv_error == op->mStatus ||
+			 upload_failed == op->mStatus ||
+			 op_timedout == op->mStatus ||
+			 post_error == op->mStatus))
 		{
 			// Okay, worth a retry.  We include 499 in this test as
 			// it's the old 'who knows?' error from many grid services...
@@ -335,14 +354,17 @@ bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
 	// This op is done, finalize it delivering it to the reply queue...
 	if (! op->mStatus)
 	{
-		LL_WARNS("CoreHttp") << "URL op failed after " << op->mPolicyRetries
+		LL_WARNS("CoreHttp") << "HTTP request " << static_cast<HttpHandle>(op)
+							 << " failed after " << op->mPolicyRetries
 							 << " retries.  Reason:  " << op->mStatus.toString()
+							 << " (" << op->mStatus.toHex() << ")"
 							 << LL_ENDL;
 	}
 	else if (op->mPolicyRetries)
 	{
-		LL_DEBUGS("CoreHttp") << "URL op succeeded after " << op->mPolicyRetries << " retries."
-							  << LL_ENDL;
+		LL_WARNS("CoreHttp") << "HTTP request " << static_cast<HttpHandle>(op)
+							 << " succeeded on retry " << op->mPolicyRetries << "."
+							 << LL_ENDL;
 	}
 
 	op->stageFromActive(mService);
-- 
GitLab