Bug 860094 - Remove saving of last payload to disk; r=rnewman, a=akeybl
authorGregory Szorc <gps@mozilla.com>
Wed, 01 May 2013 09:55:30 -0700
changeset 137648 2b89e9d3470e8d084d6565cb586075d7d8fb3e41
parent 137647 77cc698727aa6dd9361c03794cfa1d99e3330776
child 137649 9cebaf242de3bff1e13bd6f8acf714906301797c
push id2476
push usergszorc@mozilla.com
push dateTue, 21 May 2013 20:33:29 +0000
treeherdermozilla-beta@9cebaf242de3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, akeybl
bugs860094
milestone22.0
Bug 860094 - Remove saving of last payload to disk; r=rnewman, a=akeybl
services/healthreport/healthreporter.jsm
services/healthreport/tests/xpcshell/test_healthreporter.js
toolkit/components/telemetry/Histograms.json
--- a/services/healthreport/healthreporter.jsm
+++ b/services/healthreport/healthreporter.jsm
@@ -42,17 +42,16 @@ const DEFAULT_DATABASE_NAME = "healthrep
 const TELEMETRY_INIT = "HEALTHREPORT_INIT_MS";
 const TELEMETRY_INIT_FIRSTRUN = "HEALTHREPORT_INIT_FIRSTRUN_MS";
 const TELEMETRY_DB_OPEN = "HEALTHREPORT_DB_OPEN_MS";
 const TELEMETRY_DB_OPEN_FIRSTRUN = "HEALTHREPORT_DB_OPEN_FIRSTRUN_MS";
 const TELEMETRY_GENERATE_PAYLOAD = "HEALTHREPORT_GENERATE_JSON_PAYLOAD_MS";
 const TELEMETRY_JSON_PAYLOAD_SERIALIZE = "HEALTHREPORT_JSON_PAYLOAD_SERIALIZE_MS";
 const TELEMETRY_PAYLOAD_SIZE_UNCOMPRESSED = "HEALTHREPORT_PAYLOAD_UNCOMPRESSED_BYTES";
 const TELEMETRY_PAYLOAD_SIZE_COMPRESSED = "HEALTHREPORT_PAYLOAD_COMPRESSED_BYTES";
