Closed
Bug 1038936
Opened 10 years ago
Closed 10 years ago
Stop putting waffles switches in `consumer_info` API endpoint
Categories
(Marketplace Graveyard :: Code Quality, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: cvan, Assigned: kngo)
References
Details
(Keywords: perf)
We should stop using this endpoint for waffle switches: https://github.com/mozilla/fireplace/blob/76a2dc1/hearth/media/js/consumer_info.js#L13 According to Mat, there are some times when we don't hit this endpoint. And it's meant for region/carrier information only. Not sure what the sane alternatives are. Blocking the page on another separate request is out of the question.
Updated•10 years ago
|
Assignee: nobody → mpillard
Priority: -- → P3
Comment 1•10 years ago
|
||
A bit of background: we used to be as clever as possible with consumer_info and not load it if we already had the user region loaded (phones with a SIM card in for instance). This was changed in https://github.com/mozilla/fireplace/commit/76a2dc1041a3f5f652fe5c8035476ccb523c1f41#diff-d210940f972693bc8e88fd148498e269R1 There were some discussions about this on IRC, and here are the different options we have going forward: - Keep waffle switches in consumer_info. Block on consumer_info unconditionally. This is what we are doing now. - Remove waffle switches from consumer_info, instead have them as data attributes on the body, since the HTML is served by zamboni. This doesn't work for true packaged app however. Since the only switch we care about right now is Firefox Accounts, we could decide that the true packaged app has Firefox Accounts on/off at all times and update the package when we want to change that. - Expanding on the previous idea, remove waffle switches, have just a static flag in fireplace local settings, change it as necessary. We could have fireplace deployed on altdev with the Firefox Accounts switch on, for instance. Like the previous idea, we'd generate test packages with the flag on or off depending on what we want to test. The last 2 options would make it possible to revert consumer_info back to its clever state and not make that API call if the user region can be detected from the SIM, making things more performance for those users.
Keywords: perf
Updated•10 years ago
|
Updated•10 years ago
|
Assignee: mpillard → nobody
Updated•10 years ago
|
Blocks: marketplace-perf
Reporter | ||
Comment 2•10 years ago
|
||
Is this the reason Consumer Info is slow? It takes anywhere from 75-400ms from Mozilla WiFi (11 MB/s). I can't imagine it loading faster than in a few seconds anywhere else.
Comment 3•10 years ago
|
||
consumer_info is composed of 3 things right now: - Geoip, if you don't have a SIM - it was its original purpose, and it's not *that* slow. - Waffles - If you are logged in, the ids for all the apps you have installed/developed/purchased The 3rd one is probably the biggest offender for you, I imagine.
Reporter | ||
Comment 4•10 years ago
|
||
Probably. Per bug 1073225 comment 10, I was able to get it to load in 58ms today - directly from the CDN! That's awesome but also inconsistent. It took several seconds from my home Internet connection just a few weeks ago. Yeah, I imagine I've installed more apps than the average user, which could account for some of the slowness. From my machine right now, it takes ~30-70ms, which is pretty spectacular. But again, I'm on a 11 MB/s connection in MV. https://www.dropbox.com/s/sx6jh4kil5wm8gx/Screenshot%202014-10-08%2016.24.57.png?dl=0 I ran a test using WebPageTest. Location: Los Angeles, CA (which is closer to our data centre than I am) Browser: Firefox Connection: Cable http://www.webpagetest.org/result/141009_E2_4HX/ Cached: No Time: 530 ms (341 ms for body) http://www.webpagetest.org/result/141009_E2_4HX/1/details/ Cached: Yes Time: 360 ms (335ms for body, which is 6 ms slower) http://www.webpagetest.org/result/141009_E2_4HX/1/details/cached/ It looks like it's taking a significant amount of time for TLS requests. A test for /robots.txt, which is an entirely static page served from Django, had similar times: http://www.webpagetest.org/result/141009_MT_4FM/
Assignee | ||
Comment 5•10 years ago
|
||
https://github.com/mozilla/fireplace/pull/704 puts waffles in a different endpoint with offline caching (ttl: 1 week). Like consumer_info, it will block on first run and when it expires. Though maybe we get it to never expire and simply always update asynchronously. It was blocking the upgrade path for FxA for other Commonplace projects as well as the Bower upgrade. Switches removed from consumer info: https://github.com/mozilla/zamboni/commit/2d20380ce4d941e59320f96a4a3e5807492c8499
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → kngo
Comment 7•10 years ago
|
||
It's too bad consumer_info still has fxa stuff - if it had not, we could finally do what we used to and not call it at all if we already know the region from the SIM. Maybe we should file another bug for that.
Assignee | ||
Comment 8•10 years ago
|
||
I didn't realize that. With that known, I wouldn't have made the Waffle endpoint and instead rework the config endpoint to be leaner and include that stuff.
Assignee | ||
Comment 9•10 years ago
|
||
Moved FXA stuff from consumer info to site config endpoint, removed waffle endpoint. Changed Fireplace to use site config endpoint instead of waffles/consumer_info. So now you can not call it if you have the region from the SIM.
You need to log in
before you can comment on or make changes to this bug.
Description
•