1238785 - Delete health.db after Firefox Health Report is removed from Fennec

Status: RESOLVED FIXED

(Android Background Services Graveyard :: Firefox Health Report Service, defect)

Description

[Richard Newman [:rnewman]]

Now that FHR is gone, we can delete the existing database file and free up that space.

We have existing bugs to do janitorial work, so let's see if we can kill more than one bird with this stone rather than making a special case.

Comment 1

[:Margaret Leibovic]

mcomella, can you take this?

(In reply to Richard Newman [:rnewman] from comment #0)
> Now that FHR is gone, we can delete the existing database file and free up
> that space.
> 
> We have existing bugs to do janitorial work, so let's see if we can kill
> more than one bird with this stone rather than making a special case.

One dead bird is better than no dead birds, let's not block on fixing the world.

Comment 2

[:Margaret Leibovic]

Mike, let's find time to do this.

Comment 3

[:Margaret Leibovic]

This probably isn't going to happen for 46 at this rate. However, we should definitely do this cleanup to free up that space.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

This is on my radar and I'm working on a fix.

For reference, I have a profile from January 12th and it takes up the following space:
 72K health.db
 41K health.db-journal
 113K

Not huge, but not insignificant for mobile.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1238785 - Add FileCleanupService. r=ahunt

This is intentionally kept minimal to ensure simplicity.

Review commit: https://reviewboard.mozilla.org/r/45935/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45935/

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1238785 - Add FileCleanupController and tests. r=ahunt

This controller is under-featured (e.g. it's not scheduling cleanups for future
dates and it doesn't cache files it already deleted) in favor of simplicity.

Review commit: https://reviewboard.mozilla.org/r/45937/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45937/

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1238785 - Start file cleanup in onStart. r=ahunt

I added some log statements to ensure this worked correctly locally - on a new
profile:
  * Log statements were printed listed the two files I expected to be deleted
and their paths
  * The log statements did not appear after closing and reopened fennec,
meaning the process short-circuited as expected.

Ideally, I'd test that a profile that currently has these files actually gets
them deleted, but it's not easy to create profiles.

The previous patches also contributed unit tests.

Review commit: https://reviewboard.mozilla.org/r/45939/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45939/

Comment 8

[Andrzej Hunt :ahunt]

Comment on attachment 8740699 [details]
MozReview Request: Bug 1238785 - Add FileCleanupService. r=ahunt

https://reviewboard.mozilla.org/r/45935/#review43005

Looks reasonable to me!

I was wondering whether we'd want to log when File.delete() fails, but that seems unnecessary to me.

Comment 9

[Richard Newman [:rnewman]]

You probably want to delete -shm and -wal suffixes, too; they might differ across android versions. Best to catch em all.

Comment 10

[Andrzej Hunt :ahunt]

Comment on attachment 8740700 [details]
MozReview Request: Bug 1238785 - Add FileCleanupController and tests. r=ahunt

https://reviewboard.mozilla.org/r/45937/#review43015

Comment 11

[Andrzej Hunt :ahunt]

Comment on attachment 8740701 [details]
MozReview Request: Bug 1238785 - Start file cleanup in onStart. r=ahunt

https://reviewboard.mozilla.org/r/45939/#review43017

Looks good to me!

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Richard Newman [:rnewman] from comment #9)
> You probably want to delete -shm and -wal suffixes, too; they might differ
> across android versions. Best to catch em all.

Added. Gotta catch 'em all!

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/009b0d561a9c10452c674a54aff993db3aeffdcd
Bug 1238785 - Add FileCleanupService. r=ahunt

https://hg.mozilla.org/integration/fx-team/rev/a955962a5198d3964d336755df7b7968323d657e
Bug 1238785 - Add FileCleanupController and tests. r=ahunt

https://hg.mozilla.org/integration/fx-team/rev/acaefde5bf02467bf2000059ab4cd2cd23dba0a2
Bug 1238785 - Start file cleanup in onStart. r=ahunt

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The only users that this affects are long time users (i.e. had FHR). I don't think these users are likely to uninstall Firefox for the several KB of data that we're cleaning up here so I don't feel it's worth uplifting to 47 (we're tracking 47).

Margaret, what do you think?

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/009b0d561a9c
https://hg.mozilla.org/mozilla-central/rev/a955962a5198
https://hg.mozilla.org/mozilla-central/rev/acaefde5bf02

Comment 16

[:Margaret Leibovic]

(In reply to Michael Comella (:mcomella) from comment #14)
> The only users that this affects are long time users (i.e. had FHR). I don't
> think these users are likely to uninstall Firefox for the several KB of data
> that we're cleaning up here so I don't feel it's worth uplifting to 47
> (we're tracking 47).
> 
> Margaret, what do you think?

That's a smart way to think about it. I agree, if they haven't given up on Firefox yet, I don't think an extra 6 weeks will make much of a difference.