summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/browser_main.cc20
-rw-r--r--chrome/browser/net/passive_log_collector.cc26
-rw-r--r--chrome/browser/net/passive_log_collector.h14
-rw-r--r--chrome/common/net/gaia/gaia_oauth_client.cc11
-rw-r--r--chrome/common/net/url_fetcher.cc24
-rw-r--r--chrome/common/net/url_fetcher_unittest.cc10
-rw-r--r--chrome/service/cloud_print/cloud_print_url_fetcher.cc11
-rw-r--r--chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc4
-rw-r--r--net/base/backoff_entry.cc50
-rw-r--r--net/base/backoff_entry.h9
-rw-r--r--net/base/backoff_entry_unittest.cc19
-rw-r--r--net/base/net_log_event_type_list.h26
-rw-r--r--net/base/net_log_source_type_list.h5
-rw-r--r--net/tools/testserver/backoff_server.py71
-rw-r--r--net/url_request/url_request_throttler_entry.cc224
-rw-r--r--net/url_request/url_request_throttler_entry.h32
-rw-r--r--net/url_request/url_request_throttler_entry_interface.h7
-rw-r--r--net/url_request/url_request_throttler_manager.cc78
-rw-r--r--net/url_request/url_request_throttler_manager.h34
-rw-r--r--net/url_request/url_request_throttler_unittest.cc133
20 files changed, 702 insertions, 106 deletions
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 596fe0e..8876c30 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -52,6 +52,7 @@
#include "chrome/browser/metrics/thread_watcher.h"
#include "chrome/browser/net/chrome_dns_cert_provenance_checker.h"
#include "chrome/browser/net/chrome_dns_cert_provenance_checker_factory.h"
+#include "chrome/browser/net/chrome_net_log.h"
#include "chrome/browser/net/predictor_api.h"
#include "chrome/browser/net/sdch_dictionary_fetcher.h"
#include "chrome/browser/net/websocket_experiment/websocket_experiment_runner.h"
@@ -204,6 +205,10 @@
#include "views/touchui/touch_factory.h"
#endif
+namespace net {
+class NetLog;
+} // namespace net
+
// BrowserMainParts ------------------------------------------------------------
BrowserMainParts::BrowserMainParts(const MainFunctionParams& parameters)
@@ -722,9 +727,6 @@ void InitializeNetworkOptions(const CommandLine& parsed_command_line) {
net::SpdySessionPool::set_max_sessions_per_domain(value);
}
- net::URLRequestThrottlerManager::GetInstance()->set_enable_thread_checks(
- true);
-
SetDnsCertProvenanceCheckerFactory(CreateChromeDnsCertProvenanceChecker);
if (parsed_command_line.HasSwitch(switches::kEnableWebSocketOverSpdy)) {
@@ -733,6 +735,17 @@ void InitializeNetworkOptions(const CommandLine& parsed_command_line) {
}
}
+void InitializeURLRequestThrottlerManager(net::NetLog* net_log) {
+ net::URLRequestThrottlerManager::GetInstance()->set_enable_thread_checks(
+ true);
+
+ // TODO(joi): Passing the NetLog here is temporary; once I switch the
+ // URLRequestThrottlerManager to be part of the URLRequestContext it will
+ // come from there. Doing it this way for now (2011/5/12) to try to fail
+ // fast in case A/B experiment gives unexpected results.
+ net::URLRequestThrottlerManager::GetInstance()->set_net_log(net_log);
+}
+
// Creates key child threads. We need to do this explicitly since
// BrowserThread::PostTask silently deletes a posted task if the target message
// loop isn't created.
@@ -1409,6 +1422,7 @@ int BrowserMain(const MainFunctionParams& parameters) {
CommandLine::ForCurrentProcess());
InitializeNetworkOptions(parsed_command_line);
+ InitializeURLRequestThrottlerManager(browser_process->net_log());
// Initialize histogram synchronizer system. This is a singleton and is used
// for posting tasks via NewRunnableMethod. Its deleted when it goes out of
diff --git a/chrome/browser/net/passive_log_collector.cc b/chrome/browser/net/passive_log_collector.cc
index 677e1d7..9aa06c6 100644
--- a/chrome/browser/net/passive_log_collector.cc
+++ b/chrome/browser/net/passive_log_collector.cc
@@ -55,6 +55,8 @@ PassiveLogCollector::PassiveLogCollector()
ALLOW_THIS_IN_INITIALIZER_LIST(url_request_tracker_(this)),
ALLOW_THIS_IN_INITIALIZER_LIST(socket_stream_tracker_(this)),
ALLOW_THIS_IN_INITIALIZER_LIST(http_stream_job_tracker_(this)),
+ ALLOW_THIS_IN_INITIALIZER_LIST(
+ exponential_backoff_throttling_tracker_(this)),
num_events_seen_(0) {
// Define the mapping between source types and the tracker objects.
@@ -73,6 +75,8 @@ PassiveLogCollector::PassiveLogCollector()
trackers_[net::NetLog::SOURCE_DISK_CACHE_ENTRY] = &disk_cache_entry_tracker_;
trackers_[net::NetLog::SOURCE_MEMORY_CACHE_ENTRY] = &mem_cache_entry_tracker_;
trackers_[net::NetLog::SOURCE_HTTP_STREAM_JOB] = &http_stream_job_tracker_;
+ trackers_[net::NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING] =
+ &exponential_backoff_throttling_tracker_;
// Make sure our mapping is up-to-date.
for (size_t i = 0; i < arraysize(trackers_); ++i)
DCHECK(trackers_[i]) << "Unhandled SourceType: " << i;
@@ -644,3 +648,25 @@ PassiveLogCollector::HttpStreamJobTracker::DoAddEntry(
return ACTION_NONE;
}
+
+//----------------------------------------------------------------------------
+// ExponentialBackoffThrottlingTracker
+//----------------------------------------------------------------------------
+
+const size_t PassiveLogCollector::
+ ExponentialBackoffThrottlingTracker::kMaxNumSources = 100;
+const size_t PassiveLogCollector::
+ ExponentialBackoffThrottlingTracker::kMaxGraveyardSize = 25;
+
+PassiveLogCollector::
+ ExponentialBackoffThrottlingTracker::ExponentialBackoffThrottlingTracker(
+ PassiveLogCollector* parent)
+ : SourceTracker(kMaxNumSources, kMaxGraveyardSize, parent) {
+}
+
+PassiveLogCollector::SourceTracker::Action
+PassiveLogCollector::ExponentialBackoffThrottlingTracker::DoAddEntry(
+ const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
+ AddEntryToSourceInfo(entry, out_info);
+ return ACTION_NONE;
+}
diff --git a/chrome/browser/net/passive_log_collector.h b/chrome/browser/net/passive_log_collector.h
index d209031..2687c66 100644
--- a/chrome/browser/net/passive_log_collector.h
+++ b/chrome/browser/net/passive_log_collector.h
@@ -341,6 +341,19 @@ class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserver {
DISALLOW_COPY_AND_ASSIGN(HttpStreamJobTracker);
};
+ class ExponentialBackoffThrottlingTracker : public SourceTracker {
+ public:
+ static const size_t kMaxNumSources;
+ static const size_t kMaxGraveyardSize;
+
+ explicit ExponentialBackoffThrottlingTracker(PassiveLogCollector* parent);
+
+ private:
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
+ DISALLOW_COPY_AND_ASSIGN(ExponentialBackoffThrottlingTracker);
+ };
+
PassiveLogCollector();
virtual ~PassiveLogCollector();
@@ -381,6 +394,7 @@ class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserver {
DiskCacheEntryTracker disk_cache_entry_tracker_;
MemCacheEntryTracker mem_cache_entry_tracker_;
HttpStreamJobTracker http_stream_job_tracker_;
+ ExponentialBackoffThrottlingTracker exponential_backoff_throttling_tracker_;
// This array maps each NetLog::SourceType to one of the tracker instances
// defined above. Use of this array avoid duplicating the list of trackers
diff --git a/chrome/common/net/gaia/gaia_oauth_client.cc b/chrome/common/net/gaia/gaia_oauth_client.cc
index 002821d..cab666d 100644
--- a/chrome/common/net/gaia/gaia_oauth_client.cc
+++ b/chrome/common/net/gaia/gaia_oauth_client.cc
@@ -123,12 +123,11 @@ void GaiaOAuthClient::Core::OnURLFetchComplete(
bool should_retry = false;
HandleResponse(source, url, status, response_code, data, &should_retry);
if (should_retry) {
- // If the response code is greater than or equal to 500, then the back-off
- // period has been increased at the network level; otherwise, explicitly
- // call ReceivedContentWasMalformed() to count the current request as a
- // failure and increase the back-off period.
- if (response_code < 500)
- request_->ReceivedContentWasMalformed();
+ // Explicitly call ReceivedContentWasMalformed() to ensure the current
+ // request gets counted as a failure for calculation of the back-off
+ // period. If it was already a failure by status code, this call will
+ // be ignored.
+ request_->ReceivedContentWasMalformed();
num_retries_++;
request_->Start();
} else {
diff --git a/chrome/common/net/url_fetcher.cc b/chrome/common/net/url_fetcher.cc
index 95ffc58..72a167f 100644
--- a/chrome/common/net/url_fetcher.cc
+++ b/chrome/common/net/url_fetcher.cc
@@ -58,7 +58,12 @@ class URLFetcher::Core
// safe to call this multiple times.
void Stop();
- // Reports that the received content was malformed.
+ // Reports that the received content was malformed (i.e. failed parsing
+ // or validation). This makes the throttling logic that does exponential
+ // back-off when servers are having problems treat the current request as
+ // a failure. Your call to this method will be ignored if your request is
+ // already considered a failure based on the HTTP response code or response
+ // headers.
void ReceivedContentWasMalformed();
// Overridden from net::URLRequest::Delegate:
@@ -805,6 +810,9 @@ void URLFetcher::Core::OnCompletedURLRequest(
++num_retries_;
// Restarts the request if we still need to notify the delegate.
if (delegate_) {
+ // Note that backoff_delay_ may be 0 because (a) the URLRequestThrottler
+ // code does not necessarily back off on the first error, and (b) it
+ // only backs off on some of the 5xx status codes.
fetcher_->backoff_delay_ = backoff_release_time_ - base::TimeTicks::Now();
if (fetcher_->backoff_delay_ < base::TimeDelta())
fetcher_->backoff_delay_ = base::TimeDelta();
@@ -832,8 +840,18 @@ void URLFetcher::Core::InformDelegateFetchIsComplete() {
void URLFetcher::Core::NotifyMalformedContent() {
DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
- if (url_throttler_entry_ != NULL)
- url_throttler_entry_->ReceivedContentWasMalformed();
+ if (url_throttler_entry_ != NULL) {
+ int status_code = response_code_;
+ if (status_code == kInvalidHttpResponseCode) {
+ // The status code will generally be known by the time clients
+ // call the |ReceivedContentWasMalformed()| function (which ends up
+ // calling the current function) but if it's not, we need to assume
+ // the response was successful so that the total failure count
+ // used to calculate exponential back-off goes up.
+ status_code = 200;
+ }
+ url_throttler_entry_->ReceivedContentWasMalformed(status_code);
+ }
}
void URLFetcher::Core::ReleaseRequest() {
diff --git a/chrome/common/net/url_fetcher_unittest.cc b/chrome/common/net/url_fetcher_unittest.cc
index ae0b6e9..3533435 100644
--- a/chrome/common/net/url_fetcher_unittest.cc
+++ b/chrome/common/net/url_fetcher_unittest.cc
@@ -729,7 +729,7 @@ TEST_F(URLFetcherProtectTest, Overload) {
net::URLRequestThrottlerManager* manager =
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
- new net::URLRequestThrottlerEntry(manager, 200, 3, 1, 2.0, 0.0, 256));
+ new net::URLRequestThrottlerEntry(manager, "", 200, 3, 1, 2.0, 0.0, 256));
manager->OverrideEntryForTests(url, entry);
CreateFetcher(url);
@@ -752,7 +752,7 @@ TEST_F(URLFetcherProtectTest, ServerUnavailable) {
net::URLRequestThrottlerManager* manager =
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
- new net::URLRequestThrottlerEntry(manager, 200, 3, 1, 2.0, 0.0, 256));
+ new net::URLRequestThrottlerEntry(manager, "", 200, 3, 1, 2.0, 0.0, 256));
manager->OverrideEntryForTests(url, entry);
CreateFetcher(url);
@@ -776,7 +776,7 @@ TEST_F(URLFetcherProtectTestPassedThrough, ServerUnavailablePropagateResponse) {
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
new net::URLRequestThrottlerEntry(
- manager, 200, 3, 100, 2.0, 0.0, 150000));
+ manager, "", 200, 3, 100, 2.0, 0.0, 150000));
// Total time if *not* for not doing automatic backoff would be 150s.
// In reality it should be "as soon as server responds".
manager->OverrideEntryForTests(url, entry);
@@ -822,7 +822,7 @@ TEST_F(URLFetcherCancelTest, ReleasesContext) {
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
new net::URLRequestThrottlerEntry(
- manager, 200, 3, 2000, 2.0, 0.0, 4000));
+ manager, "", 200, 3, 2000, 2.0, 0.0, 4000));
manager->OverrideEntryForTests(url, entry);
// Create a separate thread that will create the URLFetcher. The current
@@ -852,7 +852,7 @@ TEST_F(URLFetcherCancelTest, CancelWhileDelayedStartTaskPending) {
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
new net::URLRequestThrottlerEntry(
- manager, 4000, 1, 2000, 2.0, 0.0, 4000));
+ manager, "", 4000, 1, 2000, 2.0, 0.0, 4000));
manager->OverrideEntryForTests(url, entry);
// Fake that a request has just started.
entry->ReserveSendingTimeForNextRequest(base::TimeTicks());
diff --git a/chrome/service/cloud_print/cloud_print_url_fetcher.cc b/chrome/service/cloud_print/cloud_print_url_fetcher.cc
index 731acd3..13af5f7 100644
--- a/chrome/service/cloud_print/cloud_print_url_fetcher.cc
+++ b/chrome/service/cloud_print/cloud_print_url_fetcher.cc
@@ -102,12 +102,11 @@ void CloudPrintURLFetcher::OnURLFetchComplete(
}
// Retry the request if needed.
if (action == RETRY_REQUEST) {
- // If the response code is greater than or equal to 500, then the back-off
- // period has been increased at the network level; otherwise, explicitly
- // call ReceivedContentWasMalformed() to count the current request as a
- // failure and increase the back-off period.
- if (response_code < 500)
- request_->ReceivedContentWasMalformed();
+ // Explicitly call ReceivedContentWasMalformed() to ensure the current
+ // request gets counted as a failure for calculation of the back-off
+ // period. If it was already a failure by status code, this call will
+ // be ignored.
+ request_->ReceivedContentWasMalformed();
++num_retries_;
if ((-1 != source->max_retries()) &&
diff --git a/chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc b/chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
index 6a2637d..9fd4318 100644
--- a/chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
+++ b/chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
@@ -324,7 +324,7 @@ TEST_F(CloudPrintURLFetcherOverloadTest, Protect) {
net::URLRequestThrottlerManager* manager =
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
- new net::URLRequestThrottlerEntry(manager, 200, 3, 1, 2.0, 0.0, 256));
+ new net::URLRequestThrottlerEntry(manager, "", 200, 3, 1, 2.0, 0.0, 256));
manager->OverrideEntryForTests(url, entry);
CreateFetcher(url, 11);
@@ -348,7 +348,7 @@ TEST_F(CloudPrintURLFetcherRetryBackoffTest, FLAKY_GiveUp) {
net::URLRequestThrottlerManager* manager =
net::URLRequestThrottlerManager::GetInstance();
scoped_refptr<net::URLRequestThrottlerEntry> entry(
- new net::URLRequestThrottlerEntry(manager, 200, 3, 1, 2.0, 0.0, 256));
+ new net::URLRequestThrottlerEntry(manager, "", 200, 3, 1, 2.0, 0.0, 256));
manager->OverrideEntryForTests(url, entry);
CreateFetcher(url, 11);
diff --git a/net/base/backoff_entry.cc b/net/base/backoff_entry.cc
index e7b7b55..5fcaed9 100644
--- a/net/base/backoff_entry.cc
+++ b/net/base/backoff_entry.cc
@@ -13,12 +13,9 @@
namespace net {
BackoffEntry::BackoffEntry(const BackoffEntry::Policy* const policy)
- : failure_count_(0),
- policy_(policy) {
+ : policy_(policy) {
DCHECK(policy_);
-
- // Can't use GetTimeNow() as it's virtual.
- exponential_backoff_release_time_ = base::TimeTicks::Now();
+ Reset();
}
BackoffEntry::~BackoffEntry() {
@@ -42,21 +39,22 @@ void BackoffEntry::InformOfRequest(bool succeeded) {
if (failure_count_ > 0)
--failure_count_;
- // The reason why we are not just cutting the release time to GetTimeNow()
- // is on the one hand, it would unset a release time set by
- // SetCustomReleaseTime and on the other we would like to push every
- // request up to our "horizon" when dealing with multiple in-flight
- // requests. Ex: If we send three requests and we receive 2 failures and
- // 1 success. The success that follows those failures will not reset the
- // release time, further requests will then need to wait the delay caused
- // by the 2 failures.
+ // The reason why we are not just cutting the release time to
+ // ImplGetTimeNow() is on the one hand, it would unset a release
+ // time set by SetCustomReleaseTime and on the other we would like
+ // to push every request up to our "horizon" when dealing with
+ // multiple in-flight requests. Ex: If we send three requests and
+ // we receive 2 failures and 1 success. The success that follows
+ // those failures will not reset the release time, further
+ // requests will then need to wait the delay caused by the 2
+ // failures.
exponential_backoff_release_time_ = std::max(
- GetTimeNow(), exponential_backoff_release_time_);
+ ImplGetTimeNow(), exponential_backoff_release_time_);
}
}
bool BackoffEntry::ShouldRejectRequest() const {
- return exponential_backoff_release_time_ > GetTimeNow();
+ return exponential_backoff_release_time_ > ImplGetTimeNow();
}
base::TimeTicks BackoffEntry::GetReleaseTime() const {
@@ -71,7 +69,7 @@ bool BackoffEntry::CanDiscard() const {
if (policy_->entry_lifetime_ms == -1)
return false;
- base::TimeTicks now = GetTimeNow();
+ base::TimeTicks now = ImplGetTimeNow();
int64 unused_since_ms =
(now - exponential_backoff_release_time_).InMilliseconds();
@@ -92,7 +90,18 @@ bool BackoffEntry::CanDiscard() const {
return unused_since_ms >= policy_->entry_lifetime_ms;
}
-base::TimeTicks BackoffEntry::GetTimeNow() const {
+void BackoffEntry::Reset() {
+ failure_count_ = 0;
+
+ // We leave exponential_backoff_release_time_ unset, meaning 0. We could
+ // initialize to ImplGetTimeNow() but because it's a virtual method it's
+ // not safe to call in the constructor (and the constructor calls Reset()).
+ // The effects are the same, i.e. ShouldRejectRequest() will return false
+ // right after Reset().
+ exponential_backoff_release_time_ = base::TimeTicks();
+}
+
+base::TimeTicks BackoffEntry::ImplGetTimeNow() const {
return base::TimeTicks::Now();
}
@@ -102,7 +111,7 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
if (effective_failure_count == 0) {
// Never reduce previously set release horizon, e.g. due to Retry-After
// header.
- return std::max(GetTimeNow(), exponential_backoff_release_time_);
+ return std::max(ImplGetTimeNow(), exponential_backoff_release_time_);
}
// The delay is calculated with this formula:
@@ -119,8 +128,9 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
// Never reduce previously set release horizon, e.g. due to Retry-After
// header.
- return std::max(GetTimeNow() + base::TimeDelta::FromMilliseconds(delay_int),
- exponential_backoff_release_time_);
+ return std::max(
+ ImplGetTimeNow() + base::TimeDelta::FromMilliseconds(delay_int),
+ exponential_backoff_release_time_);
}
} // namespace net
diff --git a/net/base/backoff_entry.h b/net/base/backoff_entry.h
index 1a7d2dc..a3bca91 100644
--- a/net/base/backoff_entry.h
+++ b/net/base/backoff_entry.h
@@ -69,10 +69,15 @@ class NET_TEST BackoffEntry : NON_EXPORTED_BASE(public base::NonThreadSafe) {
// had for Policy::entry_lifetime_ms_.
bool CanDiscard() const;
+ // Resets this entry to a fresh (as if just constructed) state.
+ void Reset();
+
+ // Returns the failure count for this entry.
+ int failure_count() const { return failure_count_; }
+
protected:
// Equivalent to TimeTicks::Now(), virtual so unit tests can override.
- // TODO(joi): Switch to constructor-time dependency injection?
- virtual base::TimeTicks GetTimeNow() const;
+ virtual base::TimeTicks ImplGetTimeNow() const;
private:
// Calculates when requests should again be allowed through.
diff --git a/net/base/backoff_entry_unittest.cc b/net/base/backoff_entry_unittest.cc
index cad40a6..320c9e3 100644
--- a/net/base/backoff_entry_unittest.cc
+++ b/net/base/backoff_entry_unittest.cc
@@ -25,7 +25,7 @@ class TestBackoffEntry : public BackoffEntry {
virtual ~TestBackoffEntry() {}
- virtual TimeTicks GetTimeNow() const {
+ virtual TimeTicks ImplGetTimeNow() const OVERRIDE {
return now_;
}
@@ -108,29 +108,30 @@ TEST(BackoffEntryTest, ReleaseTimeCalculation) {
// With zero errors, should return "now".
TimeTicks result = entry.GetReleaseTime();
- EXPECT_EQ(entry.GetTimeNow(), result);
+ EXPECT_EQ(entry.ImplGetTimeNow(), result);
// 1 error.
entry.InformOfRequest(false);
result = entry.GetReleaseTime();
- EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(1000), result);
+ EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(1000), result);
// 2 errors.
entry.InformOfRequest(false);
result = entry.GetReleaseTime();
- EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(2000), result);
+ EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(2000), result);
// 3 errors.
entry.InformOfRequest(false);
result = entry.GetReleaseTime();
- EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(4000), result);
+ EXPECT_EQ(entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(4000), result);
// 6 errors (to check it doesn't pass maximum).
entry.InformOfRequest(false);
entry.InformOfRequest(false);
entry.InformOfRequest(false);
result = entry.GetReleaseTime();
- EXPECT_EQ(entry.GetTimeNow() + TimeDelta::FromMilliseconds(20000), result);
+ EXPECT_EQ(
+ entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(20000), result);
}
TEST(BackoffEntryTest, ReleaseTimeCalculationWithJitter) {
@@ -144,8 +145,10 @@ TEST(BackoffEntryTest, ReleaseTimeCalculationWithJitter) {
entry.InformOfRequest(false);
entry.InformOfRequest(false);
TimeTicks result = entry.GetReleaseTime();
- EXPECT_LE(entry.GetTimeNow() + TimeDelta::FromMilliseconds(3200), result);
- EXPECT_GE(entry.GetTimeNow() + TimeDelta::FromMilliseconds(4000), result);
+ EXPECT_LE(
+ entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(3200), result);
+ EXPECT_GE(
+ entry.ImplGetTimeNow() + TimeDelta::FromMilliseconds(4000), result);
}
}
diff --git a/net/base/net_log_event_type_list.h b/net/base/net_log_event_type_list.h
index 8d7b20a..5d9bf22 100644
--- a/net/base/net_log_event_type_list.h
+++ b/net/base/net_log_event_type_list.h
@@ -1019,3 +1019,29 @@ EVENT_TYPE(APPCACHE_DELIVERING_ERROR_RESPONSE)
// This event is emitted whenever NetworkChangeNotifier determines that the
// underlying network has changed.
EVENT_TYPE(NETWORK_IP_ADDRESSES_CHANGED)
+
+// ------------------------------------------------------------------------
+// Exponential back-off throttling events
+// ------------------------------------------------------------------------
+
+// Emitted when back-off is disabled for a given host, or the first time
+// a localhost URL is used (back-off is always disabled for localhost).
+// {
+// "host": <The hostname back-off was disabled for>
+// }
+EVENT_TYPE(THROTTLING_DISABLED_FOR_HOST)
+
+// Emitted when a request is denied due to exponential back-off throttling.
+// {
+// "url": <URL that was being requested>,
+// "num_failures": <Failure count for the URL>,
+// "release_after_ms": <Number of milliseconds until URL will be unblocked>
+// }
+EVENT_TYPE(THROTTLING_REJECTED_REQUEST)
+
+// Emitted when throttling entry receives an X-Retry-After header.
+// {
+// "url": <URL that was being requested>,
+// "retry_after_ms": <Milliseconds until retry-after expires>
+// }
+EVENT_TYPE(THROTTLING_GOT_CUSTOM_RETRY_AFTER)
diff --git a/net/base/net_log_source_type_list.h b/net/base/net_log_source_type_list.h
index d5fed96..e0af952 100644
--- a/net/base/net_log_source_type_list.h
+++ b/net/base/net_log_source_type_list.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -18,5 +18,6 @@ SOURCE_TYPE(HOST_RESOLVER_IMPL_JOB, 8)
SOURCE_TYPE(DISK_CACHE_ENTRY, 9)
SOURCE_TYPE(MEMORY_CACHE_ENTRY, 10)
SOURCE_TYPE(HTTP_STREAM_JOB, 11)
+SOURCE_TYPE(EXPONENTIAL_BACKOFF_THROTTLING, 12)
-SOURCE_TYPE(COUNT, 12) // Always keep this as the last entry.
+SOURCE_TYPE(COUNT, 13) // Always keep this as the last entry.
diff --git a/net/tools/testserver/backoff_server.py b/net/tools/testserver/backoff_server.py
index 4188cd1..783fd83 100644
--- a/net/tools/testserver/backoff_server.py
+++ b/net/tools/testserver/backoff_server.py
@@ -13,8 +13,50 @@ import sys
import urlparse
+AJAX_TEST_PAGE = '''
+<html>
+<head>
+<script>
+
+function reportResult(txt) {
+ var element = document.createElement('p');
+ element.innerHTML = txt;
+ document.body.appendChild(element);
+}
+
+function fetch() {
+ var response_code = document.getElementById('response_code');
+
+ xmlhttp = new XMLHttpRequest();
+ xmlhttp.open("GET",
+ "http://%s:%d/%s?code=" + response_code.value,
+ true);
+ xmlhttp.onreadystatechange = function() {
+ reportResult(
+ 'readyState=' + xmlhttp.readyState + ', status=' + xmlhttp.status);
+ }
+ try {
+ xmlhttp.send(null);
+ } catch (e) {
+ reportResult('Exception: ' + e);
+ }
+}
+
+</script>
+</head>
+<body>
+<form action="javascript:fetch()">
+ Response code to get: <input id="response_code" type="text" value="503">
+ <input type="submit">
+</form>
+</body>
+</html>'''
+
+
class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
keep_running = True
+ local_ip = ''
+ port = 0
def do_GET(self):
if self.path == '/quitquitquit':
@@ -25,22 +67,43 @@ class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
RequestHandler.keep_running = False
return
+ if self.path.startswith('/ajax/'):
+ self.send_response(200)
+ self.send_header('Content-Type', 'text/html')
+ self.end_headers()
+ self.wfile.write(AJAX_TEST_PAGE % (self.local_ip,
+ self.port,
+ self.path[6:]))
+ return
+
params = urlparse.parse_qs(urlparse.urlparse(self.path).query)
+ def SendCustomRetryIfRequested():
+ if params and 'custom-retry-after' in params:
+ custom_retry = params['custom-retry-after'][0]
+ self.send_header('X-Retry-After', custom_retry)
+
if not params or not 'code' in params or params['code'][0] == '200':
self.send_response(200)
self.send_header('Content-Type', 'text/plain')
+ SendCustomRetryIfRequested()
self.end_headers()
self.wfile.write('OK')
else:
- self.send_error(int(params['code'][0]))
+ status_code = int(params['code'][0])
+ self.send_response(status_code)
+ SendCustomRetryIfRequested()
+ self.end_headers()
+ self.wfile.write('Error %d' % int(status_code))
def main():
- if len(sys.argv) != 2:
- print "Usage: %s PORT" % sys.argv[0]
+ if len(sys.argv) != 3:
+ print "Usage: %s LOCAL_IP PORT" % sys.argv[0]
sys.exit(1)
- port = int(sys.argv[1])
+ RequestHandler.local_ip = sys.argv[1]
+ port = int(sys.argv[2])
+ RequestHandler.port = port
print "To stop the server, go to http://localhost:%d/quitquitquit" % port
httpd = BaseHTTPServer.HTTPServer(('', port), RequestHandler)
while RequestHandler.keep_running:
diff --git a/net/url_request/url_request_throttler_entry.cc b/net/url_request/url_request_throttler_entry.cc
index b14bcbe..45dc619 100644
--- a/net/url_request/url_request_throttler_entry.cc
+++ b/net/url_request/url_request_throttler_entry.cc
@@ -7,8 +7,12 @@
#include <cmath>
#include "base/logging.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
#include "base/rand_util.h"
#include "base/string_number_conversions.h"
+#include "base/values.h"
+#include "net/base/net_log.h"
#include "net/url_request/url_request_throttler_header_interface.h"
#include "net/url_request/url_request_throttler_manager.h"
@@ -30,9 +34,10 @@ const int URLRequestThrottlerEntry::kDefaultMaxSendThreshold = 20;
// unavailable at the end of each back-off period for a total of about
// 48 minutes.
//
-// Ignoring the first 4 errors helps avoid back-off from kicking in on
-// flaky connections.
-const int URLRequestThrottlerEntry::kDefaultNumErrorsToIgnore = 4;
+// Ignoring the first couple of errors is just a conservative measure to
+// avoid false positives. It should help avoid back-off from kicking in e.g.
+// on flaky connections.
+const int URLRequestThrottlerEntry::kDefaultNumErrorsToIgnore = 2;
const int URLRequestThrottlerEntry::kDefaultInitialBackoffMs = 700;
const double URLRequestThrottlerEntry::kDefaultMultiplyFactor = 1.4;
const double URLRequestThrottlerEntry::kDefaultJitterFactor = 0.4;
@@ -44,20 +49,71 @@ const char URLRequestThrottlerEntry::kExponentialThrottlingHeader[] =
const char URLRequestThrottlerEntry::kExponentialThrottlingDisableValue[] =
"disable";
+// NetLog parameters when a request is rejected by throttling.
+class RejectedRequestParameters : public NetLog::EventParameters {
+ public:
+ RejectedRequestParameters(const std::string& url_id,
+ int num_failures,
+ int release_after_ms)
+ : url_id_(url_id),
+ num_failures_(num_failures),
+ release_after_ms_(release_after_ms) {
+ }
+
+ virtual Value* ToValue() const {
+ DictionaryValue* dict = new DictionaryValue();
+ dict->SetString("url", url_id_);
+ dict->SetInteger("num_failures", num_failures_);
+ dict->SetInteger("release_after_ms", release_after_ms_);
+ return dict;
+ }
+
+ private:
+ std::string url_id_;
+ int num_failures_;
+ int release_after_ms_;
+};
+
+// NetLog parameters when a response contains an X-Retry-After header.
+class RetryAfterParameters : public NetLog::EventParameters {
+ public:
+ RetryAfterParameters(const std::string& url_id,
+ int retry_after_ms)
+ : url_id_(url_id),
+ retry_after_ms_(retry_after_ms) {
+ }
+
+ virtual Value* ToValue() const {
+ DictionaryValue* dict = new DictionaryValue();
+ dict->SetString("url", url_id_);
+ dict->SetInteger("retry_after_ms", retry_after_ms_);
+ return dict;
+ }
+
+ private:
+ std::string url_id_;
+ int retry_after_ms_;
+};
+
URLRequestThrottlerEntry::URLRequestThrottlerEntry(
- URLRequestThrottlerManager* manager)
+ URLRequestThrottlerManager* manager,
+ const std::string& url_id)
: sliding_window_period_(
base::TimeDelta::FromMilliseconds(kDefaultSlidingWindowPeriodMs)),
max_send_threshold_(kDefaultMaxSendThreshold),
is_backoff_disabled_(false),
backoff_entry_(&backoff_policy_),
- manager_(manager) {
+ manager_(manager),
+ url_id_(url_id),
+ net_log_(BoundNetLog::Make(
+ manager->net_log(), NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING)) {
DCHECK(manager_);
Initialize();
}
URLRequestThrottlerEntry::URLRequestThrottlerEntry(
URLRequestThrottlerManager* manager,
+ const std::string& url_id,
int sliding_window_period_ms,
int max_send_threshold,
int initial_backoff_ms,
@@ -69,7 +125,8 @@ URLRequestThrottlerEntry::URLRequestThrottlerEntry(
max_send_threshold_(max_send_threshold),
is_backoff_disabled_(false),
backoff_entry_(&backoff_policy_),
- manager_(manager) {
+ manager_(manager),
+ url_id_(url_id) {
DCHECK_GT(sliding_window_period_ms, 0);
DCHECK_GT(max_send_threshold_, 0);
DCHECK_GE(initial_backoff_ms, 0);
@@ -106,7 +163,7 @@ bool URLRequestThrottlerEntry::IsEntryOutdated() const {
// If there are send events in the sliding window period, we still need this
// entry.
if (!send_log_.empty() &&
- send_log_.back() + sliding_window_period_ > GetTimeNow()) {
+ send_log_.back() + sliding_window_period_ > ImplGetTimeNow()) {
return false;
}
@@ -122,15 +179,37 @@ void URLRequestThrottlerEntry::DetachManager() {
}
bool URLRequestThrottlerEntry::IsDuringExponentialBackoff() const {
- if (is_backoff_disabled_)
- return false;
+ bool reject_request = false;
+ if (!is_backoff_disabled_ && GetBackoffEntry()->ShouldRejectRequest()) {
+ int num_failures = GetBackoffEntry()->failure_count();
+ int release_after_ms =
+ (GetBackoffEntry()->GetReleaseTime() - base::TimeTicks::Now())
+ .InMilliseconds();
+
+ net_log_.AddEvent(
+ NetLog::TYPE_THROTTLING_REJECTED_REQUEST,
+ make_scoped_refptr(
+ new RejectedRequestParameters(url_id_,
+ num_failures,
+ release_after_ms)));
+
+ reject_request = true;
+ }
+
+ int reject_count = reject_request ? 1 : 0;
+ UMA_HISTOGRAM_ENUMERATION(
+ "Throttling.RequestThrottled", reject_count, 2);
+ if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) {
+ UMA_HISTOGRAM_ENUMERATION(base::FieldTrial::MakeName(
+ "Throttling.RequestThrottled", "ThrottlingEnabled"), reject_count, 2);
+ }
- return GetBackoffEntry()->ShouldRejectRequest();
+ return reject_request;
}
int64 URLRequestThrottlerEntry::ReserveSendingTimeForNextRequest(
const base::TimeTicks& earliest_time) {
- base::TimeTicks now = GetTimeNow();
+ base::TimeTicks now = ImplGetTimeNow();
// If a lot of requests were successfully made recently,
// sliding_window_release_time_ may be greater than
@@ -171,7 +250,7 @@ base::TimeTicks
// wrong thing to do (i.e. it would likely be too long). Therefore, we
// return "now" so that retries are not delayed.
if (is_backoff_disabled_)
- return GetTimeNow();
+ return ImplGetTimeNow();
return GetBackoffEntry()->GetReleaseTime();
}
@@ -179,7 +258,10 @@ base::TimeTicks
void URLRequestThrottlerEntry::UpdateWithResponse(
const std::string& host,
const URLRequestThrottlerHeaderInterface* response) {
- if (response->GetResponseCode() >= 500) {
+ int response_code = response->GetResponseCode();
+ HandleMetricsTracking(response_code);
+
+ if (IsConsideredError(response_code)) {
GetBackoffEntry()->InformOfRequest(false);
} else {
GetBackoffEntry()->InformOfRequest(true);
@@ -195,15 +277,20 @@ void URLRequestThrottlerEntry::UpdateWithResponse(
}
}
-void URLRequestThrottlerEntry::ReceivedContentWasMalformed() {
+void URLRequestThrottlerEntry::ReceivedContentWasMalformed(int response_code) {
// A malformed body can only occur when the request to fetch a resource
// was successful. Therefore, in such a situation, we will receive one
- // call to ReceivedContentWasMalformed() and one call to UpdateWithResponse()
- // with a response categorized as "good". To end up counting one failure,
- // we need to count two failures here against the one success in
- // UpdateWithResponse().
- GetBackoffEntry()->InformOfRequest(false);
- GetBackoffEntry()->InformOfRequest(false);
+ // call to ReceivedContentWasMalformed() and one call to
+ // UpdateWithResponse() with a response categorized as "good". To end
+ // up counting one failure, we need to count two failures here against
+ // the one success in UpdateWithResponse().
+ //
+ // We do nothing for a response that is already being considered an error
+ // based on its status code (otherwise we would count 3 errors instead of 1).
+ if (!IsConsideredError(response_code)) {
+ GetBackoffEntry()->InformOfRequest(false);
+ GetBackoffEntry()->InformOfRequest(false);
+ }
}
URLRequestThrottlerEntry::~URLRequestThrottlerEntry() {
@@ -217,9 +304,38 @@ void URLRequestThrottlerEntry::Initialize() {
backoff_policy_.jitter_factor = kDefaultJitterFactor;
backoff_policy_.maximum_backoff_ms = kDefaultMaximumBackoffMs;
backoff_policy_.entry_lifetime_ms = kDefaultEntryLifetimeMs;
+
+ // We pretend we just had a successful response so that we have a
+ // starting point to our tracking. This is called from the
+ // constructor so we do not use the virtual ImplGetTimeNow().
+ last_successful_response_time_ = base::TimeTicks::Now();
+ last_response_was_success_ = true;
+}
+
+bool URLRequestThrottlerEntry::IsConsideredError(int response_code) {
+ // We throttle only for the status codes most likely to indicate the server
+ // is failing because it is too busy or otherwise are likely to be
+ // because of DDoS.
+ //
+ // 500 is the generic error when no better message is suitable, and
+ // as such does not necessarily indicate a temporary state, but
+ // other status codes cover most of the permanent error states.
+ // 503 is explicitly documented as a temporary state where the server
+ // is either overloaded or down for maintenance.
+ // 509 is the (non-standard but widely implemented) Bandwidth Limit Exceeded
+ // status code, which might indicate DDoS.
+ //
+ // We do not back off on 502 or 504, which are reported by gateways
+ // (proxies) on timeouts or failures, because in many cases these requests
+ // have not made it to the destination server and so we do not actually
+ // know that it is down or busy. One degenerate case could be a proxy on
+ // localhost, where you are not actually connected to the network.
+ return (response_code == 500 ||
+ response_code == 503 ||
+ response_code == 509);
}
-base::TimeTicks URLRequestThrottlerEntry::GetTimeNow() const {
+base::TimeTicks URLRequestThrottlerEntry::ImplGetTimeNow() const {
return base::TimeTicks::Now();
}
@@ -241,8 +357,22 @@ void URLRequestThrottlerEntry::HandleCustomRetryAfter(
if (value_ms < 0)
return;
- GetBackoffEntry()->SetCustomReleaseTime(
- GetTimeNow() + base::TimeDelta::FromMilliseconds(value_ms));
+ net_log_.AddEvent(
+ NetLog::TYPE_THROTTLING_GOT_CUSTOM_RETRY_AFTER,
+ make_scoped_refptr(new RetryAfterParameters(url_id_, value_ms)));
+
+ base::TimeDelta value = base::TimeDelta::FromMilliseconds(value_ms);
+ GetBackoffEntry()->SetCustomReleaseTime(ImplGetTimeNow() + value);
+
+ UMA_HISTOGRAM_CUSTOM_TIMES(
+ "Throttling.CustomRetryAfterMs", value,
+ base::TimeDelta::FromSeconds(1), base::TimeDelta::FromHours(12), 50);
+ if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) {
+ UMA_HISTOGRAM_CUSTOM_TIMES(
+ base::FieldTrial::MakeName("Throttling.CustomRetryAfterMs",
+ "ThrottlingEnabled"), value,
+ base::TimeDelta::FromSeconds(1), base::TimeDelta::FromHours(12), 50);
+ }
}
void URLRequestThrottlerEntry::HandleThrottlingHeader(
@@ -257,6 +387,54 @@ void URLRequestThrottlerEntry::HandleThrottlingHeader(
}
}
+void URLRequestThrottlerEntry::HandleMetricsTracking(int response_code) {
+ // This is essentially the same as the "Net.HttpResponseCode" UMA stat
+ // but we are tracking it separately here for the throttling experiment
+ // to make sure we count only the responses seen by throttling.
+ // TODO(joi): Remove after experiment.
+ UMA_HISTOGRAM_ENUMERATION("Throttling.HttpResponseCode", response_code, 600);
+ if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) {
+ UMA_HISTOGRAM_ENUMERATION(
+ base::FieldTrial::MakeName("Throttling.HttpResponseCode",
+ "ThrottlingEnabled"),
+ response_code, 600);
+ }
+
+ // Note that we are not interested in whether the code is considered
+ // an error for the backoff logic, but whether it is a 5xx error in
+ // general. This is because here, we are tracking the apparent total
+ // downtime of a server.
+ if (response_code >= 500) {
+ last_response_was_success_ = false;
+ } else {
+ base::TimeTicks now = ImplGetTimeNow();
+ if (!last_response_was_success_) {
+ // We are transitioning from failure to success, so generate our stats.
+ base::TimeDelta down_time = now - last_successful_response_time_;
+ int failure_count = GetBackoffEntry()->failure_count();
+
+ UMA_HISTOGRAM_COUNTS("Throttling.FailureCountAtSuccess", failure_count);
+ UMA_HISTOGRAM_CUSTOM_TIMES(
+ "Throttling.PerceivedDowntime", down_time,
+ base::TimeDelta::FromMilliseconds(10),
+ base::TimeDelta::FromHours(6), 50);
+
+ if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) {
+ UMA_HISTOGRAM_COUNTS(base::FieldTrial::MakeName(
+ "Throttling.FailureCountAtSuccess", "ThrottlingEnabled"),
+ failure_count);
+ UMA_HISTOGRAM_CUSTOM_TIMES(base::FieldTrial::MakeName(
+ "Throttling.PerceivedDowntime", "ThrottlingEnabled"), down_time,
+ base::TimeDelta::FromMilliseconds(10),
+ base::TimeDelta::FromHours(6), 50);
+ }
+ }
+
+ last_successful_response_time_ = now;
+ last_response_was_success_ = true;
+ }
+}
+
const BackoffEntry* URLRequestThrottlerEntry::GetBackoffEntry() const {
return &backoff_entry_;
}
diff --git a/net/url_request/url_request_throttler_entry.h b/net/url_request/url_request_throttler_entry.h
index 7cc4133..75e6aa0 100644
--- a/net/url_request/url_request_throttler_entry.h
+++ b/net/url_request/url_request_throttler_entry.h
@@ -10,7 +10,9 @@
#include <string>
#include "base/basictypes.h"
+#include "base/time.h"
#include "net/base/backoff_entry.h"
+#include "net/base/net_log.h"
#include "net/url_request/url_request_throttler_entry_interface.h"
namespace net {
@@ -69,12 +71,14 @@ class NET_API URLRequestThrottlerEntry
static const char kExponentialThrottlingDisableValue[];
// The manager object's lifetime must enclose the lifetime of this object.
- explicit URLRequestThrottlerEntry(URLRequestThrottlerManager* manager);
+ explicit URLRequestThrottlerEntry(URLRequestThrottlerManager* manager,
+ const std::string& url_id);
// The life span of instances created with this constructor is set to
// infinite, and the number of initial errors to ignore is set to 0.
// It is only used by unit tests.
URLRequestThrottlerEntry(URLRequestThrottlerManager* manager,
+ const std::string& url_id,
int sliding_window_period_ms,
int max_send_threshold,
int initial_backoff_ms,
@@ -100,15 +104,19 @@ class NET_API URLRequestThrottlerEntry
virtual void UpdateWithResponse(
const std::string& host,
const URLRequestThrottlerHeaderInterface* response);
- virtual void ReceivedContentWasMalformed();
+ virtual void ReceivedContentWasMalformed(int response_code);
protected:
virtual ~URLRequestThrottlerEntry();
void Initialize();
+ // Returns true if the given response code is considered an error for
+ // throttling purposes.
+ bool IsConsideredError(int response_code);
+
// Equivalent to TimeTicks::Now(), virtual to be mockable for testing purpose.
- virtual base::TimeTicks GetTimeNow() const;
+ virtual base::TimeTicks ImplGetTimeNow() const;
// Used internally to increase release time following a retry-after header.
void HandleCustomRetryAfter(const std::string& header_value);
@@ -117,6 +125,10 @@ class NET_API URLRequestThrottlerEntry
void HandleThrottlingHeader(const std::string& header_value,
const std::string& host);
+ // Used internally to keep track of failure->success transitions and
+ // generate statistics about them.
+ void HandleMetricsTracking(int response_code);
+
// Retrieves the back-off entry object we're using. Used to enable a
// unit testing seam for dependency injection in tests.
virtual const BackoffEntry* GetBackoffEntry() const;
@@ -154,9 +166,23 @@ class NET_API URLRequestThrottlerEntry
// Access it through GetBackoffEntry() to allow a unit test seam.
BackoffEntry backoff_entry_;
+ // The time of the last successful response, plus knowledge of whether
+ // the last response was successful or not, let us generate statistics on
+ // the length of perceived downtime for a given URL, and the error count
+ // when such transitions occur. This is useful for experiments with
+ // throttling but will likely become redundant after they are finished.
+ // TODO(joi): Remove when the time is right
+ base::TimeTicks last_successful_response_time_;
+ bool last_response_was_success_;
+
// Weak back-reference to the manager object managing us.
URLRequestThrottlerManager* manager_;
+ // Canonicalized URL string that this entry is for; used for logging only.
+ std::string url_id_;
+
+ BoundNetLog net_log_;
+
DISALLOW_COPY_AND_ASSIGN(URLRequestThrottlerEntry);
};
diff --git a/net/url_request/url_request_throttler_entry_interface.h b/net/url_request/url_request_throttler_entry_interface.h
index 259c5cf..a910e1a 100644
--- a/net/url_request/url_request_throttler_entry_interface.h
+++ b/net/url_request/url_request_throttler_entry_interface.h
@@ -49,9 +49,10 @@ class NET_API URLRequestThrottlerEntryInterface
// Lets higher-level modules, that know how to parse particular response
// bodies, notify of receiving malformed content for the given URL. This will
- // be handled by the throttler as if an HTTP 5xx response had been received to
- // the request, i.e. it will count as a failure.
- virtual void ReceivedContentWasMalformed() = 0;
+ // be handled by the throttler as if an HTTP 503 response had been received to
+ // the request, i.e. it will count as a failure, unless the HTTP response code
+ // indicated is already one of those that will be counted as an error.
+ virtual void ReceivedContentWasMalformed(int response_code) = 0;
protected:
friend class base::RefCountedThreadSafe<URLRequestThrottlerEntryInterface>;
diff --git a/net/url_request/url_request_throttler_manager.cc b/net/url_request/url_request_throttler_manager.cc
index 07c0605..e138bba 100644
--- a/net/url_request/url_request_throttler_manager.cc
+++ b/net/url_request/url_request_throttler_manager.cc
@@ -5,7 +5,10 @@
#include "net/url_request/url_request_throttler_manager.h"
#include "base/logging.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
#include "base/string_util.h"
+#include "net/base/net_log.h"
#include "net/base/net_util.h"
namespace net {
@@ -27,10 +30,20 @@ scoped_refptr<URLRequestThrottlerEntryInterface>
// Periodically garbage collect old entries.
GarbageCollectEntriesIfNecessary();
- // Find the entry in the map or create it.
+ // Find the entry in the map or create a new NULL entry.
scoped_refptr<URLRequestThrottlerEntry>& entry = url_entries_[url_id];
+
+ // If the entry exists but could be garbage collected at this point, we
+ // start with a fresh entry so that we possibly back off a bit less
+ // aggressively (i.e. this resets the error count when the entry's URL
+ // hasn't been requested in long enough).
+ if (entry.get() && entry->IsEntryOutdated()) {
+ entry = NULL;
+ }
+
+ // Create the entry if needed.
if (entry.get() == NULL) {
- entry = new URLRequestThrottlerEntry(this);
+ entry = new URLRequestThrottlerEntry(this, url_id);
// We only disable back-off throttling on an entry that we have
// just constructed. This is to allow unit tests to explicitly override
@@ -40,6 +53,13 @@ scoped_refptr<URLRequestThrottlerEntryInterface>
std::string host = url.host();
if (opt_out_hosts_.find(host) != opt_out_hosts_.end() ||
IsLocalhost(host)) {
+ if (!logged_for_localhost_disabled_ && IsLocalhost(host)) {
+ logged_for_localhost_disabled_ = true;
+ net_log_->AddEvent(
+ NetLog::TYPE_THROTTLING_DISABLED_FOR_HOST,
+ make_scoped_refptr(new NetLogStringParameter("host", host)));
+ }
+
// TODO(joi): Once sliding window is separate from back-off throttling,
// we can simply return a dummy implementation of
// URLRequestThrottlerEntryInterface here that never blocks anything (and
@@ -57,7 +77,18 @@ void URLRequestThrottlerManager::AddToOptOutList(const std::string& host) {
// after there are already one or more entries in url_entries_ for that
// host, the pre-existing entries may still perform back-off throttling.
// In practice, this would almost never occur.
- opt_out_hosts_.insert(host);
+ if (opt_out_hosts_.find(host) == opt_out_hosts_.end()) {
+ UMA_HISTOGRAM_COUNTS("Throttling.SiteOptedOut", 1);
+ if (base::FieldTrialList::TrialExists("ThrottlingEnabled")) {
+ UMA_HISTOGRAM_COUNTS(base::FieldTrial::MakeName(
+ "Throttling.SiteOptedOut", "ThrottlingEnabled"), 1);
+ }
+
+ net_log_->EndEvent(
+ NetLog::TYPE_THROTTLING_DISABLED_FOR_HOST,
+ make_scoped_refptr(new NetLogStringParameter("host", host)));
+ opt_out_hosts_.insert(host);
+ }
}
void URLRequestThrottlerManager::OverrideEntryForTests(
@@ -94,11 +125,31 @@ bool URLRequestThrottlerManager::enforce_throttling() {
return enforce_throttling_;
}
+void URLRequestThrottlerManager::set_net_log(NetLog* net_log) {
+ DCHECK(net_log);
+ NetLog::Source source(NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING,
+ net_log->NextID());
+ net_log_.reset(new BoundNetLog(source, net_log));
+}
+
+NetLog* URLRequestThrottlerManager::net_log() const {
+ return net_log_->net_log();
+}
+
+void URLRequestThrottlerManager::OnIPAddressChanged() {
+ OnNetworkChange();
+}
+
+void URLRequestThrottlerManager::OnOnlineStateChanged(bool online) {
+ OnNetworkChange();
+}
+
// TODO(joi): Turn throttling on by default when appropriate.
URLRequestThrottlerManager::URLRequestThrottlerManager()
: requests_since_last_gc_(0),
enforce_throttling_(false),
- enable_thread_checks_(false) {
+ enable_thread_checks_(false),
+ logged_for_localhost_disabled_(false) {
// Construction/destruction is on main thread (because BrowserMain
// retrieves an instance to call InitializeOptions), but is from then on
// used on I/O thread.
@@ -108,12 +159,22 @@ URLRequestThrottlerManager::URLRequestThrottlerManager()
url_id_replacements_.ClearUsername();
url_id_replacements_.ClearQuery();
url_id_replacements_.ClearRef();
+
+ // Make sure there is always a net_log_ instance, even if it logs to
+ // nowhere.
+ net_log_.reset(new BoundNetLog());
+
+ NetworkChangeNotifier::AddIPAddressObserver(this);
+ NetworkChangeNotifier::AddOnlineStateObserver(this);
}
URLRequestThrottlerManager::~URLRequestThrottlerManager() {
// Destruction is on main thread (AtExit), but real use is on I/O thread.
DetachFromThread();
+ NetworkChangeNotifier::RemoveIPAddressObserver(this);
+ NetworkChangeNotifier::RemoveOnlineStateObserver(this);
+
// Since, for now, the manager object might conceivably go away before
// the entries, detach the entries' back-pointer to the manager.
//
@@ -163,4 +224,13 @@ void URLRequestThrottlerManager::GarbageCollectEntries() {
}
}
+void URLRequestThrottlerManager::OnNetworkChange() {
+ // Remove all entries. Any entries that in-flight requests have a reference
+ // to will live until those requests end, and these entries may be
+ // inconsistent with new entries for the same URLs, but since what we
+ // want is a clean slate for the new connection state, this is OK.
+ url_entries_.clear();
+ requests_since_last_gc_ = 0;
+}
+
} // namespace net
diff --git a/net/url_request/url_request_throttler_manager.h b/net/url_request/url_request_throttler_manager.h
index 4b9b21f..846b682 100644
--- a/net/url_request/url_request_throttler_manager.h
+++ b/net/url_request/url_request_throttler_manager.h
@@ -16,10 +16,14 @@
#include "base/threading/non_thread_safe.h"
#include "googleurl/src/gurl.h"
#include "net/base/net_api.h"
+#include "net/base/network_change_notifier.h"
#include "net/url_request/url_request_throttler_entry.h"
namespace net {
+class BoundNetLog;
+class NetLog;
+
// Class that registers URL request throttler entries for URLs being accessed
// in order to supervise traffic. URL requests for HTTP contents should
// register their URLs in this manager on each request.
@@ -34,7 +38,9 @@ namespace net {
// although to allow it to be used as a singleton, construction and destruction
// can occur on a separate thread.
class NET_API URLRequestThrottlerManager
- : NON_EXPORTED_BASE(public base::NonThreadSafe) {
+ : NON_EXPORTED_BASE(public base::NonThreadSafe),
+ public NetworkChangeNotifier::IPAddressObserver,
+ public NetworkChangeNotifier::OnlineStateObserver {
public:
static URLRequestThrottlerManager* GetInstance();
@@ -71,6 +77,16 @@ class NET_API URLRequestThrottlerManager
void set_enforce_throttling(bool enforce);
bool enforce_throttling();
+ // Sets the NetLog instance to use.
+ void set_net_log(NetLog* net_log);
+ NetLog* net_log() const;
+
+ // IPAddressObserver interface.
+ virtual void OnIPAddressChanged() OVERRIDE;
+
+ // OnlineStateObserver interface.
+ virtual void OnOnlineStateChanged(bool online) OVERRIDE;
+
protected:
URLRequestThrottlerManager();
~URLRequestThrottlerManager();
@@ -90,6 +106,15 @@ class NET_API URLRequestThrottlerManager
// Method that does the actual work of garbage collecting.
void GarbageCollectEntries();
+ // When we switch from online to offline or change IP addresses, we
+ // clear all back-off history. This is a precaution in case the change in
+ // online state now lets us communicate without error with servers that
+ // we were previously getting 500 or 503 responses from (perhaps the
+ // responses are from a badly-written proxy that should have returned a
+ // 502 or 504 because it's upstream connection was down or it had no route
+ // to the server).
+ void OnNetworkChange();
+
// Used by tests.
int GetNumberOfEntriesForTests() const { return url_entries_.size(); }
@@ -136,6 +161,13 @@ class NET_API URLRequestThrottlerManager
// workaround.
bool enable_thread_checks_;
+ // Initially false, switches to true once we have logged because of back-off
+ // being disabled for localhost.
+ bool logged_for_localhost_disabled_;
+
+ // NetLog to use, or NULL if none configured.
+ scoped_ptr<BoundNetLog> net_log_;
+
DISALLOW_COPY_AND_ASSIGN(URLRequestThrottlerManager);
};
diff --git a/net/url_request/url_request_throttler_unittest.cc b/net/url_request/url_request_throttler_unittest.cc
index f6227b3..354adb9 100644
--- a/net/url_request/url_request_throttler_unittest.cc
+++ b/net/url_request/url_request_throttler_unittest.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram.h"
#include "base/pickle.h"
#include "base/stringprintf.h"
#include "base/string_number_conversions.h"
@@ -19,6 +20,10 @@ using base::TimeTicks;
namespace net {
namespace {
+
+using base::Histogram;
+using base::StatisticsRecorder;
+
class MockURLRequestThrottlerManager;
class MockBackoffEntry : public BackoffEntry {
@@ -29,7 +34,7 @@ class MockBackoffEntry : public BackoffEntry {
virtual ~MockBackoffEntry() {}
- TimeTicks GetTimeNow() const {
+ virtual TimeTicks ImplGetTimeNow() const OVERRIDE {
return fake_now_;
}
@@ -45,7 +50,7 @@ class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry {
public :
explicit MockURLRequestThrottlerEntry(
net::URLRequestThrottlerManager* manager)
- : net::URLRequestThrottlerEntry(manager),
+ : net::URLRequestThrottlerEntry(manager, ""),
mock_backoff_entry_(&backoff_policy_) {
InitPolicy();
}
@@ -54,7 +59,7 @@ class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry {
const TimeTicks& exponential_backoff_release_time,
const TimeTicks& sliding_window_release_time,
const TimeTicks& fake_now)
- : net::URLRequestThrottlerEntry(manager),
+ : net::URLRequestThrottlerEntry(manager, ""),
fake_time_now_(fake_now),
mock_backoff_entry_(&backoff_policy_) {
InitPolicy();
@@ -86,13 +91,13 @@ class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry {
fake_time_now_ = time_now;
mock_backoff_entry_.SetFakeNow(time_now);
- GetBackoffEntry()->InformOfRequest(true); // Sets failure count to 0.
+ GetBackoffEntry()->Reset();
GetBackoffEntry()->SetCustomReleaseTime(time_now);
set_sliding_window_release_time(time_now);
}
// Overridden for tests.
- virtual TimeTicks GetTimeNow() const { return fake_time_now_; }
+ virtual TimeTicks ImplGetTimeNow() const OVERRIDE { return fake_time_now_; }
void set_exponential_backoff_release_time(
const base::TimeTicks& release_time) {
@@ -219,15 +224,66 @@ struct GurlAndString {
class URLRequestThrottlerEntryTest : public testing::Test {
protected:
virtual void SetUp();
+
+ // After calling this function, histogram snapshots in |samples_| contain
+ // only the delta caused by the test case currently running.
+ void CalculateHistogramDeltas();
+
TimeTicks now_;
MockURLRequestThrottlerManager manager_; // Dummy object, not used.
scoped_refptr<MockURLRequestThrottlerEntry> entry_;
+
+ std::map<std::string, Histogram::SampleSet> original_samples_;
+ std::map<std::string, Histogram::SampleSet> samples_;
+};
+
+// List of all histograms we care about in these unit tests.
+const char* kHistogramNames[] = {
+ "Throttling.CustomRetryAfterMs",
+ "Throttling.FailureCountAtSuccess",
+ "Throttling.HttpResponseCode",
+ "Throttling.PerceivedDowntime",
+ "Throttling.RequestThrottled",
+ "Throttling.SiteOptedOut",
};
void URLRequestThrottlerEntryTest::SetUp() {
now_ = TimeTicks::Now();
entry_ = new MockURLRequestThrottlerEntry(&manager_);
entry_->ResetToBlank(now_);
+
+ for (size_t i = 0; i < arraysize(kHistogramNames); ++i) {
+ // Must retrieve original samples for each histogram for comparison
+ // as other tests may affect them.
+ const char* name = kHistogramNames[i];
+ Histogram::SampleSet& original = original_samples_[name];
+ Histogram* histogram;
+ if (StatisticsRecorder::FindHistogram(name, &histogram)) {
+ histogram->SnapshotSample(&original);
+ }
+ }
+}
+
+void URLRequestThrottlerEntryTest::CalculateHistogramDeltas() {
+ for (size_t i = 0; i < arraysize(kHistogramNames); ++i) {
+ const char* name = kHistogramNames[i];
+ Histogram::SampleSet& original = original_samples_[name];
+ Histogram::SampleSet& sample = samples_[name];
+
+ Histogram* histogram;
+ if (StatisticsRecorder::FindHistogram(name, &histogram)) {
+ ASSERT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
+
+ histogram->SnapshotSample(&sample);
+ // Ensure |original| size is same as |sample|, then subtract original
+ // values.
+ original.Resize(*histogram);
+ sample.Subtract(original);
+ }
+ }
+
+ // Ensure we don't accidentally use the originals in our tests.
+ original_samples_.clear();
}
std::ostream& operator<<(std::ostream& out, const base::TimeTicks& time) {
@@ -238,6 +294,10 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceDuringExponentialBackoff) {
entry_->set_exponential_backoff_release_time(
entry_->fake_time_now_ + TimeDelta::FromMilliseconds(1));
EXPECT_TRUE(entry_->IsDuringExponentialBackoff());
+
+ CalculateHistogramDeltas();
+ ASSERT_EQ(0, samples_["Throttling.RequestThrottled"].counts(0));
+ ASSERT_EQ(1, samples_["Throttling.RequestThrottled"].counts(1));
}
TEST_F(URLRequestThrottlerEntryTest, InterfaceNotDuringExponentialBackoff) {
@@ -246,6 +306,10 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceNotDuringExponentialBackoff) {
entry_->set_exponential_backoff_release_time(
entry_->fake_time_now_ - TimeDelta::FromMilliseconds(1));
EXPECT_FALSE(entry_->IsDuringExponentialBackoff());
+
+ CalculateHistogramDeltas();
+ ASSERT_EQ(2, samples_["Throttling.RequestThrottled"].counts(0));
+ ASSERT_EQ(0, samples_["Throttling.RequestThrottled"].counts(1));
}
TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateRetryAfter) {
@@ -258,13 +322,18 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateRetryAfter) {
"increase release_time";
entry_->ResetToBlank(now_);
- header_w_delay_header.fake_retry_value_ = "-5.5";
+ MockURLRequestThrottlerHeaderAdapter header_w_negative_header(
+ "-5.5", "", 200);
+ entry_->UpdateWithResponse("", &header_w_negative_header);
EXPECT_EQ(entry_->GetExponentialBackoffReleaseTime(), entry_->fake_time_now_)
<< "When given a negative value, it should not change the release_time";
+
+ CalculateHistogramDeltas();
+ ASSERT_EQ(1, samples_["Throttling.CustomRetryAfterMs"].TotalCount());
}
TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateFailure) {
- MockURLRequestThrottlerHeaderAdapter failure_response(505);
+ MockURLRequestThrottlerHeaderAdapter failure_response(503);
entry_->UpdateWithResponse("", &failure_response);
EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), entry_->fake_time_now_)
<< "A failure should increase the release_time";
@@ -278,12 +347,19 @@ TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateSuccess) {
}
TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateSuccessThenFailure) {
- MockURLRequestThrottlerHeaderAdapter failure_response(500);
+ MockURLRequestThrottlerHeaderAdapter failure_response(503);
MockURLRequestThrottlerHeaderAdapter success_response(200);
entry_->UpdateWithResponse("", &success_response);
entry_->UpdateWithResponse("", &failure_response);
EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), entry_->fake_time_now_)
<< "This scenario should add delay";
+ entry_->UpdateWithResponse("", &success_response);
+
+ CalculateHistogramDeltas();
+ ASSERT_EQ(1, samples_["Throttling.HttpResponseCode"].counts(503));
+ ASSERT_EQ(2, samples_["Throttling.HttpResponseCode"].counts(200));
+ ASSERT_EQ(1, samples_["Throttling.FailureCountAtSuccess"].counts(1));
+ ASSERT_EQ(1, samples_["Throttling.PerceivedDowntime"].TotalCount());
}
TEST_F(URLRequestThrottlerEntryTest, IsEntryReallyOutdated) {
@@ -308,7 +384,7 @@ TEST_F(URLRequestThrottlerEntryTest, IsEntryReallyOutdated) {
TEST_F(URLRequestThrottlerEntryTest, MaxAllowedBackoff) {
for (int i = 0; i < 30; ++i) {
- MockURLRequestThrottlerHeaderAdapter response_adapter(505);
+ MockURLRequestThrottlerHeaderAdapter response_adapter(503);
entry_->UpdateWithResponse("", &response_adapter);
}
@@ -318,7 +394,7 @@ TEST_F(URLRequestThrottlerEntryTest, MaxAllowedBackoff) {
}
TEST_F(URLRequestThrottlerEntryTest, MalformedContent) {
- MockURLRequestThrottlerHeaderAdapter response_adapter(505);
+ MockURLRequestThrottlerHeaderAdapter response_adapter(503);
for (int i = 0; i < 5; ++i)
entry_->UpdateWithResponse("", &response_adapter);
@@ -329,7 +405,7 @@ TEST_F(URLRequestThrottlerEntryTest, MalformedContent) {
// UpdateWithResponse to pair with ReceivedContentWasMalformed() since that
// is what happens in practice (if a body is received, then a non-500
// response must also have been received).
- entry_->ReceivedContentWasMalformed();
+ entry_->ReceivedContentWasMalformed(200);
MockURLRequestThrottlerHeaderAdapter success_adapter(200);
entry_->UpdateWithResponse("", &success_adapter);
EXPECT_GT(entry_->GetExponentialBackoffReleaseTime(), release_after_failures);
@@ -386,6 +462,9 @@ TEST(URLRequestThrottlerManager, IsUrlStandardised) {
GurlAndString(GURL("http://www.example.com/index.php?code=1#superEntry"),
std::string("http://www.example.com/index.php"),
__LINE__),
+ GurlAndString(GURL("http://www.example.com/index.php#superEntry"),
+ std::string("http://www.example.com/index.php"),
+ __LINE__),
GurlAndString(GURL("http://www.example.com:1234/"),
std::string("http://www.example.com:1234/"),
__LINE__)};
@@ -485,4 +564,36 @@ TEST(URLRequestThrottlerManager, OptOutHeader) {
ExpectEntryAllowsAllOnErrorIfOptedOut(localhost_entry, true);
}
+TEST(URLRequestThrottlerManager, ClearOnNetworkChange) {
+ for (int i = 0; i < 3; ++i) {
+ MockURLRequestThrottlerManager manager;
+ scoped_refptr<net::URLRequestThrottlerEntryInterface> entry_before =
+ manager.RegisterRequestUrl(GURL("http://www.example.com/"));
+ MockURLRequestThrottlerHeaderAdapter failure_adapter(503);
+ for (int j = 0; j < 10; ++j) {
+ // Host doesn't really matter in this scenario so we skip it.
+ entry_before->UpdateWithResponse("", &failure_adapter);
+ }
+ EXPECT_TRUE(entry_before->IsDuringExponentialBackoff());
+
+ switch (i) {
+ case 0:
+ manager.OnIPAddressChanged();
+ break;
+ case 1:
+ manager.OnOnlineStateChanged(true);
+ break;
+ case 2:
+ manager.OnOnlineStateChanged(false);
+ break;
+ default:
+ FAIL();
+ }
+
+ scoped_refptr<net::URLRequestThrottlerEntryInterface> entry_after =
+ manager.RegisterRequestUrl(GURL("http://www.example.com/"));
+ EXPECT_FALSE(entry_after->IsDuringExponentialBackoff());
+ }
+}
+
} // namespace net