Settings can read DB multiple times on startup, again

RESOLVED FIXED

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

unspecified
x86_64
Linux
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [target 28/2] QARegressExclude, [qa-])

Attachments

(1 attachment)

Bug 840322 pretty carefully set things up so there could only be one pending request for the settings DB, but attachment 715842 [details] [diff] [review] regressed this.

We now consistently see two getAll() requests, which is going to contribute the problem in bug 842215 and regress memory usage as well.
Created attachment 716711 [details] [diff] [review]
Fix regression
Assignee: nobody → jones.chris.g
Attachment #716711 - Flags: review?(timdream)
Attachment #716711 - Flags: review?(etienne)
Attachment #716711 - Flags: review?(alive)
Attachment #716711 - Flags: review?(21)
blocking-b2g: --- → tef?
Attachment #716711 - Flags: review?(kaze)

Comment 2

5 years ago
Comment on attachment 716711 [details] [diff] [review]
Fix regression

I tried the same way to prevent multiple query on bug 842215, and verified it works.
Attachment #716711 - Flags: review+
Attachment #716711 - Flags: review?(timdream)
Attachment #716711 - Flags: review?(kaze)
Attachment #716711 - Flags: review?(etienne)
Attachment #716711 - Flags: review?(alive)
Attachment #716711 - Flags: review?(21)
CC'ing offender :-/
(You'll want to grab this one, if you haven't already.)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #3)
> CC'ing offender :-/

BTW, no worries; the invariant maintained by the original code was fairly subtle (sorry! my fault), and we don't have any performance tests that would have caught regressions from violating it.
blocking-b2g: tef? → tef+
Whiteboard: [target 28/2]

Comment 6

5 years ago
Can this land in preparation for uplift?

Updated

5 years ago
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → affected
https://github.com/mozilla-b2g/gaia/commit/949a0eed6cb28bfcd9a943effef96e356ca83ebf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(There was a small typo in the commit.  [sic])
v1-train@3c9f82b
v1.0.1@05457b4
status-b2g18: affected → fixed
status-b2g18-v1.0.1: affected → fixed

Updated

5 years ago
Flags: in-moztrap-

Updated

5 years ago
Whiteboard: [target 28/2] → [target 28/2] QARegressExclude

Comment 10

5 years ago
Cannot verify, need steps to blackbox test this issue.

Updated

5 years ago
Whiteboard: [target 28/2] QARegressExclude → [target 28/2] QARegressExclude, [qa-]
You need to log in before you can comment on or make changes to this bug.