New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LoadFromFileCacheTtlMs is not honored (*after* first request) #1418

Closed
0x42h opened this Issue May 16, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@0x42h
Copy link

0x42h commented May 16, 2017

When LoadFromFile is set with a custom LoadFromFileCacheTtlMs, the first request wil honor the LoadFromFileCacheTtlMs, but after the first request, it wil default back to max-age: 300 (5 minutes), as per the non-public psol code.

Example:

inside the server config:

pagespeed LoadFromFile "http://example.com" "/var/www"
pagespeed LoadFromFileCacheTtlMs 31556926000;

First request

Request URL:
https://example.com/test.js?1

Response headers:

HTTP/1.1 200 OK
Server: nginx/1.10.0 (Ubuntu)
Content-Type: application/javascript
Transfer-Encoding: chunked
Connection: keep-alive
Date: Tue, 16 May 2017 17:31:57 GMT
Expires: Wed, 16 May 2018 23:20:43 GMT
Last-Modified: Tue, 17 Jan 2017 14:01:55 GMT
Cache-Control: max-age=31556926, s-maxage=10

Second request

Request URL:
https://example.com/test.js?1

Response headers:

Server: nginx/1.10.0 (Ubuntu)
Content-Type: application/javascript
Connection: keep-alive
X-Original-Content-Length: 32197
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 8806
ETag: W/"PSA-aj-CiswHsWsJj"
Date: Tue, 16 May 2017 17:32:53 GMT
Expires: Tue, 16 May 2017 17:37:53 GMT
Cache-Control: max-age=300

Third request

Request URL:
https://example.com/test.js?2

Response headers:

Server: nginx/1.10.0 (Ubuntu)
Content-Type: application/javascript
Transfer-Encoding: chunked
Connection: keep-alive
Date: Tue, 16 May 2017 17:34:15 GMT
Expires: Wed, 16 May 2018 23:23:01 GMT
Last-Modified: Tue, 17 Jan 2017 14:01:55 GMT
Cache-Control: max-age=31556926, s-maxage=10

Fourth request

Request URL:
https://example.com/test.js?2

Response headers:

Server: nginx/1.10.0 (Ubuntu)
Content-Type: application/javascript
Connection: keep-alive
X-Original-Content-Length: 32197
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 8806
ETag: W/"PSA-aj-CiswHsWsJj"
Date: Tue, 16 May 2017 17:35:03 GMT
Expires: Tue, 16 May 2017 17:40:03 GMT
Cache-Control: max-age=300

The even results are repeated for every request after this, with the exact same URL, maintaining the (PSOL/default) 300 seconds.

@jmarantz

This comment has been minimized.

Copy link
Contributor

jmarantz commented May 16, 2017

@0x42h

This comment has been minimized.

Copy link

0x42h commented May 16, 2017

After the second and third (and as far as the amount of times I try to the present of this moment (more than 10)), it keeps defaulting to 300 seconds.

It was the intention to show that different query vars make a difference. Note that the ETags are the same for the 2nd request and the 4th request too, in spite of the query vars, which makes sense, of course, as the content never changed, for which I do not understand the following behavior either.

Why should query vars make any difference at all, as this feature is made to serve static content from disk, which does not change, in spite of the query vars, which pagespeed checks for, I suppose, every time a request is done, afaiu the documentation.

If the date of the file in question is not changed on disk, then I would say that it should just give the same optimized response, regardless of the query vars, but let me not get off topic. Let's first see why I receive the TTL I set at first and then the default TTL of PSOL.

@0x42h

This comment has been minimized.

Copy link

0x42h commented Jun 2, 2017

Anyone?

@0x42h

This comment has been minimized.

Copy link

0x42h commented Jun 2, 2017

Example: https://www.weghuis.nl/wp-content/uploads/2016/11/11357415279.jpg = max-age=300
https://www.weghuis.nl/wp-content/uploads/2016/11/11357415279.jpg?{enter not yet before entered chars here to make sure the request is "new"} = max-age=31556926, s-maxage=10

For example:
First request of https://www.weghuis.nl/wp-content/uploads/2016/11/11357415279.jpg?random = max-age=31556926, s-maxage=10
Second request of https://www.weghuis.nl/wp-content/uploads/2016/11/11357415279.jpg?random = max-age=300

X-Page-Speed: 1.12.34.2-0

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jun 2, 2017

I can confirm the behaviour on master, and I think it's a bug. After looking in to it, I think that InPlaceRewriteContext::FixFetchFallbackHeaders needs to use load_from_file_cache_ttl_ms_ where it now uses implicit_cache_ttl_ms, when it is adjusting the headers for a response that is mapped by LoadFromFile configuration.

oschaaf added a commit to apache/incubator-pagespeed-mod that referenced this issue Jun 2, 2017

LoadFromFileCacheTtlMs is not honored (*after* first request)
Not ready to go in yet, needs more verification of code paths.

Fixes apache/incubator-pagespeed-ngx#1418

oschaaf added a commit to apache/incubator-pagespeed-mod that referenced this issue Jun 2, 2017

LoadFromFileCacheTtlMs is not honored (*after* first request)
Not ready to go in yet, needs more verification of code paths.

Fixes apache/incubator-pagespeed-ngx#1418
@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Jun 2, 2017

Created apache/incubator-pagespeed-mod#1552
I need to spend some more time verifying that more thorougly, but a couple of manual tests seem to indicate it addresses this.

@0x42h

This comment has been minimized.

Copy link

0x42h commented Jun 3, 2017

Hallo Otto,

Something like that was my suspicion. Thank you for the confirmation.

As in this bug report for mod_pagespeed...

pagespeed InPlaceResourceOptimization off 

... circumvents the issue, but obviously then you do not have in place resource replacements.

apache/incubator-pagespeed-mod#989

oschaaf added a commit to apache/incubator-pagespeed-mod that referenced this issue Jun 4, 2017

LoadFromFileCacheTtlMs is not honored (*after* first request) (#1552)
Fix: LoadFromFileCacheTtlMs is not honored (*after* first request)

Fixes apache/incubator-pagespeed-ngx#1418
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment