Bug 1156253 - Part 2 - Use OS.File lz4 compression for archived telemetry pings. r=gfritzsche
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Thu, 07 May 2015 06:58:00 +0200
changeset 274493 23bf90af772dd63e989042a38fab2812cc556c13
parent 274492 73dcdc97ecdbdfb58f0e6799d8eaad6795a17fc5
child 274494 7daf22db7a3a4f9f10cc31c30b07b582adf007ff
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1156253
milestone40.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 1156253 - Part 2 - Use OS.File lz4 compression for archived telemetry pings. r=gfritzsche
toolkit/components/telemetry/TelemetryStorage.jsm
toolkit/components/telemetry/tests/unit/test_PingAPI.js
--- a/toolkit/components/telemetry/TelemetryStorage.jsm
+++ b/toolkit/components/telemetry/TelemetryStorage.jsm
@@ -418,51 +418,64 @@ let TelemetryStorageImpl = {
   /**
    * Save an archived ping to disk.
    *
    * @param {object} ping The ping data to archive.
    * @return {promise} Promise that is resolved when the ping is successfully archived.
    */
   saveArchivedPing: Task.async(function*(ping) {
     const creationDate = new Date(ping.creationDate);
-    const filePath = getArchivedPingPath(ping.id, creationDate, ping.type);
+    // Get the archived ping path and append the lz4 suffix to it (so we have 'jsonlz4').
+    const filePath = getArchivedPingPath(ping.id, creationDate, ping.type) + "lz4";
     yield OS.File.makeDir(OS.Path.dirname(filePath), { ignoreExisting: true,
                                                        from: OS.Constants.Path.profileDir });
-    yield TelemetryStorage.savePingToFile(ping, filePath, true);
+    yield this.savePingToFile(ping, filePath, /*overwrite*/ true, /*compressed*/ true);
   }),
 
   /**
    * Load an archived ping from disk.
    *
    * @param {string} id The pings id.
    * @param {number} timestampCreated The pings creation timestamp.
    * @param {string} type The pings type.
    * @return {promise<object>} Promise that is resolved with the ping data.
    */
-  loadArchivedPing: function(id, timestampCreated, type) {
+  loadArchivedPing: Task.async(function*(id, timestampCreated, type) {
     this._log.trace("loadArchivedPing - id: " + id + ", timestampCreated: " + timestampCreated + ", type: " + type);
     const path = getArchivedPingPath(id, new Date(timestampCreated), type);
-    this._log.trace("loadArchivedPing - loading ping from: " + path);
-    return this.loadPingFile(path);
-  },
+    const pathCompressed = path + "lz4";
+
+    try {
+      // Try to load a compressed version of the archived ping first.
+      this._log.trace("loadArchivedPing - loading ping from: " + pathCompressed);
+      return yield this.loadPingFile(pathCompressed, /*compressed*/ true);
+    } catch (ex if ex.becauseNoSuchFile) {
+      // If that fails, look for the uncompressed version.
+      this._log.trace("loadArchivedPing - compressed ping not found, loading: " + path);
+      return yield this.loadPingFile(path, /*compressed*/ false);
+    }
+  }),
 
   /**
    * Remove an archived ping from disk.
    *
    * @param {string} id The pings id.
    * @param {number} timestampCreated The pings creation timestamp.
    * @param {string} type The pings type.
    * @return {promise<object>} Promise that is resolved when the pings is removed.
    */
-  _removeArchivedPing: function(id, timestampCreated, type) {
+  _removeArchivedPing: Task.async(function*(id, timestampCreated, type) {
     this._log.trace("_removeArchivedPing - id: " + id + ", timestampCreated: " + timestampCreated + ", type: " + type);
     const path = getArchivedPingPath(id, new Date(timestampCreated), type);
+    const pathCompressed = path + "lz4";
+
     this._log.trace("_removeArchivedPing - removing ping from: " + path);
-    return OS.File.remove(path);
-  },
+    yield OS.File.remove(path, {ignoreAbsent: true});
+    yield OS.File.remove(pathCompressed, {ignoreAbsent: true});
+  }),
 
   /**
    * Get a list of info on the archived pings.
    * This will scan the archive directory and grab basic data about the existing
    * pings out of their filename.
    *
    * @return {promise<sequence<object>>}
    */
@@ -484,17 +497,17 @@ let TelemetryStorageImpl = {
         this._log.warn("loadArchivedPingList - skipping invalidly named subdirectory " + dir.path);
         continue;
       }
 
       this._log.trace("loadArchivedPingList - checking in subdir: " + dir.path);
       let pingIterator = new OS.File.DirectoryIterator(dir.path);
       let pings = (yield pingIterator.nextBatch()).filter(e => !e.isDir);
 
-      // Now process any ping files of the form "<timestamp>.<uuid>.<type>.json"
+      // Now process any ping files of the form "<timestamp>.<uuid>.<type>.[json|jsonlz4]".
       for (let p of pings) {
         // data may be null if the filename doesn't match the above format.
         let data = this._getArchivedPingDataFromFileName(p.name);
         if (!data) {
           continue;
         }
 
         // In case of conflicts, overwrite only with newer pings.
@@ -523,24 +536,29 @@ let TelemetryStorageImpl = {
   /**
    * Save a single ping to a file.
    *
    * @param {object} ping The content of the ping to save.
    * @param {string} file The destination file.
    * @param {bool} overwrite If |true|, the file will be overwritten if it exists,
    * if |false| the file will not be overwritten and no error will be reported if
    * the file exists.
+   * @param {bool} [compress=false] If |true|, the file will use lz4 compression. Otherwise no
+   * compression will be used.
    * @returns {promise}
    */
-  savePingToFile: function(ping, filePath, overwrite) {
+  savePingToFile: function(ping, filePath, overwrite, compress = false) {
     return Task.spawn(function*() {
       try {
         let pingString = JSON.stringify(ping);
-        yield OS.File.writeAtomic(filePath, pingString, {tmpPath: filePath + ".tmp",
-                                  noOverwrite: !overwrite});
+        let options = { tmpPath: filePath + ".tmp", noOverwrite: !overwrite };
+        if (compress) {
+          options.compression = "lz4";
+        }
+        yield OS.File.writeAtomic(filePath, pingString, options);
       } catch(e if e.becauseExists) {
       }
     })
   },
 
   /**
    * Save a ping to its file.
    *
@@ -728,35 +746,40 @@ let TelemetryStorageImpl = {
   testLoadHistograms: function(file) {
     pingsLoaded = 0;
     return this.loadHistograms(file.path);
   },
 
   /**
    * Loads a ping file.
    * @param {String} aFilePath The path of the ping file.
+   * @param {Boolean} [aCompressed=false] If |true|, expects the file to be compressed using lz4.
    * @return {Promise<Object>} A promise resolved with the ping content or rejected if the
    *                           ping contains invalid data.
    */
-  loadPingFile: Task.async(function* (aFilePath) {
-    let array = yield OS.File.read(aFilePath);
+  loadPingFile: Task.async(function* (aFilePath, aCompressed = false) {
+    let options = {};
+    if (aCompressed) {
+      options.compression = "lz4";
+    }
+    let array = yield OS.File.read(aFilePath, options);
     let decoder = new TextDecoder();
     let string = decoder.decode(array);
 
     let ping = JSON.parse(string);
     // The ping's payload used to be stringified JSON.  Deal with that.
     if (typeof(ping.payload) == "string") {
       ping.payload = JSON.parse(ping.payload);
     }
     return ping;
   }),
 
   /**
    * Archived pings are saved with file names of the form:
-   * "<timestamp>.<uuid>.<type>.json"
+   * "<timestamp>.<uuid>.<type>.[json|jsonlz4]"
    * This helper extracts that data from a given filename.
    *
    * @param fileName {String} The filename.
    * @return {Object} Null if the filename didn't match the expected form.
    *                  Otherwise an object with the extracted data in the form:
    *                  { timestamp: <number>,
    *                    id: <string>,
    *                    type: <string> }
@@ -765,18 +788,18 @@ let TelemetryStorageImpl = {
     // Extract the parts.
     let parts = fileName.split(".");
     if (parts.length != 4) {
       this._log.trace("_getArchivedPingDataFromFileName - should have 4 parts");
       return null;
     }
 
     let [timestamp, uuid, type, extension] = parts;
-    if (extension != "json") {
-      this._log.trace("_getArchivedPingDataFromFileName - should have a 'json' extension");
+    if (extension != "json" && extension != "jsonlz4") {
+      this._log.trace("_getArchivedPingDataFromFileName - should have 'json' or 'jsonlz4' extension");
       return null;
     }
 
     // Check for a valid timestamp.
     timestamp = parseInt(timestamp);
     if (Number.isNaN(timestamp)) {
       this._log.trace("_getArchivedPingDataFromFileName - should have a valid timestamp");
       return null;
--- a/toolkit/components/telemetry/tests/unit/test_PingAPI.js
+++ b/toolkit/components/telemetry/tests/unit/test_PingAPI.js
@@ -74,48 +74,59 @@ add_task(function* test_archivedPings() 
   TelemetryArchive._testReset();
 
   let pingList = yield TelemetryArchive.promiseArchivedPingList();
   Assert.deepEqual(expectedPingList, pingList,
                    "Should have submitted pings in archive list after restart");
   yield checkLoadingPings();
 
   // Write invalid pings into the archive with both valid and invalid names.
-  let writeToArchivedDir = Task.async(function*(dirname, filename, content) {
+  let writeToArchivedDir = Task.async(function*(dirname, filename, content, compressed) {
     const dirPath = OS.Path.join(gPingsArchivePath, dirname);
     yield OS.File.makeDir(dirPath, { ignoreExisting: true });
     const filePath = OS.Path.join(dirPath, filename);
     const options = { tmpPath: filePath + ".tmp", noOverwrite: false };
+    if (compressed) {
+      options.compression = "lz4";
+    }
     yield OS.File.writeAtomic(filePath, content, options);
   });
 
   const FAKE_ID1 = "10000000-0123-0123-0123-0123456789a1";
   const FAKE_ID2 = "20000000-0123-0123-0123-0123456789a2";
+  const FAKE_ID3 = "20000000-0123-0123-0123-0123456789a3";
   const FAKE_TYPE = "foo";
 
   // These should get rejected.
   yield writeToArchivedDir("xx", "foo.json", "{}");
   yield writeToArchivedDir("2010-02", "xx.xx.xx.json", "{}");
   // This one should get picked up...
   yield writeToArchivedDir("2010-02", "1." + FAKE_ID1 + "." + FAKE_TYPE + ".json", "{}");
   // ... but get overwritten by this one.
   yield writeToArchivedDir("2010-02", "2." + FAKE_ID1 + "." + FAKE_TYPE + ".json", "");
   // This should get picked up fine.
   yield writeToArchivedDir("2010-02", "3." + FAKE_ID2 + "." + FAKE_TYPE + ".json", "");
+  // This compressed ping should get picked up fine as well.
+  yield writeToArchivedDir("2010-02", "4." + FAKE_ID3 + "." + FAKE_TYPE + ".jsonlz4", "");
 
   expectedPingList.push({
     id: FAKE_ID1,
     type: "foo",
     timestampCreated: 2,
   });
   expectedPingList.push({
     id: FAKE_ID2,
     type: "foo",
     timestampCreated: 3,
   });
+  expectedPingList.push({
+    id: FAKE_ID3,
+    type: "foo",
+    timestampCreated: 4,
+  });
   expectedPingList.sort((a, b) => a.timestampCreated - b.timestampCreated);
 
   // Reset the TelemetryArchive so we scan the archived dir again.
   yield TelemetryArchive._testReset();
 
   // Check that we are still picking up the valid archived pings on disk,
   // plus the valid ones above.
   pingList = yield TelemetryArchive.promiseArchivedPingList();