[Tracking Requested - why for this release]: We'll end up unnecessarily sending TBs of cacheable data for Release users once tiles is enabled. It's currently disabled for 33 by bug 1073823, so we won't hit this yet. I understand the last 33 beta is happening tomorrow, so I'll try to fix this today although it would be nice to test on Nightly first. oyiptong noticed that we're sending a ton of data (hundreds of GB a day?) for fetch requests for just beta users, and turns out it's because we POST the fetch request where the server returns a redirect to a links file, and Firefox invalidates cache entry for subsequent gets in this situation. The cache headers sent by the redirected endpoint are invalidated: http://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#6117 The server added the GET functionality: https://github.com/mozilla/onyx/commit/28796349e6c88ab8381c7707e7763f90c945ed15

Is there an endpoint that I can test this with?

oyiptong, is switching to GET necessary or does it even help? Is it actually that we need to add If-Modified-Since header to our POST? current POST: x = new XMLHttpRequest(); x.open("POST", "https://tiles.up.mozillalabs.com/v2/links/fetch", false); x.send('{"locale":"en-US"}'); console.log(x.status, x.statusText, x.getAllResponseHeaders().replace(/\r/g, "")) 200 "OK" "Content-Type: application/octet-stream Content-Length: 216040 Connection: keep-alive Date: Sun, 28 Sep 2014 14:59:41 GMT Last-Modified: Thu, 28 Aug 2014 00:57:06 GMT Etag: "9da3094dbe9ee89cbc70070142daea5b" Accept-Ranges: bytes Server: AmazonS3 Age: 13118 X-Cache: Hit from cloudfront Via: 1.1 587d7cc9256adbdfe54523e1d39a50bb.cloudfront.net (CloudFront) X-Amz-Cf-Id: 7XsZNyAZW8V6ZwpE2HQyrn91t7mRt_Od_gwZ2jmP_cKef-4ZYWdN_A== new GET directly to cloudfront: x = new XMLHttpRequest(); x.open("GET", "https://d1tiksivlekcfk.cloudfront.net/directoryLinks.json", false); /*x.setRequestHeader("If-Modified-Since", "Thu, 28 Aug 2014 00:57:06 GMT");*/ x.send('{"locale":"en-US"}'); console.log(x.status, x.statusText, x.getAllResponseHeaders().replace(/\r/g, "")) 200 "OK" "Content-Type: application/octet-stream Content-Length: 216040 Connection: keep-alive Date: Sun, 28 Sep 2014 14:59:41 GMT Last-Modified: Thu, 28 Aug 2014 00:57:06 GMT Etag: "9da3094dbe9ee89cbc70070142daea5b" Accept-Ranges: bytes Server: AmazonS3 Age: 13234 X-Cache: Hit from cloudfront Via: 1.1 69dc5215907ea1f94133bedbe807deb4.cloudfront.net (CloudFront) X-Amz-Cf-Id: upw7rvIcsEqLMvXADx_zXvxm7lGOBcUwb-S8LSR7HzRuHVNo1F9j8g== subsequent GET to cloudfront: 200 "OK" "Content-Type: application/octet-stream Content-Length: 216040 Connection: keep-alive Date: Sun, 28 Sep 2014 14:59:41 GMT Last-Modified: Thu, 28 Aug 2014 00:57:06 GMT Etag: "9da3094dbe9ee89cbc70070142daea5b" Accept-Ranges: bytes Server: AmazonS3 Age: 13287 X-Cache: Hit from cloudfront Via: 1.1 69dc5215907ea1f94133bedbe807deb4.cloudfront.net (CloudFront) X-Amz-Cf-Id: lQEpus6-ytUawvkuk-CXHO46WS8dI9k6TqRBBpza86sd2N_arCW9ow== However, if I add in If-Not-Modified-Since to the current POST endpoint: x = new XMLHttpRequest(); x.open("POST", "https://tiles.up.mozillalabs.com/v2/links/fetch", false); x.setRequestHeader("If-Modified-Since", "Thu, 28 Aug 2014 00:57:06 GMT"); x.send('{"locale":"en-US"}'); console.log(x.status, x.statusText, x.getAllResponseHeaders().replace(/\r/g, "")) 304 "Not Modified" "Connection: keep-alive Date: Wed, 01 Oct 2014 22:09:51 GMT Etag: "9da3094dbe9ee89cbc70070142daea5b" Server: AmazonS3 Age: 13337 X-Cache: Hit from cloudfront Via: 1.1 69dc5215907ea1f94133bedbe807deb4.cloudfront.net (CloudFront) X-Amz-Cf-Id: v6IZaIZHFSaU7iTMOVe2-a55nWso9bsyMdyk6PJpEXf1Un3rbfAQeg==

