MAINT-7495 Viewer retries too many time apon 504 from login.cgi

11 files changed, 79 insertions, 28 deletions

diff --git a/indra/llcorehttp/_httpinternal.h b/indra/llcorehttp/_httpinternal.h
index 79c89d6c92..690ebbecd8 100644
--- a/indra/llcorehttp/_httpinternal.h
+++ b/indra/llcorehttp/_httpinternal.h
@@ -127,9 +127,12 @@ const int HTTP_TRACE_MAX = HTTP_TRACE_CURL_BODIES;
 // We want to span a few windows to allow transport to slow
 // after onset of the throttles and then recover without a final
 // failure.  Other systems may need other constants.
-const int HTTP_RETRY_COUNT_DEFAULT = 8;
+const int HTTP_RETRY_COUNT_DEFAULT = 5;
 const int HTTP_RETRY_COUNT_MIN = 0;
 const int HTTP_RETRY_COUNT_MAX = 100;
+const HttpTime HTTP_RETRY_BACKOFF_MIN_DEFAULT = 1E6L; // 1 sec
+const HttpTime HTTP_RETRY_BACKOFF_MAX_DEFAULT = 5E6L; // 5 sec
+const HttpTime HTTP_RETRY_BACKOFF_MAX = 20E6L; // 20 sec
 
 const int HTTP_REDIRECTS_DEFAULT = 10;
 
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 07cc0e4625..fceed8524b 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -140,6 +140,8 @@ HttpOpRequest::HttpOpRequest()
 	  mPolicy503Retries(0),
 	  mPolicyRetryAt(HttpTime(0)),
 	  mPolicyRetryLimit(HTTP_RETRY_COUNT_DEFAULT),
+	  mPolicyMinRetryBackoff(HttpTime(HTTP_RETRY_BACKOFF_MIN_DEFAULT)),
+	  mPolicyMaxRetryBackoff(HttpTime(HTTP_RETRY_BACKOFF_MAX_DEFAULT)),
 	  mCallbackSSLVerify(NULL)
 {
 	// *NOTE:  As members are added, retry initialization/cleanup
@@ -434,6 +436,9 @@ void HttpOpRequest::setupCommon(HttpRequest::policy_t policy_id,
 		mPolicyRetryLimit = options->getRetries();
 		mPolicyRetryLimit = llclamp(mPolicyRetryLimit, HTTP_RETRY_COUNT_MIN, HTTP_RETRY_COUNT_MAX);
 		mTracing = (std::max)(mTracing, llclamp(options->getTrace(), HTTP_TRACE_MIN, HTTP_TRACE_MAX));
+
+		mPolicyMinRetryBackoff = llclamp(options->getMinBackoff(), HttpTime(0), HTTP_RETRY_BACKOFF_MAX);
+		mPolicyMaxRetryBackoff = llclamp(options->getMaxBackoff(), mPolicyMinRetryBackoff, HTTP_RETRY_BACKOFF_MAX);
 	}
 }
 
diff --git a/indra/llcorehttp/_httpoprequest.h b/indra/llcorehttp/_httpoprequest.h
index dbcc57d0fd..43d49324af 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -232,6 +232,8 @@ public:
 	int					mPolicy503Retries;
 	HttpTime			mPolicyRetryAt;
 	int					mPolicyRetryLimit;
+	HttpTime			mPolicyMinRetryBackoff; // initial delay between retries (mcs)
+	HttpTime			mPolicyMaxRetryBackoff;
 };  // end class HttpOpRequest
 
 
diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp
index b2709b53ec..4889cac9bf 100644
--- a/indra/llcorehttp/_httppolicy.cpp
+++ b/indra/llcorehttp/_httppolicy.cpp
@@ -151,20 +151,16 @@ void HttpPolicy::addOp(const HttpOpRequest::ptr_t &op)
 
 void HttpPolicy::retryOp(const HttpOpRequest::ptr_t &op)
 {
-	static const HttpTime retry_deltas[] =
-		{
-			 250000,			// 1st retry in 0.25 S, etc...
-			 500000,
-			1000000,
-			2000000,
-			5000000				// ... to every 5.0 S.
-		};
-	static const int delta_max(int(LL_ARRAY_SIZE(retry_deltas)) - 1);
 	static const HttpStatus error_503(503);
 
 	const HttpTime now(totalTime());
 	const int policy_class(op->mReqPolicy);
-	HttpTime delta(retry_deltas[llclamp(op->mPolicyRetries, 0, delta_max)]);
+
+	HttpTime delta_min = op->mPolicyMinRetryBackoff;
+	HttpTime delta_max = op->mPolicyMaxRetryBackoff;
+	// mPolicyRetries limited to 100
+	U32 delta_factor = op->mPolicyRetries <= 10 ? 1 << op->mPolicyRetries : 1024;
+	HttpTime delta = llmin(delta_min * delta_factor, delta_max);
 	bool external_delta(false);
 
 	if (op->mReplyRetryAfter > 0 && op->mReplyRetryAfter < 30)
diff --git a/indra/llcorehttp/httpoptions.cpp b/indra/llcorehttp/httpoptions.cpp
index aab447f2dd..df5aa52fa9 100644
--- a/indra/llcorehttp/httpoptions.cpp
+++ b/indra/llcorehttp/httpoptions.cpp
@@ -39,6 +39,8 @@ HttpOptions::HttpOptions() :
     mTimeout(HTTP_REQUEST_TIMEOUT_DEFAULT),
     mTransferTimeout(HTTP_REQUEST_XFER_TIMEOUT_DEFAULT),
     mRetries(HTTP_RETRY_COUNT_DEFAULT),
+    mMinRetryBackoff(HTTP_RETRY_BACKOFF_MIN_DEFAULT),
+    mMaxRetryBackoff(HTTP_RETRY_BACKOFF_MAX_DEFAULT),
     mUseRetryAfter(HTTP_USE_RETRY_AFTER_DEFAULT),
     mFollowRedirects(true),
     mVerifyPeer(false),
@@ -81,6 +83,16 @@ void HttpOptions::setRetries(unsigned int retries)
 	mRetries = retries;
 }
 
