605647 - Statistics csv/json files should have hashes in their URL

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Code Quality, defect, P3)

Description

[Wil Clouser [:clouserw]]

We have far future expires headers on our CSV and JSON exports, which is a great idea until our stats export fails one day and we have zeroes cached forever.

We should be doing a simple hash (how many rows, sum of numbers, whatever it takes to make it unique) and tacking it on the end of the URL.  Hopefully it never changes.

Comment 1

[Wil Clouser [:clouserw]]

Let's do this:

1) If the numbers (you pick which ones! :) are zeroes, send a no cache header

2) Make sure the expires header is only for +7 days

Comment 2

[Andy McKay]

Don't think looking for zeroes is useful, we need to be looking for absence of data.

https://github.com/jbalogh/zamboni/commit/4f93312b8bea2f75c6306eb60608edf201b2ae05

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

Neither https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20101018-20101117.csv nor https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20101018-20101117.json have no missing data (that I can tell), and yet, I'm not seeing the Expires: header:

Access-Control-Allow-Methods:GET
Access-Control-Allow-Origin:*
Cache-Control:no-cache
Connection:Keep-Alive
Content-Encoding:gzip
Content-Type:text/plain; charset=utf-8
Date:Wed, 17 Nov 2010 23:42:41 GMT
Keep-Alive:timeout=5, max=1000
Server:Apache
Transfer-Encoding:chunked
Vary:Accept-Encoding
Via:Moz-Cache-pm-zlb-amo01
X-Backend-Server:pm-app-amo24
x-content-security-policy-report-only:policy-uri /services/csp/policy
x-frame-options:DENY

Am I testing this correctly?

Comment 4

[Wil Clouser [:clouserw]]

Yeah, those should have Expires headers, not sure what's going on.  We can bump this to .4 if we need to - these URLs aren't live.

Comment 5

[Andy McKay]

Not able to reproduce yet:

~/sandboxes/zamboni(master) $ curl -D out.txt https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20101018-20101117.json > /dev/null
...
~/sandboxes/zamboni(master) $ grep Cache out.txt
Cache-Control: max-age=604800

Comment 6

[Andy McKay]

Betting you were logged in, getting different answers depending upon login status.

Comment 7

[Jeff Balogh (:jbalogh)]

Zeus is modifying the cache headers if you're logged in.

(zamboni)$ curl -sI https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20101018-20101117.json A Cache-Control
Cache-Control: max-age=604800
(zamboni)$ curl -H'Cookie: sessionid=me' -sI https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20101018-20101117.json A Cache-Control
Cache-Control: no-cache

Comment 8

[Andy McKay]

With help from Jeff, we think the following is happening. Zeus sees that the
user is logged in because it has the cookies AMOv3 (remora) and sessionid
(zamboni). It then disables caching because logged in users are not cached.
This alters the cache headers.

Presumably we can alter Zeus not to do that on certain URLs to preserve
caching.

A valid test at this point is:

https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20101018-20101117.json

data and so Cache-Control: max-age=604800 set

https://addons-next.allizom.org/z/en-US/firefox/addon/1865/statistics/downloads-day-20301018-20301117.json

no data and so Cache-Control: max-age=0 set

But this has to be done as an anonymous user.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Thanks, Andy -- followed those steps in comment 8 (while logged out), and it's verified FIXED.

Would you mind spinning off an IT bug for the Zeus tweaking?

Comment 10

[Wil Clouser [:clouserw]]

right now it's:

if logged_in:
  dont_cache()

we can adjust this to be:

if cache_control_header:
  respect_the_header()

else if logged_in:
  dont_cache()

Anyone see a reason not to do that?

Comment 11

[Andy McKay]

Sounds good to me.

Comment 12

[Wil Clouser [:clouserw]]

bug 613349