Closed
Bug 1107781
Opened 10 years ago
Closed 10 years ago
Remove the call log db upgrade code
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.2 S10 (17apr)
People
(Reporter: gsvelto, Assigned: gsvelto)
References
Details
(Whiteboard: [planned-sprint c=3])
Attachments
(1 file, 3 obsolete files)
The call log db currently contains a long and winding update procedure to reach schema version 4. This procedure is complex and slow as well as unnecessary since no phone shipped with a database with a schema version prior to 4. The only use-case that would be affected by this are the reference workloads.
Removing that code would greatly reduce complexity of the call log DB. We should however upgrade the reference workloads so that they don't need to go through the upgrade process anymore.
Assignee | ||
Comment 1•10 years ago
|
||
300+ lines slashed. This removes the migration logic for the data in the database but leaves the schema migration intact. I've also removed the progress-bar and it's associated code because it's also not needed anymore and we don't want to keep that extra complexity around (the fact that we needed a progress bar to upgrade the call log DB is also crazy!). I haven't upgraded the reference workloads yet, that'll be coming in the next patch.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Attachment #8532390 -
Flags: feedback?(drs.bugzilla)
Updated•10 years ago
|
Whiteboard: [planned-sprint c=3]
Target Milestone: --- → 2.2 S2 (19dec)
Comment 2•10 years ago
|
||
Comment on attachment 8532390 [details] [diff] [review]
[PATCH WIP] Call log db machete attack
Review of attachment 8532390 [details] [diff] [review]:
-----------------------------------------------------------------
Looks reasonable. As mentioned offline, we can hack out more than this.
Attachment #8532390 -
Flags: feedback?(drs.bugzilla) → feedback+
Assignee | ||
Comment 3•10 years ago
|
||
This is the first patch, chopping out the upgrade procedure, now with a more reasonable description :) This is patch 2/3 because I want the first patch in the series to be the reference workload upgrade so that in no version the workloads are broken.
Attachment #8532390 -
Attachment is obsolete: true
Assignee | ||
Comment 4•10 years ago
|
||
This was called "Machete - part 2" before I could come up with a better name. It chops out the unused dialerRecents object store plus all its associated methods and unit-tests. This also reworks the unit-tests so that they have proper setup()s and the database is cleared between each one. Redundant checks are also removed. I'm considering splitting the later part in two for making review easier.
Updated•10 years ago
|
Target Milestone: 2.2 S2 (19dec) → 2.2 S3 (9jan)
Updated•10 years ago
|
Target Milestone: 2.2 S3 (9jan) → 2.2 S7 (6mar)
Updated•10 years ago
|
Target Milestone: 2.2 S7 (6mar) → 2.2 S8 (20mar)
Updated•10 years ago
|
Target Milestone: 2.2 S8 (20mar) → 2.2 S9 (3apr)
Updated•10 years ago
|
Target Milestone: 2.2 S9 (3apr) → 2.2 S10 (17apr)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8533953 [details] [diff] [review]
[PATCH 2/3] Remove the upgrade procedure from versions 1 to 5 of the call log database
Obsoleting, I'll replace the patches here with a PR.
Attachment #8533953 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8533955 [details] [diff] [review]
[PATCH 3/3] Remove the unused dialerRecents store and all associated code, adjust the unit-tests accordingly
Likewise.
Attachment #8533955 -
Attachment is obsolete: true
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8591734 [details] [review]
[gaia] gabrielesvelto:bug-1107781-call-log-machete-attack > mozilla-b2g:master
After some further wrestling with the reference workloads the call log database upgrade code is finally ready to go; r? Jon for the reference workloads and Doug for the dialer parts.
The PR is split in three patches:
- The first one upgrades the call log reference workloads to version 5 of the database and adds a couple of other empty reference workloads so that the reference-workload-empty target works out-of-the-box. These reference workloads will work on any device starting with the early 1.0.0 ones so we shouldn't have backward compatibility problems even if we use them on really old branches.
- The second patch chops out the upgrade procedure; it also removes the upgrade progress overlay which is now useless since upgrades won't take minutes anymore.
- The third patch removes the dialerRecents store which has been unused for a while and bumps the database version to 6. All the related dead code is removed and the related unit-tests have also been removed. This also reworks the unit-tests so that they have proper setup()s and the database is cleared between each one. Redundant checks are also removed; this makes the patch unfortunately large (even though it's still made mostly of deletions) but it's overdue IMHO. The unit-tests were in just as bad shape as the rest of the code, padded with redundant checks and often relying on the result of the previous test to work since the database wasn't cleared between them.
All in all I'm really happy to see this code go. It took me a while to properly migrate all the reference workloads because the upgrade code was - surprise, surprise - non-atomic and spawned transactions from within transactions which would then fail more-or-less randomly. The larger the call log the more likely the failure. It took me four tries to get the x-heavy reference workload properly migrated.
Attachment #8591734 -
Flags: review?(jhylands)
Attachment #8591734 -
Flags: review?(drs)
Comment 9•10 years ago
|
||
Comment on attachment 8591734 [details] [review]
[gaia] gabrielesvelto:bug-1107781-call-log-machete-attack > mozilla-b2g:master
Tamara, could you take a look at this as well? I'll review once you have.
Attachment #8591734 -
Flags: review?(thills)
Comment 10•10 years ago
|
||
Comment on attachment 8591734 [details] [review]
[gaia] gabrielesvelto:bug-1107781-call-log-machete-attack > mozilla-b2g:master
Hi Gabriele,
It looks good. So nice to get rid of all that! I think what you did with the tests in call_log_db.js is a big improvement... much simpler.
I did very basic testing on the call log and also ran the tests.
Thanks,
-tamara
Attachment #8591734 -
Flags: review?(thills) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8591734 [details] [review]
[gaia] gabrielesvelto:bug-1107781-call-log-machete-attack > mozilla-b2g:master
Thanks for the quick review Tamara, clearing Doug's r? flag.
Attachment #8591734 -
Flags: review?(drs)
Comment 12•10 years ago
|
||
Comment on attachment 8591734 [details] [review]
[gaia] gabrielesvelto:bug-1107781-call-log-machete-attack > mozilla-b2g:master
Assuming that this works, the only real concern I have with reference workload related updates are that they maintain backwards compatibility with earlier versions of b2g/fxos. Since that appears to be the case here, I have no issues.
Attachment #8591734 -
Flags: review?(jhylands) → review+
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Jon Hylands [:jhylands] from comment #12)
> Assuming that this works, the only real concern I have with reference
> workload related updates are that they maintain backwards compatibility with
> earlier versions of b2g/fxos. Since that appears to be the case here, I have
> no issues.
Yes, I've explicitly made sure that the upgraded workloads work with everything starting with v1.0.0. Thanks for the review, time to land.
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 14•10 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/b9667acfc50a7596346947b946f63a2351bfaf35
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•