From 1a5fa01fb894d8e7da575d313fd5270fe4289ca7 Mon Sep 17 00:00:00 2001
From: andreykproductengine <andreykproductengine@lindenlab.com>
Date: Mon, 24 Jul 2017 17:06:12 +0300
Subject: [PATCH] MAINT-7495 Viewer retries too many time apon 504 from
login.cgi
---
indra/llcorehttp/_httpinternal.h | 5 ++++-
indra/llcorehttp/_httpoprequest.cpp | 5 +++++
indra/llcorehttp/_httpoprequest.h | 2 ++
indra/llcorehttp/_httppolicy.cpp | 16 ++++++---------
indra/llcorehttp/httpoptions.cpp | 12 ++++++++++++
indra/llcorehttp/httpoptions.h | 22 ++++++++++++++++++++-
indra/newview/app_settings/settings.xml | 4 ++--
indra/newview/lllogininstance.cpp | 11 ++++++++---
indra/newview/llxmlrpclistener.cpp | 2 +-
indra/newview/llxmlrpctransaction.cpp | 26 ++++++++++++++++---------
indra/newview/llxmlrpctransaction.h | 2 +-
11 files changed, 79 insertions(+), 28 deletions(-)
diff --git a/indra/llcorehttp/_httpinternal.h b/indra/llcorehttp/_httpinternal.h
index 79c89d6c920..690ebbecd8d 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 07cc0e46255..fceed8524be 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 dbcc57d0fdf..43d49324af8 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -232,6 +232,8 @@ class HttpOpRequest : public HttpOperation
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 b2709b53ecf..4889cac9bf1 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 aab447f2dd1..df5aa52fa9d 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 510eaa45bba..8a6de61b04b 100644
--- a/indra/llcorehttp/httpoptions.h
+++ b/indra/llcorehttp/httpoptions.h
@@ -101,13 +101,31 @@ class HttpOptions : private boost::noncopyable
/// 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 @@ class HttpOptions : private boost::noncopyable
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 0303581d623..4154c963781 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 b4d0bb6823f..40e98947a3d 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 cc3645131dd..99070d5bee1 100644
--- a/indra/newview/llxmlrpclistener.cpp
+++ b/indra/newview/llxmlrpclistener.cpp
@@ -312,7 +312,7 @@ class Poller
}
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 f8b38669b61..0c8495a6e49 100644
--- a/indra/newview/llxmlrpctransaction.cpp
+++ b/indra/newview/llxmlrpctransaction.cpp
@@ -208,7 +208,7 @@ class LLXMLRPCTransaction::Impl
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 @@ class LLXMLRPCTransaction::Impl
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 3a1c9c82b70..7a9bc991f7f 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
--
GitLab