SH-4061 WIP - moved retry policy to llmessage, added integration test

5 files changed, 374 insertions, 74 deletions

diff --git a/indra/llmessage/CMakeLists.txt b/indra/llmessage/CMakeLists.txt
index 6fa2669be6..6df724f960 100644..100755
--- a/indra/llmessage/CMakeLists.txt
+++ b/indra/llmessage/CMakeLists.txt
@@ -67,6 +67,7 @@ set(llmessage_SOURCE_FILES
     llpartdata.cpp
     llproxy.cpp
     llpumpio.cpp
+    llhttpretrypolicy.cpp
     llsdappservices.cpp
     llsdhttpserver.cpp
     llsdmessage.cpp
@@ -266,5 +267,6 @@ if (LL_TESTS)
   LL_ADD_INTEGRATION_TEST(llhttpclientadapter "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(llpartdata "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(llxfer_file "" "${test_libs}")
+  LL_ADD_INTEGRATION_TEST(llhttpretrypolicy "" "${test_libs}")
 endif (LL_TESTS)
 
diff --git a/indra/llmessage/llhttpretrypolicy.cpp b/indra/llmessage/llhttpretrypolicy.cpp
new file mode 100755
index 0000000000..23d9e64c13
--- /dev/null
+++ b/indra/llmessage/llhttpretrypolicy.cpp
@@ -0,0 +1,71 @@
+/** 
+ * @file llhttpretrypolicy.h
+ * @brief Header for a retry policy class intended for use with http responders.
+ *
+ * $LicenseInfo:firstyear=2013&license=viewerlgpl$
+ * Second Life Viewer Source Code
+ * Copyright (C) 2013, Linden Research, Inc.
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License only.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ * 
+ * Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
+ * $/LicenseInfo$
+ */
+
+#include "linden_common.h"
+#include "llhttpretrypolicy.h"
+
+void LLAdaptiveRetryPolicy::onFailure(S32 status, const LLSD& headers)
+{
+	if (mRetryCount > 0)
+	{
+		mDelay = llclamp(mDelay*mBackoffFactor,mMinDelay,mMaxDelay);
+	}
+	// Honor server Retry-After header.
+	// Status 503 may ask us to wait for a certain amount of time before retrying.
+	F32 wait_time = mDelay;
+	F32 retry_header_time;
+	if (headers.has(HTTP_IN_HEADER_RETRY_AFTER)
+		&& getSecondsUntilRetryAfter(headers[HTTP_IN_HEADER_RETRY_AFTER].asStringRef(), retry_header_time))
+	{
+		wait_time = retry_header_time;
+	}
+
+	if (mRetryCount>=mMaxRetries)
+	{
+		llinfos << "Too many retries " << mRetryCount << ", will not retry" << llendl;
+		mShouldRetry = false;
+	}
+	if (!isHttpServerErrorStatus(status))
+	{
+		llinfos << "Non-server error " << status << ", will not retry" << llendl;
+		mShouldRetry = false;
+	}
+	if (mShouldRetry)
+	{
+		llinfos << "Retry count " << mRetryCount << " should retry after " << wait_time << llendl;
+		mRetryTimer.reset();
+		mRetryTimer.setTimerExpirySec(wait_time);
+	}
+	mRetryCount++;
+}
+	
+
+bool LLAdaptiveRetryPolicy::shouldRetry(F32& seconds_to_wait) const
+{
+	llassert(mRetryCount>0); // have to call onFailure() before shouldRetry()
+	seconds_to_wait = mShouldRetry ? mRetryTimer.getRemainingTimeF32() : F32_MAX;
+	return mShouldRetry;
+}
diff --git a/indra/llmessage/llhttpretrypolicy.h b/indra/llmessage/llhttpretrypolicy.h
new file mode 100755
index 0000000000..f2eba0965b
--- /dev/null
+++ b/indra/llmessage/llhttpretrypolicy.h
@@ -0,0 +1,76 @@
+/** 
+ * @file file llhttpretrypolicy.h
+ * @brief declarations for http retry policy class.
+ *
+ * $LicenseInfo:firstyear=2013&license=viewerlgpl$
+ * Second Life Viewer Source Code
+ * Copyright (C) 2013, Linden Research, Inc.
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License only.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ * 
+ * Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
+ * $/LicenseInfo$
+ */
+										
+#ifndef LL_RETRYPOLICY_H
+#define LL_RETRYPOLICY_H
+
+#include "lltimer.h"
+#include "llthread.h"
+#include "llhttpconstants.h"
+
+// This is intended for use with HTTP Clients/Responders, but is not
+// specifically coupled with those classes.
+class LLHTTPRetryPolicy: public LLThreadSafeRefCount
+{
+public:
+	LLHTTPRetryPolicy() {}
+	virtual ~LLHTTPRetryPolicy() {}
+	// Call once after an HTTP failure to update state.
+	virtual void onFailure(S32 status, const LLSD& headers) = 0;
+	virtual bool shouldRetry(F32& seconds_to_wait) const = 0;
+};
+
+// Very general policy with geometric back-off after failures,
+// up to a maximum delay, and maximum number of retries.
+class LLAdaptiveRetryPolicy: public LLHTTPRetryPolicy
+{
+public:
+	LLAdaptiveRetryPolicy(F32 min_delay, F32 max_delay, F32 backoff_factor, U32 max_retries):
+		mMinDelay(min_delay),
+		mMaxDelay(max_delay),
+		mBackoffFactor(backoff_factor),
+		mMaxRetries(max_retries),
+		mDelay(min_delay),
+		mRetryCount(0),
+		mShouldRetry(true)
+	{
+	}
+
+	void onFailure(S32 status, const LLSD& headers);
+	bool shouldRetry(F32& seconds_to_wait) const;
+
+private:
+	F32 mMinDelay; // delay never less than this value
+	F32 mMaxDelay; // delay never exceeds this value
+	F32 mBackoffFactor; // delay increases by this factor after each retry, up to mMaxDelay.
+	U32 mMaxRetries; // maximum number of times shouldRetry will return true.
+	F32 mDelay; // current default delay.
+	U32 mRetryCount; // number of times shouldRetry has been called.
+	LLTimer mRetryTimer; // time until next retry.
+	bool mShouldRetry; // Becomes false after too many retries, or the wrong sort of status received, etc.
+};
+
+#endif
diff --git a/indra/llmessage/tests/llhttpretrypolicy_test.cpp b/indra/llmessage/tests/llhttpretrypolicy_test.cpp
new file mode 100755
index 0000000000..ebf626d9a8
--- /dev/null
+++ b/indra/llmessage/tests/llhttpretrypolicy_test.cpp
@@ -0,0 +1,220 @@
+/** 
+ * @file llhttpretrypolicy_test.cpp
+ * @brief Header tests to exercise the LLHTTPRetryPolicy classes.
+ *
+ * $LicenseInfo:firstyear=2013&license=viewerlgpl$
+ * Second Life Viewer Source Code
+ * Copyright (C) 2013, Linden Research, Inc.
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License only.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ * 
+ * Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
+ * $/LicenseInfo$
+ */
+
+#include "linden_common.h"
+#include "llhttpretrypolicy.h"
+#include "lltut.h"
+
+namespace tut
+{
+struct TestData
+{
+};
+
+typedef test_group<TestData>	RetryPolicyTestGroup;
+typedef RetryPolicyTestGroup::object		RetryPolicyTestObject;
+RetryPolicyTestGroup retryPolicyTestGroup("retry_policy");
+
+template<> template<>
+void RetryPolicyTestObject::test<1>()
+{
+	LLAdaptiveRetryPolicy never_retry(1.0,1.0,1.0,0);
+	LLSD headers;
+	F32 wait_seconds;
+	
+	never_retry.onFailure(500,headers);
+	ensure("never retry", !never_retry.shouldRetry(wait_seconds)); 
+}
+
+template<> template<>
+void RetryPolicyTestObject::test<2>()
+{
+	LLAdaptiveRetryPolicy retry404(1.0,2.0,3.0,10);
+	LLSD headers;
+	F32 wait_seconds;
+	
+	retry404.onFailure(404,headers);
+	ensure("no retry on 404", !retry404.shouldRetry(wait_seconds)); 
+}
+
+template<> template<>
+void RetryPolicyTestObject::test<3>()
+{
+	// Should retry after 1.0, 2.0, 3.0, 3.0 seconds.
+	LLAdaptiveRetryPolicy basic_retry(1.0,3.0,2.0,4);
+	LLSD headers;
+	F32 wait_seconds;
+	bool should_retry;
+	U32 frac_bits = 6;
+
+	// Starting wait 1.0
+	basic_retry.onFailure(500,headers);
+	should_retry = basic_retry.shouldRetry(wait_seconds);
+	ensure("basic_retry 1", should_retry);
+	ensure_approximately_equals("basic_retry 1", wait_seconds, 1.0F, frac_bits);
+
+	// Double wait to 2.0
+	basic_retry.onFailure(500,headers);
+	should_retry = basic_retry.shouldRetry(wait_seconds);
+	ensure("basic_retry 2", should_retry);
+	ensure_approximately_equals("basic_retry 2", wait_seconds, 2.0F, frac_bits);
+
+	// Hit max wait of 3.0 (4.0 clamped to max 3)
+	basic_retry.onFailure(500,headers);
+	should_retry = basic_retry.shouldRetry(wait_seconds);
+	ensure("basic_retry 3", should_retry);
+	ensure_approximately_equals("basic_retry 3", wait_seconds, 3.0F, frac_bits);
+
+	// At max wait, should stay at 3.0
+	basic_retry.onFailure(500,headers);
+	should_retry = basic_retry.shouldRetry(wait_seconds);
+	ensure("basic_retry 4", should_retry);
+	ensure_approximately_equals("basic_retry 4", wait_seconds, 3.0F, frac_bits);
+
+	// Max retries, should fail now.
+	basic_retry.onFailure(500,headers);
+	should_retry = basic_retry.shouldRetry(wait_seconds);
+	ensure("basic_retry 5", !should_retry);
+}
+
+// Retries should stop as soon as a non-5xx error is received.
+template<> template<>
+void RetryPolicyTestObject::test<4>()
+{
+	// Should retry after 1.0, 2.0, 3.0, 3.0 seconds.
+	LLAdaptiveRetryPolicy killer404(1.0,3.0,2.0,4);
+	LLSD headers;
+	F32 wait_seconds;
+	bool should_retry;
+	U32 frac_bits = 6;
+
+	// Starting wait 1.0
+	killer404.onFailure(500,headers);
+	should_retry = killer404.shouldRetry(wait_seconds);
+	ensure("killer404 1", should_retry);
+	ensure_approximately_equals("killer404 1", wait_seconds, 1.0F, frac_bits);
+
+	// Double wait to 2.0
+	killer404.onFailure(500,headers);
+	should_retry = killer404.shouldRetry(wait_seconds);
+	ensure("killer404 2", should_retry);
+	ensure_approximately_equals("killer404 2", wait_seconds, 2.0F, frac_bits);
+
+	// Should fail on non-5xx
+	killer404.onFailure(404,headers);
+	should_retry = killer404.shouldRetry(wait_seconds);
+	ensure("killer404 3", !should_retry);
+
+	// After a non-5xx, should keep failing.
+	killer404.onFailure(500,headers);
+	should_retry = killer404.shouldRetry(wait_seconds);
+	ensure("killer404 4", !should_retry);
+}
+
+// Test handling of "retry-after" header. If present, this header
+// value overrides the computed delay, but does not affect the
+// progression of delay values.  For example, if the normal
+// progression of delays would be 1,2,4,8..., but the 2nd and 3rd calls
+// get a retry header of 33, the pattern would become 1,33,33,8...
+template<> template<>
+void RetryPolicyTestObject::test<5>()
+{
+	LLAdaptiveRetryPolicy policy(1.0,25.0,2.0,6);
+	LLSD headers_with_retry;
+	headers_with_retry[HTTP_IN_HEADER_RETRY_AFTER] = "666";
+	LLSD headers_without_retry;
+	F32 wait_seconds;
+	bool should_retry;
+	U32 frac_bits = 6;
+
+	policy.onFailure(500,headers_without_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 1", should_retry);
+	ensure_approximately_equals("retry header 1", wait_seconds, 1.0F, frac_bits);
+
+	policy.onFailure(500,headers_without_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 2", should_retry);
+	ensure_approximately_equals("retry header 2", wait_seconds, 2.0F, frac_bits);
+
+	policy.onFailure(500,headers_with_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 3", should_retry);
+	// 4.0 overrides by header -> 666.0
+	ensure_approximately_equals("retry header 3", wait_seconds, 666.0F, frac_bits);
+
+	policy.onFailure(500,headers_with_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 4", should_retry);
+	// 8.0 overrides by header -> 666.0
+	ensure_approximately_equals("retry header 4", wait_seconds, 666.0F, frac_bits);
+
+	policy.onFailure(500,headers_without_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 5", should_retry);
+	ensure_approximately_equals("retry header 5", wait_seconds, 16.0F, frac_bits);
+
+	policy.onFailure(500,headers_without_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 6", should_retry);
+	ensure_approximately_equals("retry header 6", wait_seconds, 25.0F, frac_bits);
+
+	policy.onFailure(500,headers_with_retry);
+	should_retry = policy.shouldRetry(wait_seconds);
+	ensure("retry header 7", !should_retry);
+}
+
+// Test getSecondsUntilRetryAfter(const std::string& retry_after, F32& seconds_to_wait),
+// used by header parsing of the retry policy.
+template<> template<>
+void RetryPolicyTestObject::test<6>()
+{
+	F32 seconds_to_wait;
+	bool success;
+
+	std::string str1("0");
+	seconds_to_wait = F32_MAX;
+	success = getSecondsUntilRetryAfter(str1, seconds_to_wait);
+	ensure("parse 1", success);
+	ensure_equals("parse 1", seconds_to_wait, 0.0);
+
+	std::string str2("999.9");
+	seconds_to_wait = F32_MAX;
+	success = getSecondsUntilRetryAfter(str2, seconds_to_wait);
+	ensure("parse 2", success);
+	ensure_approximately_equals("parse 2", seconds_to_wait, 999.9F, 8);
+
+	time_t nowseconds;
+	time(&nowseconds);
+	std::string str3 = LLDate((F64)nowseconds).asRFC1123();
+	seconds_to_wait = F32_MAX;
+	success = getSecondsUntilRetryAfter(str3, seconds_to_wait);
+	ensure("parse 3", success);
+	ensure_approximately_equals("parse 3", seconds_to_wait, 0.0F, 6);
+}
+
+}
+
diff --git a/indra/newview/llappearancemgr.cpp b/indra/newview/llappearancemgr.cpp
index c2be472cbc..85f6f92278 100755
--- a/indra/newview/llappearancemgr.cpp
+++ b/indra/newview/llappearancemgr.cpp
@@ -52,6 +52,7 @@
 #include "llwearablelist.h"
 #include "llsdutil.h"
 #include "llsdserialize.h"
+#include "llhttpretrypolicy.h"
 
 #if LL_MSVC
 // disable boost::lexical_cast warning
@@ -2957,78 +2958,6 @@ void LLAppearanceMgr::updateClothingOrderingInfo(LLUUID cat_id, bool update_base
 	if (inventory_changed) gInventory.notifyObservers();
 }
 
-// This is intended for use with HTTP Clients/Responders, but is not
-// specifically coupled with those classes.
-class LLHTTPRetryPolicy: public LLThreadSafeRefCount
-{
-public:
-	LLHTTPRetryPolicy() {}
-	virtual ~LLHTTPRetryPolicy() {}
-	virtual bool shouldRetry(S32 status, const LLSD& headers, F32& seconds_to_wait) = 0;
-};
-
-// Example of simplest possible policy, not necessarily recommended.
-// This would be a potentially dangerous policy to enable.  Removing for now:
-#if 0
-class LLAlwaysRetryImmediatelyPolicy: public LLHTTPRetryPolicy
-{
-public:
-	LLAlwaysRetryImmediatelyPolicy() {}
-	bool shouldRetry(S32 status, const LLSD& headers, F32& seconds_to_wait)
-	{
-		seconds_to_wait = 0.0;
-		return true;
-	}
-};
-#endif
-
-// Very general policy with geometric back-off after failures,
-// up to a maximum delay, and maximum number of retries.
-class LLAdaptiveRetryPolicy: public LLHTTPRetryPolicy
-{
-public:
-	LLAdaptiveRetryPolicy(F32 min_delay, F32 max_delay, F32 backoff_factor, U32 max_retries):
-		mMinDelay(min_delay),
-		mMaxDelay(max_delay),
-		mBackoffFactor(backoff_factor),
-		mMaxRetries(max_retries),
-		mDelay(min_delay),
-		mRetryCount(0)
-	{
-	}
-
-	bool shouldRetry(S32 status, const LLSD& headers, F32& seconds_to_wait)
-	{
-#if 0
-		// *TODO: Test using status codes to only retry server errors.
-		// Only server errors would potentially return a different result on retry.
-		if (!isHttpServerErrorStatus(status)) return false;
-#endif
-
-#if 0
-		// *TODO: Honor server Retry-After header.
-		// Status 503 may ask us to wait for a certain amount of time before retrying.
-		if (!headers.has(HTTP_IN_HEADER_RETRY_AFTER)
-			|| !getSecondsUntilRetryAfter(headers[HTTP_IN_HEADER_RETRY_AFTER].asStringRef(), seconds_to_wait))
-#endif
-		{
-			seconds_to_wait = mDelay;
-			mDelay = llclamp(mDelay*mBackoffFactor,mMinDelay,mMaxDelay);
-		}
-
-		mRetryCount++;
-		return (mRetryCount<=mMaxRetries);
-	}
-
-private:
-	F32 mMinDelay; // delay never less than this value
-	F32 mMaxDelay; // delay never exceeds this value
-	F32 mBackoffFactor; // delay increases by this factor after each retry, up to mMaxDelay.
-	U32 mMaxRetries; // maximum number of times shouldRetry will return true.
-	F32 mDelay; // current delay.
-	U32 mRetryCount; // number of times shouldRetry has been called.
-};
-
 class RequestAgentUpdateAppearanceResponder: public LLHTTPClient::Responder
 {
 	LOG_CLASS(RequestAgentUpdateAppearanceResponder);
@@ -3084,7 +3013,8 @@ protected:
 	void onFailure()
 	{
 		F32 seconds_to_wait;
-		if (mRetryPolicy->shouldRetry(getStatus(), getResponseHeaders(), seconds_to_wait))
+		mRetryPolicy->onFailure(getStatus(), getResponseHeaders());
+		if (mRetryPolicy->shouldRetry(seconds_to_wait))
 		{
 			llinfos << "retrying" << llendl;
 			doAfterInterval(boost::bind(&LLAppearanceMgr::requestServerAppearanceUpdate,
@@ -3327,7 +3257,8 @@ protected:
 		LL_WARNS("Avatar") << "While attempting to increment the agent's cof we got an error "
 				<< dumpResponse() << LL_ENDL;
 		F32 seconds_to_wait;
-		if (mRetryPolicy->shouldRetry(getStatus(), getResponseHeaders(), seconds_to_wait))
+		mRetryPolicy->onFailure(getStatus(), getResponseHeaders());
+		if (mRetryPolicy->shouldRetry(seconds_to_wait))
 		{
 			llinfos << "retrying" << llendl;
 			doAfterInterval(boost::bind(&LLAppearanceMgr::incrementCofVersion,