Closed Bug 1156355 Opened 9 years ago Closed 8 years ago

Telemetry redesign: Consolidate file logic in TelemetryStorage.jsm

Categories

(Toolkit :: Telemetry, defect, P2)

defect
Points:
1

Tracking

()

RESOLVED FIXED
Tracking Status
firefox40 --- affected

People

(Reporter: gfritzsche, Unassigned)

References

Details

(Whiteboard: [unifiedTelemetry] [measurement:client])

Attachments

(1 obsolete file)

Per: https://docs.google.com/document/d/1lKRfNl5aJ0W2flOp67Y5O3WDz2dSSz7a6r23DQ89poU/edit?pli=1

TelemetryStorage.jsm should be a helper module with almost no state.

This module should be in charge of
* the saving/loading pings & scanning ping dirs
* serializing writes to the same file
* pruning & deleting of persisted pings

This module should know how to do higher-level operations such as saving/removing the aborted session file.
This module should handle the details of saving/loading/scanning ping directories:
* TelemetryController will use this for pending pings
  * we won’t be able to load them all into memory, so we should handle them just like the archived pings for cheap scanning
* TelemetryController will use this for the archived pings

The interface is at least:
* savePendingPing(ping) called by TelemetryPing
* loadPendingPing(id) called by TelemetryPing
* getPendingPingList()
* saveArchivedPing(ping) (done in bug 1150134)
* loadArchivedPing(id) (done in bug 1150134)
* getArchivedPingList() (done in bug 1150134)
* saveAbortedSessionPing()
* loadAbortedSessionPing()
* removeAbortedSessionPing()
Blocks: 1156358
Also we should:
* rename loadHistograms()
* rename |get pingsLoaded()| to something more fitting with its name
* change loadSavedPings() to not load all pings from this (here or in bug 1041616 or ...)
  * pending pings should use a file name scheme that contains their id, type & creation timestamp (like archived pings)
  * we should build a list of pending pings
  * we can clean old pings etc. out just from that cheap-to-build list
  * when we have send opportunities etc. we can load pings from that list and send them
Blocks: 1156712
Attached patch Part 1: Refactor aborted-session ping handling (obsolete) (deleted) — Splinter Review
Move aborted-session file handling details into TelemetryStorage.
Assignee: nobody → gfritzsche
Status: NEW → ASSIGNED
Comment on attachment 8600473 [details] [diff] [review]
Part 1: Refactor aborted-session ping handling

I moved this part over to bug 1156253 to not block that one.
Attachment #8600473 - Attachment is obsolete: true
Depends on: 1156253
Whiteboard: [b5] [unifiedTelemetry]
Whiteboard: [b5] [unifiedTelemetry] → [unifiedTelemetry]
Assignee: gfritzsche → nobody
Status: ASSIGNED → NEW
Points: --- → 1
Priority: -- → P2
Whiteboard: [unifiedTelemetry] → [unifiedTelemetry] [measurement:client]
Depends on: 1190801
Priority: P2 → P3
Priority: P3 → P2
I think this bug can be safely closed as we've got all the stuff we wanted in TelemetryStorage, with the other modules relying on this one for Telemetry I/O operations.

- Saving, loading and removing the aborted-session ping [1]
- Saving, loading, removing a pending ping and scanning the pending ping directory [2]
- Saving, loading an archived ping and scanning the archived ping directory [3]
- Pruning/Enforcing quota on the pending ping directory [4]
- Pruning/Enforcing quota on the archived ping directory [5]

It also seems that the changes from comment 1 were already addressed.

[1] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#306,316,349
[2] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#230,257,267,281,295
[3] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#150,160,171
[4] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#191,200
[5] - https://dxr.mozilla.org/mozilla-central/rev/c2256ee8ae9a8ee0bf7ab49a8b1924720d846cc7/toolkit/components/telemetry/TelemetryStorage.jsm#182
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Resolution: WORKSFORME → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: