Bug 1539166 - Simplify daily ping reschedule r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Fri, 05 Apr 2019 15:48:54 +0000
changeset 468191 50ada71796e43e3b4c7e221687060f25539aa692
parent 468190 19caa11e5eb7ab6d11ad24a998ff4de4d628c95e
child 468192 5bac63a7f0219d47130e16c0691764f22cf995f7
push id35822
push usershindli@mozilla.com
push dateFri, 05 Apr 2019 21:47:45 +0000
treeherdermozilla-central@98064c475d2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1539166
milestone68.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 1539166 - Simplify daily ping reschedule r=chutten This is only called on environment-changed pings. We avoid re-setting the scheduler timeout, so that we can trigger other pings at regular intervals unrelated to main pings. This will not cause the daily ping to be sent more often, at worst we schedule something once too frequently. Depends on D26149 Differential Revision: https://phabricator.services.mozilla.com/D26150
toolkit/components/telemetry/app/TelemetryScheduler.jsm
toolkit/components/telemetry/pings/TelemetrySession.jsm
--- a/toolkit/components/telemetry/app/TelemetryScheduler.jsm
+++ b/toolkit/components/telemetry/app/TelemetryScheduler.jsm
@@ -14,18 +14,16 @@ const {TelemetrySession} = ChromeUtils.i
 const {TelemetryUtils} = ChromeUtils.import("resource://gre/modules/TelemetryUtils.jsm");
 const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm", this);
 const {clearTimeout, setTimeout} = ChromeUtils.import("resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyServiceGetters(this, {
   idleService: ["@mozilla.org/widget/idleservice;1", "nsIIdleService"],
 });
 
-const REASON_ENVIRONMENT_CHANGE = "environment-change";
-
 const MIN_SUBSESSION_LENGTH_MS = Services.prefs.getIntPref("toolkit.telemetry.minSubsessionLength", 5 * 60) * 1000;
 
 const LOGGER_NAME = "Toolkit.Telemetry";
 
 // Seconds of idle time before pinging.
 // On idle-daily a gather-telemetry notification is fired, during it probes can
 // start asynchronous tasks to gather data.
 const IDLE_TIMEOUT_SECONDS = Services.prefs.getIntPref("toolkit.telemetry.idleTimeout", 5 * 60);
@@ -326,34 +324,36 @@ var TelemetryScheduler = {
     }
 
     // No ping is due.
     this._log.trace("_schedulerTickLogic - No ping due.");
     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.
+   * Re-schedule the daily ping if some other equivalent ping was sent.
+   *
+   * This is only called from TelemetrySession when a main ping with reason 'environment-change'
+   * is sent.
+   *
+   * @param {Object} [payload] The payload of the ping that was sent,
+   *                           to be stored as an aborted-session ping.
    */
-  reschedulePings(reason, competingPayload = null) {
+  rescheduleDailyPing(payload) {
     if (this._shuttingDown) {
-      this._log.error("reschedulePings - already shutdown");
+      this._log.error("rescheduleDailyPing - already shutdown");
       return;
     }
 
-    this._log.trace("reschedulePings - reason: " + reason);
+    this._log.trace("rescheduleDailyPing");
     let now = Policy.now();
-    if (reason == REASON_ENVIRONMENT_CHANGE) {
-      // We just generated an environment-changed ping, save it as an aborted session and
-      // update the schedules.
-      this._saveAbortedPing(now.getTime(), competingPayload);
-      // If we're close to midnight, skip today's daily ping and reschedule it for tomorrow.
-      let nearestMidnight = TelemetryUtils.getNearestMidnight(now, SCHEDULER_MIDNIGHT_TOLERANCE_MS);
-      if (nearestMidnight) {
-        this._lastDailyPingTime = now.getTime();
-      }
+
+    // We just generated an environment-changed ping, save it as an aborted session and
+    // update the schedules.
+    this._saveAbortedPing(now.getTime(), payload);
+
+    // If we're close to midnight, skip today's daily ping and reschedule it for tomorrow.
+    let nearestMidnight = TelemetryUtils.getNearestMidnight(now, SCHEDULER_MIDNIGHT_TOLERANCE_MS);
+    if (nearestMidnight) {
+      this._lastDailyPingTime = now.getTime();
     }
-
-    this._rescheduleTimeout();
   },
 };
--- a/toolkit/components/telemetry/pings/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/pings/TelemetrySession.jsm
@@ -1174,17 +1174,17 @@ var Impl = {
     let timeDelta = now - this._lastEnvironmentChangeDate;
     if (timeDelta <= MIN_SUBSESSION_LENGTH_MS) {
       this._log.trace(`_onEnvironmentChange - throttling; last change was ${Math.round(timeDelta / 1000)}s ago.`);
       return;
     }
 
     this._lastEnvironmentChangeDate = now;
     let payload = this.getSessionPayload(REASON_ENVIRONMENT_CHANGE, true);
-    TelemetryScheduler.reschedulePings(REASON_ENVIRONMENT_CHANGE, payload);
+    TelemetryScheduler.rescheduleDailyPing(payload);
 
     let options = {
       addClientId: true,
       addEnvironment: true,
       overrideEnvironment: oldEnvironment,
     };
     TelemetryController.submitExternalPing(getPingType(payload), payload, options);
   },