+void HttpOptions::setMinBackoff(HttpTime delay)
+{
+	mMinRetryBackoff = delay;
+}
+
+void HttpOptions::setMaxBackoff(HttpTime delay)
+{
+	mMaxRetryBackoff = delay;
+}
+
 void HttpOptions::setUseRetryAfter(bool use_retry)
 {
 	mUseRetryAfter = use_retry;
diff --git a/indra/llcorehttp/httpoptions.h b/indra/llcorehttp/httpoptions.h
index 510eaa45bb..8a6de61b04 100644
--- a/indra/llcorehttp/httpoptions.h
+++ b/indra/llcorehttp/httpoptions.h
@@ -101,13 +101,31 @@ public:
 
     /// Sets the number of retries on an LLCore::HTTPRequest before the 
     /// request fails.
-	// Default:  8
+	// Default:  5
 	void				setRetries(unsigned int retries);
 	unsigned int		getRetries() const
 	{
 		return mRetries;
 	}
 
+	/// Sets minimal delay before request retries. In microseconds.
+	/// HttpPolicy will increase delay from min to max with each retry
+	// Default: 1 000 000 mcs
+	void				setMinBackoff(HttpTime delay);
+	HttpTime			getMinBackoff() const
+	{
+		return mMinRetryBackoff;
+	}
+
+	/// Sets maximum delay before request retries. In microseconds.
+	/// HttpPolicy will increase delay from min to max with each retry
+	// Default:  5 000 000 mcs
+	void				setMaxBackoff(HttpTime delay);
+	HttpTime			getMaxBackoff() const
+	{
+		return mMaxRetryBackoff;
+	}
+
 	// Default:  true
 	void				setUseRetryAfter(bool use_retry);
 	bool				getUseRetryAfter() const
@@ -166,6 +184,8 @@ protected:
 	unsigned int		mTimeout;
 	unsigned int		mTransferTimeout;
 	unsigned int		mRetries;
+	HttpTime			mMinRetryBackoff;
+	HttpTime			mMaxRetryBackoff;
 	bool				mUseRetryAfter;
 	bool				mFollowRedirects;
 	bool				mVerifyPeer;
diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml
index 0303581d62..4154c96378 100644
--- a/indra/newview/app_settings/settings.xml
+++ b/indra/newview/app_settings/settings.xml
@@ -5592,12 +5592,12 @@
     <key>Type</key>
     <string>F32</string>
     <key>Value</key>
-    <real>10.0</real>
+    <real>40.0</real>
   </map>
   <key>LoginSRVPump</key>
   <map>
     <key>Comment</key>
-    <string>Name of the message pump that handles SRV request</string>
+    <string>Name of the message pump that handles SRV request (deprecated)</string>
     <key>Persist</key>
     <integer>0</integer>
     <key>Type</key>
diff --git a/indra/newview/lllogininstance.cpp b/indra/newview/lllogininstance.cpp
index b4d0bb6823..40e98947a3 100644
--- a/indra/newview/lllogininstance.cpp
+++ b/indra/newview/lllogininstance.cpp
@@ -63,6 +63,8 @@
 #include <boost/scoped_ptr.hpp>
 #include <sstream>
 
+const S32 LOGIN_MAX_RETRIES = 3;
+
 class LLLoginInstance::Disposable {
 public:
 	virtual ~Disposable() {}
@@ -610,13 +612,16 @@ void LLLoginInstance::constructAuthParams(LLPointer<LLCredential> user_credentia
 	request_params["host_id"] = gSavedSettings.getString("HostID");
 	request_params["extended_errors"] = true; // request message_id and message_args
 
+	// Specify desired timeout/retry options
+	LLSD http_params;
+	http_params["timeout"] = gSavedSettings.getF32("LoginSRVTimeout");
+	http_params["retries"] = LOGIN_MAX_RETRIES;
+
 	mRequestData.clear();
 	mRequestData["method"] = "login_to_simulator";
 	mRequestData["params"] = request_params;
 	mRequestData["options"] = requested_options;
-
-	mRequestData["cfg_srv_timeout"] = gSavedSettings.getF32("LoginSRVTimeout");
-	mRequestData["cfg_srv_pump"] = gSavedSettings.getString("LoginSRVPump");
+	mRequestData["http_params"] = http_params;
 }
 
 bool LLLoginInstance::handleLoginEvent(const LLSD& event)
diff --git a/indra/newview/llxmlrpclistener.cpp b/indra/newview/llxmlrpclistener.cpp
index cc3645131d..99070d5bee 100644
--- a/indra/newview/llxmlrpclistener.cpp
+++ b/indra/newview/llxmlrpclistener.cpp
@@ -312,7 +312,7 @@ public:
         }
         XMLRPC_RequestSetData(request, xparams);
 
-        mTransaction.reset(new LLXMLRPCTransaction(mUri, request));
+        mTransaction.reset(new LLXMLRPCTransaction(mUri, request, true, command.has("http_params")? LLSD(command["http_params"]) : LLSD()));
 		mPreviousStatus = mTransaction->status(NULL);
 
         // Free the XMLRPC_REQUEST object and the attached data values.
diff --git a/indra/newview/llxmlrpctransaction.cpp b/indra/newview/llxmlrpctransaction.cpp
index f8b38669b6..0c8495a6e4 100644
--- a/indra/newview/llxmlrpctransaction.cpp
+++ b/indra/newview/llxmlrpctransaction.cpp
@@ -208,7 +208,7 @@ public:
 	std::string         mCertStore;
 	LLPointer<LLCertificate> mErrorCert;
 
-	Impl(const std::string& uri, XMLRPC_REQUEST request, bool useGzip);
+	Impl(const std::string& uri, XMLRPC_REQUEST request, bool useGzip, const LLSD& httpParams);
 	Impl(const std::string& uri,
 		const std::string& method, LLXMLRPCValue params, bool useGzip);
 	~Impl();
@@ -219,7 +219,7 @@ public:
 	void setHttpStatus(const LLCore::HttpStatus &status);
 
 private:
-	void init(XMLRPC_REQUEST request, bool useGzip);
+	void init(XMLRPC_REQUEST request, bool useGzip, const LLSD& httpParams);
 };
 
 LLXMLRPCTransaction::Handler::Handler(LLCore::HttpRequest::ptr_t &request, 
@@ -315,13 +315,13 @@ void LLXMLRPCTransaction::Handler::onCompleted(LLCore::HttpHandle handle,
 //=========================================================================
 
 LLXMLRPCTransaction::Impl::Impl(const std::string& uri,
-		XMLRPC_REQUEST request, bool useGzip)
+		XMLRPC_REQUEST request, bool useGzip, const LLSD& httpParams)
 	: mHttpRequest(),
 	  mStatus(LLXMLRPCTransaction::StatusNotStarted),
 	  mURI(uri),
 	  mResponse(0)
 {
-	init(request, useGzip);
+	init(request, useGzip, httpParams);
 }
 
 
@@ -337,7 +337,7 @@ LLXMLRPCTransaction::Impl::Impl(const std::string& uri,
 	XMLRPC_RequestSetRequestType(request, xmlrpc_request_call);
 	XMLRPC_RequestSetData(request, params.getValue());
 	
-	init(request, useGzip);
+	init(request, useGzip, LLSD());
     // DEV-28398: without this XMLRPC_RequestFree() call, it looks as though
     // the 'request' object is simply leaked. It's less clear to me whether we
     // should also ask to free request value data (second param 1), since the
@@ -345,7 +345,7 @@ LLXMLRPCTransaction::Impl::Impl(const std::string& uri,
     XMLRPC_RequestFree(request, 1);
 }
 
-void LLXMLRPCTransaction::Impl::init(XMLRPC_REQUEST request, bool useGzip)
+void LLXMLRPCTransaction::Impl::init(XMLRPC_REQUEST request, bool useGzip, const LLSD& httpParams)
 {
 	LLCore::HttpOptions::ptr_t httpOpts;
 	LLCore::HttpHeaders::ptr_t httpHeaders;
@@ -359,7 +359,15 @@ void LLXMLRPCTransaction::Impl::init(XMLRPC_REQUEST request, bool useGzip)
 	// LLRefCounted starts with a 1 ref, so don't add a ref in the smart pointer
 	httpOpts = LLCore::HttpOptions::ptr_t(new LLCore::HttpOptions()); 
 
-	httpOpts->setTimeout(40L);
+	// delay between repeats will start from 5 sec and grow to 20 sec with each repeat
+	httpOpts->setMinBackoff(5E6L);
+	httpOpts->setMaxBackoff(20E6L);
+
+	httpOpts->setTimeout(httpParams.has("timeout") ? httpParams["timeout"].asInteger() : 40L);
+	if (httpParams.has("retries"))
+	{
+		httpOpts->setRetries(httpParams["retries"].asInteger());
+	}
 
 	bool vefifySSLCert = !gSavedSettings.getBOOL("NoVerifySSLCert");
 	mCertStore = gSavedSettings.getString("CertStore");
@@ -526,8 +534,8 @@ void LLXMLRPCTransaction::Impl::setHttpStatus(const LLCore::HttpStatus &status)
 
 
 LLXMLRPCTransaction::LLXMLRPCTransaction(
-	const std::string& uri, XMLRPC_REQUEST request, bool useGzip)
-: impl(* new Impl(uri, request, useGzip))
+	const std::string& uri, XMLRPC_REQUEST request, bool useGzip, const LLSD& httpParams)
+: impl(* new Impl(uri, request, useGzip, httpParams))
 { }
 
 
diff --git a/indra/newview/llxmlrpctransaction.h b/indra/newview/llxmlrpctransaction.h
index 3a1c9c82b7..7a9bc991f7 100644
--- a/indra/newview/llxmlrpctransaction.h
+++ b/indra/newview/llxmlrpctransaction.h
@@ -85,7 +85,7 @@ class LLXMLRPCTransaction
 {
 public:
 	LLXMLRPCTransaction(const std::string& uri,
-		XMLRPC_REQUEST request, bool useGzip = true);
+		XMLRPC_REQUEST request, bool useGzip = true, const LLSD& httpParams = LLSD());
 		// does not take ownership of the request object
 		// request can be freed as soon as the transaction is constructed