-const TELEMETRY_SAVE_LAST_PAYLOAD = "HEALTHREPORT_SAVE_LAST_PAYLOAD_MS";
 const TELEMETRY_UPLOAD = "HEALTHREPORT_UPLOAD_MS";
 const TELEMETRY_SHUTDOWN_DELAY = "HEALTHREPORT_SHUTDOWN_DELAY_MS";
 const TELEMETRY_COLLECT_CONSTANT = "HEALTHREPORT_COLLECT_CONSTANT_DATA_MS";
 const TELEMETRY_COLLECT_DAILY = "HEALTHREPORT_COLLECT_DAILY_MS";
 const TELEMETRY_SHUTDOWN = "HEALTHREPORT_SHUTDOWN_MS";
 const TELEMETRY_COLLECT_CHECKPOINT = "HEALTHREPORT_POST_COLLECT_CHECKPOINT_MS";
 
 /**
@@ -98,19 +97,25 @@ function AbstractHealthReporter(branch, 
 
   // Yes, this will probably run concurrently with remaining constructor work.
   let hasFirstRun = this._prefs.get("service.firstRun", false);
   this._initHistogram = hasFirstRun ? TELEMETRY_INIT : TELEMETRY_INIT_FIRSTRUN;
   this._dbOpenHistogram = hasFirstRun ? TELEMETRY_DB_OPEN : TELEMETRY_DB_OPEN_FIRSTRUN;
 
   TelemetryStopwatch.start(this._initHistogram, this);
 
-  this._ensureDirectoryExists(this._stateDir)
-      .then(this._onStateDirCreated.bind(this),
-            this._onInitError.bind(this));
+  // As soon as we could have storage, we need to register cleanup or
+  // else bad things (like hangs) happen on shutdown.
+  Services.obs.addObserver(this, "quit-application", false);
+  Services.obs.addObserver(this, "profile-before-change", false);
+
+  this._storageInProgress = true;
+  TelemetryStopwatch.start(this._dbOpenHistogram, this);
+  Metrics.Storage(this._dbName).then(this._onStorageCreated.bind(this),
+                                     this._onInitError.bind(this));
 }
 
 AbstractHealthReporter.prototype = Object.freeze({
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),
 
   /**
    * Whether the service is fully initialized and running.
    *
@@ -136,28 +141,16 @@ AbstractHealthReporter.prototype = Objec
     this._initializeHadError = true;
     this._initiateShutdown();
     this._initializedDeferred.reject(error);
 
     // FUTURE consider poisoning prototype's functions so calls fail with a
     // useful error message.
   },
 
-  _onStateDirCreated: function () {
-    // As soon as we have could storage, we need to register cleanup or
-    // else bad things happen on shutdown.
-    Services.obs.addObserver(this, "quit-application", false);
-    Services.obs.addObserver(this, "profile-before-change", false);
-
-    this._storageInProgress = true;
-    TelemetryStopwatch.start(this._dbOpenHistogram, this);
-    Metrics.Storage(this._dbName).then(this._onStorageCreated.bind(this),
-                                       this._onInitError.bind(this));
-  },
-
   // Called when storage has been opened.
   _onStorageCreated: function (storage) {
     TelemetryStopwatch.finish(this._dbOpenHistogram, this);
     delete this._dbOpenHistogram;
     this._log.info("Storage initialized.");
     this._storage = storage;
     this._storageInProgress = false;
 
@@ -809,59 +802,16 @@ AbstractHealthReporter.prototype = Objec
 
         deferred.reject(error);
       }
     );
 
     return deferred.promise;
   },
 
-  get _lastPayloadPath() {
-    return OS.Path.join(this._stateDir, "lastpayload.json");
-  },
-
-  _saveLastPayload: function (payload) {
-    let path = this._lastPayloadPath;
-    let pathTmp = path + ".tmp";
-
-    let encoder = new TextEncoder();
-    let buffer = encoder.encode(payload);
-
-    return OS.File.writeAtomic(path, buffer, {tmpPath: pathTmp});
-  },
-
-  /**
-   * Obtain the last uploaded payload.
-   *
-   * The promise is resolved to a JSON-decoded object on success. The promise
-   * is rejected if the last uploaded payload could not be found or there was
-   * an error reading or parsing it.
-   *
-   * This reads the last payload from disk. If you are looking for a
-   * current snapshot of the data, see `getJSONPayload` and
-   * `collectAndObtainJSONPayload`.
-   *
-   * @return Promise<object>
-   */
-  getLastPayload: function () {
-    let path = this._lastPayloadPath;
-
-    return OS.File.read(path).then(
-      function onData(buffer) {
-        let decoder = new TextDecoder();
-        let json = JSON.parse(decoder.decode(buffer));
-
-        return CommonUtils.laterTickResolvingPromise(json);
-      },
-      function onError(error) {
-        return Promise.reject(error);
-      }
-    );
-  },
-
   _now: function _now() {
     return new Date();
   },
 
   // These are stolen from AppInfoProvider.
   appInfoVersion: 1,
   appInfoFields: {
     // From nsIXULAppInfo.
@@ -1228,25 +1178,16 @@ HealthReporter.prototype = Object.freeze
     let now = this._now();
 
     return Task.spawn(function doUpload() {
       let payload = yield this.getJSONPayload();
 
       let histogram = Services.telemetry.getHistogramById(TELEMETRY_PAYLOAD_SIZE_UNCOMPRESSED);
       histogram.add(payload.length);
 
-      TelemetryStopwatch.start(TELEMETRY_SAVE_LAST_PAYLOAD, this);
-      try {
-        yield this._saveLastPayload(payload);
-        TelemetryStopwatch.finish(TELEMETRY_SAVE_LAST_PAYLOAD, this);
-      } catch (ex) {
-        TelemetryStopwatch.cancel(TELEMETRY_SAVE_LAST_PAYLOAD, this);
-        throw ex;
-      }
-
       let hrProvider = this.getProvider("org.mozilla.healthreport");
       if (hrProvider) {
         let event = this.lastSubmitID ? "continuationUploadAttempt"
                                       : "firstDocumentUploadAttempt";
         hrProvider.recordEvent(event, now);
       }
 
       TelemetryStopwatch.start(TELEMETRY_UPLOAD, this);
--- a/services/healthreport/tests/xpcshell/test_healthreporter.js
+++ b/services/healthreport/tests/xpcshell/test_healthreporter.js
@@ -593,17 +593,17 @@ add_task(function test_data_submission_s
     let request = new DataSubmissionRequest(deferred, now);
     reporter.requestDataUpload(request);
     yield deferred.promise;
     do_check_eq(request.state, request.SUBMISSION_SUCCESS);
     do_check_true(reporter.lastPingDate.getTime() > 0);
     do_check_true(reporter.haveRemoteData());
 
     // Ensure data from providers made it to payload.
-    let o = yield reporter.getLastPayload();
+    let o = yield reporter.getJSONPayload(true);
     do_check_true("DummyProvider.DummyMeasurement" in o.data.last);
     do_check_true("DummyConstantProvider.DummyMeasurement" in o.data.last);
 
     let data = yield getHealthReportProviderValues(reporter, now);
     do_check_eq(data._v, 1);
     do_check_eq(data.firstDocumentUploadAttempt, 1);
     do_check_eq(data.uploadSuccess, 1);
     do_check_eq(Object.keys(data).length, 3);
@@ -699,32 +699,16 @@ add_task(function test_policy_accept_rej
     do_check_false(policy.dataSubmissionPolicyAccepted);
     do_check_false(reporter.willUploadData);
   } finally {
     reporter._shutdown();
     yield shutdownServer(server);
   }
 });
 
-add_task(function test_upload_save_payload() {
-  let [reporter, server] = yield getReporterAndServer("upload_save_payload");
-
-  try {
-    let deferred = Promise.defer();
-    let request = new DataSubmissionRequest(deferred, new Date(), false);
-
-    yield reporter._uploadData(request);
-    let json = yield reporter.getLastPayload();
-    do_check_true("thisPingDate" in json);
-  } finally {
-    reporter._shutdown();
-    yield shutdownServer(server);
-  }
-});
-
 add_task(function test_error_message_scrubbing() {
   let reporter = yield getReporter("error_message_scrubbing");
 
   try {
     let profile = Services.dirsvc.get("ProfD", Ci.nsIFile).path;
     reporter._recordError("Foo " + profile);
 
     do_check_eq(reporter._errors.length, 1);
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -2976,23 +2976,16 @@
   },
   "HEALTHREPORT_UPLOAD_MS": {
     "kind": "exponential",
     "high": "60000",
     "n_buckets": 20,
     "extended_statistics_ok": true,
     "description": "Time (ms) it takes to upload the Health Report payload."
   },
-  "HEALTHREPORT_SAVE_LAST_PAYLOAD_MS": {
-    "kind": "exponential",
-    "high": "10000",
-    "n_buckets": 10,
-    "extended_statistics_ok": true,
-    "description": "Time (ms) it takes to persist the last submitted Health Report payload to disk."
-  },
   "HEALTHREPORT_COLLECT_CONSTANT_DATA_MS": {
     "kind": "exponential",
     "high": "20000",
     "n_buckets": 15,
     "description": "Time (ms) it takes FHR to collect constant data."
   },
   "HEALTHREPORT_COLLECT_DAILY_MS": {
     "kind": "exponential",