Ha actually, I should have looked more closely at the subsequent GET output: x = new XMLHttpRequest(); x.open("GET", "https://d1tiksivlekcfk.cloudfront.net/directoryLinks.json", false); x.send(); console.log(x.status, x.statusText, x.getAllResponseHeaders().replace(/\r/g, "")) 200 "OK" "Content-Type: application/octet-stream Content-Length: 216040 Connection: keep-alive Date: Sun, 28 Sep 2014 14:59:41 GMT Last-Modified: Thu, 28 Aug 2014 00:57:06 GMT Etag: "9da3094dbe9ee89cbc70070142daea5b" Accept-Ranges: bytes Server: AmazonS3 Age: 13664 X-Cache: Hit from cloudfront Via: 1.1 d13aa395c8a3e4eca4e94319e2655f1e.cloudfront.net (CloudFront) X-Amz-Cf-Id: j6Qr13rRXyL1xJPko0p82FjOtVY3sV5Mr5xO_tVWEOmdo2C2p8jytw== It's responding with the exact same date, so I'm guessing it's hitting the Firefox cache and replaying the same 200 status.

Just to be complete, I set up a dummy server that always responds 303: s.send_header("Location", "https://dtex4kvbppovt.cloudfront.net/STAR/en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409.json") POST: results in Connection: keep-alive and about:cache's fetch count for en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409.json does not change. 200 "OK" "Content-Type: application/json Content-Length: 399307 Connection: keep-alive Date: Fri, 26 Sep 2014 20:32:39 GMT Cache-Control: max-age=31536000 Last-Modified: Thu, 25 Sep 2014 20:41:20 GMT Etag: "b8af0b8a143d91caf211b9b813e02769" Server: AmazonS3 Age: 441628 X-Cache: Hit from cloudfront Via: 1.1 e2af8a85927835558866752f53562ecd.cloudfront.net (CloudFront) X-Amz-Cf-Id: J9U73KwAhs6MnOgfwMMIIc5iIr9Qmsz3egPiuW-d8lqTj6GUU-8vfw== GET: results in no keep-alive and about:cache fetch count increments by 1 200 "OK" "Content-Type: application/json Content-Length: 399307 Date: Fri, 26 Sep 2014 20:32:39 GMT Cache-Control: max-age=31536000 Last-Modified: Thu, 25 Sep 2014 20:41:20 GMT Etag: "b8af0b8a143d91caf211b9b813e02769" Server: AmazonS3 Age: 440442 X-Cache: Hit from cloudfront Via: 1.1 107fa1bd02fde2bfcae7dabbfe0b0205.cloudfront.net (CloudFront) X-Amz-Cf-Id: bGjPlJ9mFmjTZ18evUKPCO0J7KbgzBFXC0M-MyHKnF-DRouU7VeFSQ==

oyiptong, is there a good way to test that we don't run into the opposite problem of Firefox always hitting the cache even if there's an update? Will the file name change for each update? E.g., v2/links/fetch/en-US would 303 to en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409.json now then 303 to a different file name later?

