Bug 1191312 - Stop retrying daily pings in TelemetrySession scheduler. r=gfritzsche
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Thu, 06 Aug 2015 02:55:00 +0200
changeset 288449 b6583df2c867c13734d38bc1176fd1d870e98451
parent 288448 8ad569af634d9603a93173e180c6e2fdbd004060
child 288450 db20ce0ea851d630d2975bbfb9adb10a40a0b054
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1191312
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1191312 - Stop retrying daily pings in TelemetrySession scheduler. r=gfritzsche
toolkit/components/telemetry/TelemetrySession.jsm
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -80,18 +80,16 @@ const TELEMETRY_INTERVAL = 60000;
 // Delay before intializing telemetry (ms)
 const TELEMETRY_DELAY = 60000;
 // Delay before initializing telemetry if we're testing (ms)
 const TELEMETRY_TEST_DELAY = 100;
 // Execute a scheduler tick every 5 minutes.
 const SCHEDULER_TICK_INTERVAL_MS = 5 * 60 * 1000;
 // When user is idle, execute a scheduler tick every 60 minutes.
 const SCHEDULER_TICK_IDLE_INTERVAL_MS = 60 * 60 * 1000;
-// The maximum number of times a scheduled operation can fail.
-const SCHEDULER_RETRY_ATTEMPTS = 3;
 
 // The tolerance we have when checking if it's midnight (15 minutes).
 const SCHEDULER_MIDNIGHT_TOLERANCE_MS = 15 * 60 * 1000;
 
 // Coalesce the daily and aborted-session pings if they are both due within
 // two minutes from each other.
 const SCHEDULER_COALESCE_THRESHOLD_MS = 2 * 60 * 1000;
 
@@ -376,19 +374,16 @@ let TelemetryScheduler = {
   _lastSessionCheckpointTime: 0,
 
   // For sanity checking.
   _lastAdhocPingTime: 0,
   _lastTickTime: 0,
 
   _log: null,
 
-  // The number of times a daily ping fails.
-  _dailyPingRetryAttempts: 0,
-
   // The timer which drives the scheduler.
   _schedulerTimer: null,
   // The interval used by the scheduler timer.
   _schedulerInterval: 0,
   _shuttingDown: true,
   _isUserIdle: false,
 
   /**
@@ -541,35 +536,31 @@ let TelemetryScheduler = {
     }
     this._lastTickTime = now;
 
     // Check if the daily ping is due.
     const shouldSendDaily = this._isDailyPingDue(nowDate);
 
     if (shouldSendDaily) {
       this._log.trace("_schedulerTickLogic - Daily ping due.");
-      return Impl._sendDailyPing().then(() => this._dailyPingSucceeded(now),
-                                        () => this._dailyPingFailed(now));
+      this._lastDailyPingTime = now;
+      return Impl._sendDailyPing();
     }
 
     // Check if the aborted-session ping is due. If a daily ping was saved above, it was
     // already duplicated as an aborted-session ping.
     const isAbortedPingDue =
       (now - this._lastSessionCheckpointTime) >= ABORTED_SESSION_UPDATE_INTERVAL_MS;
     if (isAbortedPingDue) {
       this._log.trace("_schedulerTickLogic - Aborted session ping due.");
       return this._saveAbortedPing(now);
     }
 
     // No ping is due.
     this._log.trace("_schedulerTickLogic - No ping due.");
-    // It's possible, because of sleeps, that we're no longer within midnight tolerance for
-    // daily pings. Because of that, daily retry attempts would not be 0 on the next midnight.
-    // Reset that count on do-nothing ticks.
-    this._dailyPingRetryAttempts = 0;
     return Promise.resolve();
   },
 
   /**
    * Update the scheduled pings if some other ping was sent.
    * @param {String} reason The reason of the ping that was sent.
    * @param {Object} [competingPayload=null] The payload of the ping that was sent. The
    *                 reason of this payload will be changed.
@@ -593,43 +584,16 @@ let TelemetryScheduler = {
         this._lastDailyPingTime = now.getTime();
       }
     }
 
     this._rescheduleTimeout();
   },
 
   /**
-   * Called when a scheduled operation successfully completes (ping sent or saved).
-   * @param {Number} now The current time, in milliseconds.
-   */
-  _dailyPingSucceeded: function(now) {
-    this._log.trace("_dailyPingSucceeded");
-    this._lastDailyPingTime = now;
-    this._dailyPingRetryAttempts = 0;
-  },
-
-  /**
-   * Called when a scheduled operation fails (ping sent or saved).
-   * @param {Number} now The current time, in milliseconds.
-   */
-  _dailyPingFailed: function(now) {
-    this._log.error("_dailyPingFailed");
-    this._dailyPingRetryAttempts++;
-
-    // If we reach the maximum number of retry attempts for a daily ping, log the error
-    // and skip this daily ping.
-    if (this._dailyPingRetryAttempts >= SCHEDULER_RETRY_ATTEMPTS) {
-      this._log.error("_pingFailed - The daily ping failed too many times. Skipping it.");
-      this._dailyPingRetryAttempts = 0;
-      this._lastDailyPingTime = now;
-    }
-  },
-
-  /**
    * Stops the scheduler.
    */
   shutdown: function() {
     if (this._shuttingDown) {
       if (this._log) {
         this._log.error("shutdown - Already shut down");
       } else {
         Cu.reportError("TelemetryScheduler.shutdown - Already shut down");
@@ -1889,18 +1853,18 @@ let Impl = {
       addEnvironment: true,
     };
 
     let promise = TelemetryController.submitExternalPing(getPingType(payload), payload, options);
 
     // Also save the payload as an aborted session. If we delay this, aborted-session can
     // lag behind for the profileSubsessionCounter and other state, complicating analysis.
     if (IS_UNIFIED_TELEMETRY) {
-      let abortedPromise = this._saveAbortedSessionPing(payload);
-      promise = promise.then(() => abortedPromise);
+      this._saveAbortedSessionPing(payload)
+          .catch(e => this._log.error("_sendDailyPing - Failed to save the aborted session ping", e));
     }
 
     return promise;
   },
 
   /**
    * Loads session data from the session data file.
    * @return {Promise<boolean>} A promise which is resolved with a true argument when