Backed out changesets ade5347d0627, 0b9529b44e23, a139d273f07b, and 0994ae599f13 (bug 1139460) for widespread mochitest crashes.
authorRyan VanderMeulen <ryanvm@gmail.com>
Tue, 24 Mar 2015 14:31:55 -0400
changeset 265735 a18c01416b7f14d4d582e7a4094a2398c43239ed
parent 265734 4944dc94d987f000842733919d9ed1cb29e18c7f
child 265736 6e463e53546badf11d118dd8081bec9a5b07d591
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1139460
milestone39.0a1
backs outade5347d06278866c7167e862adce26d9abff3cc
0b9529b44e2348ecd8b4bcd2833f820186007492
a139d273f07b881f0be4499d806792fc8ed7387f
0994ae599f13a81f5159d620e09a142de0d6a4f0
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
Backed out changesets ade5347d0627, 0b9529b44e23, a139d273f07b, and 0994ae599f13 (bug 1139460) for widespread mochitest crashes. CLOSED TREE
browser/experiments/Experiments.jsm
toolkit/components/telemetry/TelemetryPing.jsm
toolkit/components/telemetry/docs/pings.rst
--- a/browser/experiments/Experiments.jsm
+++ b/browser/experiments/Experiments.jsm
@@ -134,16 +134,38 @@ function configureLogging() {
     } else {
       gLogger.removeAppender(gLogAppenderDump);
       gLogAppenderDump = null;
     }
     gLogDumping = logDumping;
   }
 }
 