oyiptong, if you need a build to test with: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/edilee@gmail.com-4ef5077fe4d6/try-macosx64/firefox-35.0a1.en-US.mac.dmg https://tbpl.mozilla.org/?tree=Try&rev=4ef5077fe4d6

Ed, go to build for beta 9 will happen in a few hours. could you explain your plan? :) Thanks

I would feel better with some baking on nightly35 before uplift to aurora34 or beta33. Given that beta33 already has tiles turned off, it's not super critical to land now other than avoiding an uplift to release33 when tiles is turned on for that channel. The server endpoint isn't live yet, so I'm not entirely sure if the client changes fixes the problem (although initial dev testing seems to confirm fixed). But it would also be difficult for other people to verify the fix as well right now.

adw has been the only reviewer for DirectoryLinksProvider.jsm. This change switches from a POST with locale passed in the body post data to GET with locale in the url. Firefox does not cache redirected GETs from POSTs, so the new GET endpoint will 303 to the same redirect GET from before but Firefox will now cache that.

Comment on attachment 8500626 [details] [diff] [review] v1 Review of attachment 8500626 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/modules/DirectoryLinksProvider.jsm @@ +183,5 @@ > }, > > _fetchAndCacheLinks: function DirectoryLinksProvider_fetchAndCacheLinks(uri) { > + // Replace with the same display locale used for selecting links data > + uri = uri.replace("%LOCALE%", this.locale); Shouldn't we use nsURLFormatter.formatURLPref()? Or does our locale selection have different requirements than what that gives us? ::: browser/modules/test/xpcshell/test_DirectoryLinksProvider.js @@ +293,4 @@ > yield DirectoryLinksProvider.init(); > yield cleanJsonFile(); > // this must trigger directory links json download and save it to cache file > + yield DirectoryLinksProvider._fetchAndCacheLinks(kExampleURL + "%LOCALE%"); Shouldn't this change the pref rather than calling into a private method?

(In reply to Tim Taubert [:ttaubert] from comment #10) > > + uri = uri.replace("%LOCALE%", this.locale); > Shouldn't we use nsURLFormatter.formatURLPref()? Or does our locale > selection have different requirements than what that gives us? That's what I originally had in the wip patch, but I'm not entirely sure how |this.locale| and formatURLPref can differ. I was even thinking about changing this.locale to use something like formatUrl("%LOCALE%") to read out the value to use. From my quick research, I didn't use formatUrl because that would use the Firefox built locale instead of potentially something else that the user chooses for display. Either way, we'll want to be consistent in the locale we send to the server and the locale we use to key off data returned from the server. > > + yield DirectoryLinksProvider._fetchAndCacheLinks(kExampleURL + "%LOCALE%"); > Shouldn't this change the pref rather than calling into a private method? Yeah, it seems that there's a helper to do just that: promiseDirectoryDownloadOnPrefChange

This keeps the |this.locale| which does the same thing as the addon manager in using the OS locale if pref set then falling back to Firefox locale. AddonManager: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#246 UrlFormatter: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/urlformatter/nsURLFormatter.js#90 Also adds cleanup to the pref change observer instead of relying on promiseCleanDirectoryLinksProvider to be called.

Given what comment #0 says in terms of traffic, we should try to QA this if possible.

I'd want to ensure that we've done and end to end test with the final end point that is going to be used in the go-live release for tiles. There is some time here, but not a lot of time. In general, the sooner we can do such an end to end test the better. Karl, is there a final endpoint to test against?

Redirecting Clint's question in comment 14 to :mostlygeek, who is in charge of the production deployment.

(In reply to Clint Talbert ( :ctalbert ) from comment #14) > is there a final endpoint to test against? https://tiles.services.mozilla.com/v2/links/fetch/en-US

I've verified that the "browser.newtabpage.directory.source" pref is changed to "https://tiles.services.mozilla.com/v2/links/fetch/%LOCALE%" using latest Nightly, build ID: 20141015030202. I'm not sure if and how I could verify the traffic. Ed, do you have any idea of how I could test it?

Is there a normal way of testing web traffic / caching? I did it through HTTP logging, but maybe using a tool outside of Firefox might be better. Many years ago, I've used things like wireshark although I'm not sure how it works with https.

We tried to monitor this on our end but unfortunately we were unable to do it. Our IT guys were also unable to help us regarding this matter. Could someone on your side please verify this on the server as you previously did?

I will set up a staging server that we can use to monitor if caching is working correctly. Stay tuned.

I did some testing with a custom onyx endpoint and a custom tiles_index.json. It looks like the caching is working, though not quite what I expected. Test #1 "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 "GET /test/STAR/en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409b.json HTTP/1.1" 200 399307 This successfully redirected to the data blobl Test #2 "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 (no fetch) I was expecting a request with If-Modified-Since, but FF doesn't send anything. Tests #3, #4 same results as #2. Test #5 - changing the tile-index.json to point at new data blob "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 "GET /test/STAR/en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409c.json HTTP/1.1" 200 399307 It successfully pulls down the new file Test #6, #7 "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 "GET /test/STAR/en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409c.json HTTP/1.1" 304 0 "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 "GET /test/STAR/en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409c.json HTTP/1.1" 304 0 Firefox properly fetches and gets a 304 response back. So it seems a little buggy though caching seems to be working.

(In reply to Benson Wong [:mostlygeek] from comment #23) > "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 > (no fetch) vs > "GET /v2/links/fetch/en-US HTTP/1.1" 303 423 > "GET /test/STAR/en-US.4868533e4d1b515ae58dfa7b96adff01b0eb9409c.json > HTTP/1.1" 304 0 I'm not familiar enough with what situations Firefox serves content directly from its own cache or not. But either situation is better than Firefox redownloading the whole json each tile. One thing you can check if you still have that profile open: 1) load about:cache 2) click "List Cache Entries" under "disk" 3) search for the .json cache entries 4) check the response-head for the case where Firefox uses its own cache vs checking for 304 -- the output should look like.. response-head: HTTP/1.1 200 OK Content-Type: application/json Content-Length: 399307 Date: Fri, 26 Sep 2014 20:32:39 GMT Cache-Control: max-age=31536000 Last-Modified: Thu, 25 Sep 2014 20:41:20 GMT Etag: "b8af0b8a143d91caf211b9b813e02769" Server: AmazonS3 Age: 1194478 X-Cache: Hit from cloudfront Via: 1.1 5a4c25dc864206e0083c239ad3cd0113.cloudfront.net (CloudFront) X-Amz-Cf-Id: 4svPgzhmtUEZdAPB-40nInTmI2bxG5cuwSeKcKbvds5iHRQO5h7ZBw==

Interesting checking about:cache. It appears that FF by default gave it a default expiry time, looks to be about 20minutes. After that expiry time it will send a request and get back a 304. Nice.

Are we able to see that cloudfront has been returning 304s since last week? Or maybe bandwidth usage? If we do compare bandwidth, make sure to compare with Beta33 numbers when it was still enabled -- it was turned off on Beta Oct 1 by bug 1073823.

Approval Request Comment [Feature/regressing bug #]: Bug 986521 [User impact if declined]: Users will unnecessarily download unchanged links data every day [Describe test coverage new/current, TBPL]: Added tests to check for GET with locale. Manual testing from client side and server side of Firefox caching. [Risks and why]: Low - js changes to low-dependency code and baked on Nightly for a little bit [String/UUID change made/needed]: none

Comment on attachment 8500769 [details] [diff] [review] v1.1 Approval Request Comment: See comment 27 for a?beta/release. Only difference here is jsm+test have moved on aurora35/nightly36 from their original location on beta34/release33.

https://hg.mozilla.org/releases/mozilla-aurora/rev/2dd8793d1580 https://hg.mozilla.org/releases/mozilla-beta/rev/da489398c483