diff options
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 |