Domain Reliability: Switch to BackoffEntry in Scheduler

components/domain_reliability/scheduler.cc

Index: components/domain_reliability/scheduler.cc
diff --git a/components/domain_reliability/scheduler.cc b/components/domain_reliability/scheduler.cc
index 636130c9eb0f9a4fdcf16caa1c4bff3d4a04e997..0ab4f3ba8822974a676d309fd4ea83563c5502ea 100644
--- a/components/domain_reliability/scheduler.cc
+++ b/components/domain_reliability/scheduler.cc
@@ -69,7 +69,6 @@ DomainReliabilityScheduler::DomainReliabilityScheduler(
     const Params& params,
     const ScheduleUploadCallback& callback)
     : time_(time),
-      collectors_(num_collectors),
       params_(params),
       callback_(callback),
       upload_pending_(false),
@@ -77,6 +76,19 @@ DomainReliabilityScheduler::DomainReliabilityScheduler(
       upload_running_(false),
       collector_index_(kInvalidCollectorIndex),
       last_upload_finished_(false) {
+  backoff_policy_.num_errors_to_ignore = 0;
+  backoff_policy_.initial_delay_ms =
+      params.upload_retry_interval.InMilliseconds();
+  backoff_policy_.multiply_factor = 2.0;
+  backoff_policy_.jitter_factor = 0.1;
+  backoff_policy_.maximum_backoff_ms = 60 * 60 * 1000;
+  backoff_policy_.entry_lifetime_ms = 60 * 60 * 1000;

[davidben, 2014/11/06 22:39:36]

Nit: maybe pull these up into constants.

[Deprecated (see juliatuttle), 2014/11/07 19:45:09]

Done.

+  backoff_policy_.always_use_initial_delay = false;
+
+  for (size_t i = 0; i < num_collectors; ++i) {
+    collectors_.push_back(
+      new MockableTimeBackoffEntry(&backoff_policy_, time_));
+  }
 }
 
 DomainReliabilityScheduler::~DomainReliabilityScheduler() {}
@@ -116,31 +128,21 @@ void DomainReliabilityScheduler::OnUploadComplete(bool success) {
   VLOG(1) << "Upload to collector " << collector_index_
           << (success ? " succeeded." : " failed.");
 
-  CollectorState* collector = &collectors_[collector_index_];
+  net::BackoffEntry* backoff = collectors_[collector_index_];
   collector_index_ = kInvalidCollectorIndex;
+  backoff->InformOfRequest(success);
 
-  if (success) {
-    collector->failures = 0;
-  } else {
+  if (!success) {
     // Restore upload_pending_ and first_beacon_time_ to pre-upload state,
     // since upload failed.
     upload_pending_ = true;
     first_beacon_time_ = old_first_beacon_time_;
-
-    ++collector->failures;
   }
 
-  base::TimeTicks now = time_->NowTicks();
-  base::TimeDelta retry_interval = GetUploadRetryInterval(collector->failures);
-  collector->next_upload = now + retry_interval;
-
-  last_upload_end_time_ = now;
+  last_upload_end_time_ = time_->NowTicks();
   last_upload_success_ = success;
   last_upload_finished_ = true;
 
-  VLOG(1) << "Next upload to collector at least "
-          << retry_interval.InSeconds() << " seconds from now.";
-
   MaybeScheduleUpload();
 }
 
@@ -168,20 +170,22 @@ base::Value* DomainReliabilityScheduler::GetWebUIData() const {
     data->Set("last_upload", last);
   }
 
-  base::ListValue* collectors = new base::ListValue();
+  /* base::ListValue* collectors = new base::ListValue();
   for (size_t i = 0; i < collectors_.size(); ++i) {
-    const CollectorState* state = &collectors_[i];
+    const CollectorState* state = collectors_[i];
     base::DictionaryValue* value = new base::DictionaryValue();
     value->SetInteger("failures", state->failures);
     value->SetInteger("next_upload", (state->next_upload - now).InSeconds());
     collectors->Append(value);
   }
-  data->Set("collectors", collectors);
+  data->Set("collectors", collectors); */

[davidben, 2014/11/06 22:39:36]

Did you mean to delete this code or revise it?

[Deprecated (see juliatuttle), 2014/11/07 19:45:09]

Revise; it turns out it's fairly simple, so done.

 
   return data;
 }
 
-DomainReliabilityScheduler::CollectorState::CollectorState() : failures(0) {}
+void DomainReliabilityScheduler::MakeDeterministicForTesting() {
+  backoff_policy_.jitter_factor = 0.0;
+}
 
 void DomainReliabilityScheduler::MaybeScheduleUpload() {
   if (!upload_pending_ || upload_scheduled_ || upload_running_)
@@ -228,16 +232,18 @@ void DomainReliabilityScheduler::GetNextUploadTimeAndCollector(
   size_t min_index = kInvalidCollectorIndex;
 
   for (size_t i = 0; i < collectors_.size(); ++i) {
-    CollectorState* collector = &collectors_[i];
+    net::BackoffEntry* backoff = collectors_[i];
     // If a collector is usable, use the first one in the list.
-    if (collector->failures == 0 || collector->next_upload <= now) {
+    if (!backoff->ShouldRejectRequest()) {
       min_time = now;
       min_index = i;
       break;
+    }
+
     // If not, keep track of which will be usable soonest:
-    } else if (min_index == kInvalidCollectorIndex ||
-        collector->next_upload < min_time) {
-      min_time = collector->next_upload;
+    base::TimeTicks time = backoff->GetReleaseTime();
+    if (min_index == kInvalidCollectorIndex || time < min_time) {
+      min_time = time;
       min_index = i;
     }
   }
@@ -247,16 +253,4 @@ void DomainReliabilityScheduler::GetNextUploadTimeAndCollector(
   *collector_index_out = min_index;
 }
 
-base::TimeDelta DomainReliabilityScheduler::GetUploadRetryInterval(
-    unsigned failures) {
-  if (failures == 0)
-    return base::TimeDelta::FromSeconds(0);
-  else {
-    // Don't back off more than 64x the original delay.
-    if (failures > 7)
-      failures = 7;
-    return params_.upload_retry_interval * (1 << (failures - 1));
-  }
-}
-
 }  // namespace domain_reliability