+// Takes an array of promises and returns a promise that is resolved once all of
+// them are rejected or resolved.
+function allResolvedOrRejected(promises) {
+  if (!promises.length) {
+    return Promise.resolve([]);
+  }
+
+  let countdown = promises.length;
+  let deferred = Promise.defer();
+
+  for (let p of promises) {
+    let helper = () => {
+      if (--countdown == 0) {
+        deferred.resolve();
+      }
+    };
+    Promise.resolve(p).then(helper, helper);
+  }
+
+  return deferred.promise;
+}
+
 // Loads a JSON file using OS.file. file is a string representing the path
 // of the file to be read, options contains additional options to pass to
 // OS.File.read.
 // Returns a Promise resolved with the json payload or rejected with
 // OS.File.Error or JSON.parse() errors.
 function loadJSONAsync(file, options) {
   return Task.spawn(function() {
     let rawData = yield OS.File.read(file, options);
--- a/toolkit/components/telemetry/TelemetryPing.jsm
+++ b/toolkit/components/telemetry/TelemetryPing.jsm
@@ -4,26 +4,23 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cr = Components.results;
 const Cu = Components.utils;
-const myScope = this;
 
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://gre/modules/debug.js", this);
 Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/osfile.jsm", this);
 Cu.import("resource://gre/modules/Promise.jsm", this);
-Cu.import("resource://gre/modules/PromiseUtils.jsm", this);
-Cu.import("resource://gre/modules/Task.jsm", this);
 Cu.import("resource://gre/modules/DeferredTask.jsm", this);
 Cu.import("resource://gre/modules/Preferences.jsm");
 
 const LOGGER_NAME = "Toolkit.Telemetry";
 const LOGGER_PREFIX = "TelemetryPing::";
 
 const PREF_BRANCH = "toolkit.telemetry.";
 const PREF_BRANCH_LOG = PREF_BRANCH + "log.";
@@ -37,18 +34,16 @@ const PREF_FHR_UPLOAD_ENABLED = "datarep
 const PING_FORMAT_VERSION = 4;
 
 // Delay before intializing telemetry (ms)
 const TELEMETRY_DELAY = 60000;
 // Delay before initializing telemetry if we're testing (ms)
 const TELEMETRY_TEST_DELAY = 100;
 // The number of days to keep pings serialised on the disk in case of failures.
 const DEFAULT_RETENTION_DAYS = 14;
-// Timeout after which we consider a ping submission failed.
-const PING_SUBMIT_TIMEOUT_MS = 2 * 60 * 1000;
 
 XPCOMUtils.defineLazyServiceGetter(this, "Telemetry",
                                    "@mozilla.org/base/telemetry;1",
                                    "nsITelemetry");
 XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
                                   "resource://gre/modules/AsyncShutdown.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryFile",
                                   "resource://gre/modules/TelemetryFile.jsm");
@@ -266,22 +261,17 @@ let Impl = {
   // Undefined if this is not the first run, or the previous build ID is unknown.
   _previousBuildID: undefined,
   _clientID: null,
   // A task performing delayed initialization
   _delayedInitTask: null,
   // The deferred promise resolved when the initialization task completes.
   _delayedInitTaskDeferred: null,
 
-  // This is a public barrier Telemetry clients can use to add blockers to the shutdown
-  // of TelemetryPing.
-  // After this barrier, clients can not submit Telemetry pings anymore.
   _shutdownBarrier: new AsyncShutdown.Barrier("TelemetryPing: Waiting for clients."),
-  // This is a private barrier blocked by pending async ping activity (sending & saving).
-  _connectionsBarrier: new AsyncShutdown.Barrier("TelemetryPing: Waiting for pending ping activity"),
 
   /**
    * Get the data for the "application" section of the ping.
    */
   _getApplicationSection: function() {
     // Querying architecture and update channel can throw. Make sure to recover and null
     // those fields.
     let arch = null;
@@ -322,21 +312,16 @@ let Impl = {
    *                  environment data.
    *
    * @returns Promise<Object> A promise that resolves when the ping is completely assembled.
    */
   assemblePing: function assemblePing(aType, aPayload, aOptions = {}) {
     this._log.trace("assemblePing - Type " + aType + ", Server " + this._server +
                     ", aOptions " + JSON.stringify(aOptions));
 
-    // Clone the payload data so we don't race against unexpected changes in subobjects that are
-    // still referenced by other code.
-    // We can't trust all callers to do this properly on their own.
-    let payload = Cu.cloneInto(aPayload, myScope);
-
     // Fill the common ping fields.
     let pingData = {
       type: aType,
       id: generateUUID(),
       creationDate: (new Date()).toISOString(),
       version: PING_FORMAT_VERSION,
       application: this._getApplicationSection(),
       payload: aPayload,
@@ -375,24 +360,16 @@ let Impl = {
   /**
    * Only used in tests.
    */
   setServer: function (aServer) {
     this._server = aServer;
   },
 
   /**
-   * Track any pending ping send and save tasks through the promise passed here.
-   * This is needed to block shutdown on any outstanding ping activity.
-   */
-  _trackPendingPingTask: function (aPromise) {
-    this._connectionsBarrier.client.addBlocker("Waiting for ping task", aPromise);
-  },
-
-  /**
    * Adds a ping to the pending ping list by moving it to the saved pings directory
    * and adding it to the pending ping list.
    *
    * @param {String} aPingPath The path of the ping to add to the pending ping list.
    * @param {Boolean} [aRemoveOriginal] If true, deletes the ping at aPingPath after adding
    *                  it to the saved pings directory.
    * @return {Promise} Resolved when the ping is correctly moved to the saved pings directory.
    */
@@ -419,49 +396,38 @@ let Impl = {
    *                  environment data.
    *
    * @returns {Promise} A promise that resolves when the ping is sent.
    */
   send: function send(aType, aPayload, aOptions) {
     this._log.trace("send - Type " + aType + ", Server " + this._server +
                     ", aOptions " + JSON.stringify(aOptions));
 
-    let promise = this.assemblePing(aType, aPayload, aOptions)
+    return this.assemblePing(aType, aPayload, aOptions)
         .then(pingData => {
           // Once ping is assembled, send it along with the persisted ping in the backlog.
           let p = [
             // Persist the ping if sending it fails.
             this.doPing(pingData, false)
                 .catch(() => TelemetryFile.savePing(pingData, true)),
             this.sendPersistedPings(),
           ];
           return Promise.all(p);
         },
         error => this._log.error("send - Rejection", error));
-
-    this._trackPendingPingTask(promise);
-    return promise;
   },
 
   /**
    * Send the persisted pings to the server.
-   *
-   * @return Promise A promise that is resolved when all pings finished sending or failed.
    */
   sendPersistedPings: function sendPersistedPings() {
     this._log.trace("sendPersistedPings");
-
     let pingsIterator = Iterator(this.popPayloads());
-    let p = [for (data of pingsIterator) this.doPing(data, true).catch((e) => {
-      this._log.error("sendPersistedPings - doPing rejected", e);
-    })];
-
-    let promise = Promise.all(p);
-    this._trackPendingPingTask(promise);
-    return promise;
+    let p = [data for (data in pingsIterator)].map(data => this.doPing(data, true));
+    return Promise.all(p);
   },
 
   /**
    * Saves all the pending pings, plus the passed one, to disk.
    *
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} aOptions Options object.
@@ -513,18 +479,18 @@ let Impl = {
                               .then(() => { return pingData.id; });
         } else {
           return TelemetryFile.savePing(pingData, aOptions.overwrite)
                               .then(() => { return pingData.id; });
         }
       }, error => this._log.error("savePing - Rejection", error));
   },
 
-  onPingRequestFinished: function(success, startTime, ping, isPersisted) {
-    this._log.trace("onPingRequestFinished - success: " + success + ", persisted: " + isPersisted);
+  finishPingRequest: function finishPingRequest(success, startTime, ping, isPersisted) {
+    this._log.trace("finishPingRequest - Success " + success + ", Persisted " + isPersisted);
 
     let hping = Telemetry.getHistogramById("TELEMETRY_PING");
     let hsuccess = Telemetry.getHistogramById("TELEMETRY_SUCCESS");
 
     hsuccess.add(success);
     hping.add(new Date() - startTime);
 
     if (success && isPersisted) {
@@ -564,100 +530,63 @@ let Impl = {
     }
 
     let slug = pathComponents.join("/");
     return "/submit/telemetry/" + slug;
   },
 
   doPing: function doPing(ping, isPersisted) {
     this._log.trace("doPing - Server " + this._server + ", Persisted " + isPersisted);
-
-    const isNewPing = isNewPingFormat(ping);
-    const version = isNewPing ? PING_FORMAT_VERSION : 1;
-    const url = this._server + this.submissionPath(ping) + "?v=" + version;
-
+    let deferred = Promise.defer();
+    let isNewPing = isNewPingFormat(ping);
+    let version = isNewPing ? PING_FORMAT_VERSION : 1;
+    let url = this._server + this.submissionPath(ping) + "?v=" + version;
     let request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
                   .createInstance(Ci.nsIXMLHttpRequest);
     request.mozBackgroundRequest = true;
-    request.timeout = PING_SUBMIT_TIMEOUT_MS;
-
     request.open("POST", url, true);
     request.overrideMimeType("text/plain");
     request.setRequestHeader("Content-Type", "application/json; charset=UTF-8");
 
     let startTime = new Date();
-    let deferred = PromiseUtils.defer();
 
-    let onRequestFinished = (success, event) => {
-      let onCompletion = () => {
+    function handler(success) {
+      let handleCompletion = event => {
         if (success) {
           deferred.resolve();
         } else {
           deferred.reject(event);
         }
       };
 
-      this.onPingRequestFinished(success, startTime, ping, isPersisted)
-        .then(() => onCompletion(),
-              (error) => {
-                this._log.error("doPing - request success: " + success + ", error" + error);
-                onCompletion();
-              });
-    };
-
-    let errorhandler = (event) => {
-      this._log.error("doPing - error making request to " + url + ": " + event.type);
-      onRequestFinished(false, event);
-    };
-    request.onerror = errorhandler;
-    request.ontimeout = errorhandler;
-    request.onabort = errorhandler;
-
-    request.onload = (event) => {
-      let status = request.status;
-      let statusClass = status - (status % 100);
-      let success = false;
-
-      if (statusClass === 200) {
-        // We can treat all 2XX as success.
-        this._log.info("doPing - successfully loaded, status: " + status);
-        success = true;
-      } else if (statusClass === 400) {
-        // 4XX means that something with the request was broken.
-        this._log.error("doPing - error submitting to " + url + ", status: " + status
-                        + " - ping request broken?");
-        // TODO: we should handle this better, but for now we should avoid resubmitting
-        // broken requests by pretending success.
-        success = true;
-      } else if (statusClass === 500) {
-        // 5XX means there was a server-side error and we should try again later.
-        this._log.error("doPing - error submitting to " + url + ", status: " + status
-                        + " - server error, should retry later");
-      } else {
-        // We received an unexpected status codes.
-        this._log.error("doPing - error submitting to " + url + ", status: " + status
-                        + ", type: " + event.type);
-      }
-
-      onRequestFinished(success, event);
-    };
+      return function(event) {
+        this.finishPingRequest(success, startTime, ping, isPersisted)
+          .then(() => handleCompletion(event),
+                error => {
+                  this._log.error("doPing - Request Success " + success + ", Error " +
+                                  error);
+                  handleCompletion(event);
+                });
+      };
+    }
+    request.addEventListener("error", handler(false).bind(this), false);
+    request.addEventListener("load", handler(true).bind(this), false);
 
     // If that's a legacy ping format, just send its payload.
     let networkPayload = isNewPing ? ping : ping.payload;
     request.setRequestHeader("Content-Encoding", "gzip");
     let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
                     .createInstance(Ci.nsIScriptableUnicodeConverter);
     converter.charset = "UTF-8";
     let utf8Payload = converter.ConvertFromUnicode(JSON.stringify(networkPayload));
     utf8Payload += converter.Finish();
     let payloadStream = Cc["@mozilla.org/io/string-input-stream;1"]
                         .createInstance(Ci.nsIStringInputStream);
     payloadStream.data = this.gzipCompressString(utf8Payload);
     request.send(payloadStream);
-
     return deferred.promise;
   },
 
   gzipCompressString: function gzipCompressString(string) {
     let observer = {
       buffer: "",
       onStreamComplete: function(loader, context, status, length, result) {
         this.buffer = String.fromCharCode.apply(this, result);
@@ -799,64 +728,51 @@ let Impl = {
     AsyncShutdown.sendTelemetry.addBlocker("TelemetryPing: shutting down",
                                            () => this.shutdown(),
                                            () => this._getState());
 
     this._delayedInitTask.arm();
     return this._delayedInitTaskDeferred.promise;
   },
 
-  // Do proper shutdown waiting and cleanup.
-  _cleanupOnShutdown: Task.async(function*() {
-    if (!this._initialized) {
-      return;
-    }
-
-    try {
-      // First wait for clients processing shutdown.
-      yield this._shutdownBarrier.wait();
-      // Then wait for any outstanding async ping activity.
-      yield this._connectionsBarrier.wait();
-
-      // Should down dependent components.
-      try {
-        yield TelemetryEnvironment.shutdown();
-      } catch (e) {
-        this._log.error("shutdown - environment shutdown failure", e);
-      }
-    } finally {
-      // Reset state.
-      this._initialized = false;
-      this._initStarted = false;
-    }
-  }),
-
   shutdown: function() {
     this._log.trace("shutdown");
 
+    let cleanup = () => {
+      if (!this._initialized) {
+        return;
+      }
+      let reset = () => {
+        this._initialized = false;
+        this._initStarted = false;
+      };
+      return this._shutdownBarrier.wait().then(
+               () => TelemetryEnvironment.shutdown().then(reset, reset));
+    };
+
     // We can be in one the following states here:
     // 1) setupTelemetry was never called
     // or it was called and
     //   2) _delayedInitTask was scheduled, but didn't run yet.
     //   3) _delayedInitTask is running now.
     //   4) _delayedInitTask finished running already.
 
     // This handles 1).
     if (!this._initStarted) {
       return Promise.resolve();
     }
 
     // This handles 4).
     if (!this._delayedInitTask) {
       // We already ran the delayed initialization.
-      return this._cleanupOnShutdown();
+      return cleanup();
     }
 
     // This handles 2) and 3).
-    return this._delayedInitTask.finalize().then(() => this._cleanupOnShutdown());
+    return this._delayedInitTask.finalize().then(cleanup);
   },
 
   /**
    * This observer drives telemetry.
    */
   observe: function (aSubject, aTopic, aData) {
     // The logger might still be not available at this point.
     if (!this._log) {
@@ -891,13 +807,11 @@ let Impl = {
   /**
    * Get an object describing the current state of this module for AsyncShutdown diagnostics.
    */
   _getState: function() {
     return {
       initialized: this._initialized,
       initStarted: this._initStarted,
       haveDelayedInitTask: !!this._delayedInitTask,
-      shutdownBarrier: this._shutdownBarrier.state,
-      connectionsBarrier: this._connectionsBarrier.state,
     };
   },
 };
--- a/toolkit/components/telemetry/docs/pings.rst
+++ b/toolkit/components/telemetry/docs/pings.rst
@@ -14,22 +14,16 @@ It contains some basic information share
 Submission
 ==========
 
 Pings are submitted via a common API on ``TelemetryPing``. It allows callers to choose a custom retention period that determines how long pings are kept on disk if submission wasn't successful.
 If a ping failed to submit (e.g. because of missing internet connection), Telemetry will retry to submit it until its retention period is up.
 
 *Note:* the :doc:`main pings <main-ping>` are kept locally even after successful submission to enable the HealthReport and SelfSupport features. They will be deleted after their retention period of 180 days.
 
-The telemetry server team is working towards `the common services status codes <https://wiki.mozilla.org/CloudServices/DataPipeline/HTTPEdgeServerSpecification#Server_Responses>`_, but for now the following logic is sufficient for Telemetry:
-
-* `2XX` - success, don't resubmit
-* `4XX` - there was some problem with the request - the client should not try to resubmit as it would just receive the same response
-* `5XX` - there was a server-side error, the client should try to resubmit later
-
 Ping types
 ==========
 
 * :doc:`main <main-ping>` - contains the information collected by Telemetry (Histograms, hang stacks, ...)
 * :doc:`saved-session <main-ping>` - contains the *"classic"* Telemetry payload with measurements covering the whole browser session. Used to make storage of saved-session easier server-side.
 * ``activation`` - *planned* - sent right after installation or profile creation
 * ``upgrade`` - *planned* - sent right after an upgrade
 * ``deletion`` - *planned* - on opt-out we may have to tell the server to delete user data