850704 - Don't use SQLite for FHR storage, or consider major changes to SQLite schema

Status: RESOLVED WONTFIX

(Firefox Health Report Graveyard :: Client: Desktop, defect, P4)

Description

[Gregory Szorc [:gps]]

I'm mass filing large ticket items from FHR's performance review: https://wiki.mozilla.org/Performance/FHR. This bug is for the first item:

1. JSON vs SQLite storage The SQLite schema used in FHR is overly complex and seems over-engineered and has a degree of redundancy. We could simplify FHR storage by storing data in JSON files, this will cut down amount of IO and fsyncs. We should flush out current session data on shutdown(without fsync to delay shutdown).

Comment 1

[Richard Newman [:rnewman]]

My general perspective on this: it's not a trivial change, and the performance implications on both desktop and mobile for various timescales for each storage mechanism aren't well understood.

I don't think spending the time to swap out the storage backend for JSON is an obvious performance win versus the current codebase. Measurements over time will give a better picture about whether we should address this.

Ideally we'll wait long enough that someone will produce a solution that's a better fit than both JSON and sqlite -- there are enough places in the tree where a persistent non-relational storage solution is needed.

Marking this as P4, which right now means "less important than robustness or landing FHR on Android". We'll re-triage again as our milestones shift.

Comment 2

[(dormant account)]

As I explained to gps, not using sqlite will result in a lot less fsyncs(only 1 fsync around fhr submission time). This means better overall perf, battery life, reduced disk overhead, etc. It's a huge improvement, especially on mobile.

This should be higher priority given disk-space constraints on mobile + benefit of reducing background IO.

Comment 3

[Richard Newman [:rnewman]]

(In reply to Taras Glek (:taras) from comment #2)
> As I explained to gps, not using sqlite will result in a lot less
> fsyncs(only 1 fsync around fhr submission time). This means better overall
> perf, battery life, reduced disk overhead, etc. It's a huge improvement,
> especially on mobile.

Remember that FHR records data frequently, not just at submission time; I'd like to retain some control over how reliable that recording is. And we can reduce fsyncs but keep sqlite by using WAL.


> This should be higher priority given disk-space constraints on mobile +
> benefit of reducing background IO.

In general we'd rather trade disk space and background IO for heap, because our Android heap is very constrained.

I'd like to get a good picture of the performance and reliability characteristics of the system as a whole with the code that we have, rather than delay a release for additional work that addresses a speculative problem.

If we find that using sqlite is costing more in performance than it buys us in correctness and functionality, then we can figure out and build a better solution. But let's do that *after* we ship something that meets our organizational goals, not before.

Comment 4

[Gregory Szorc [:gps]]

If we ever implement batching and flushing, this will significantly cut down on the number of transactions we perform (assuming we batch all queued writes into a single transaction). This should drastically cut down on the number of fsyncs. At that time, I believe the I/O characteristics will be very similar to raw file. Not quite as close to metal, but surely a lot closer than we are now. And, we will still have the full power of SQL.

Once we batch and flush, we'll also be able to close the connection when its not in use (if so desired). This will free up more memory.

Unless we're intent on killing SQLite, my vote is to see where batching and flushing gets us.

Comment 5

[Alessio Placitelli [:Dexter]]

FHR is going away per bug 1209088, wontfix.