apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
696 stars 158 forks source link

purging fails if ModPagespeed=off in global config #1077

Open lesfen opened 9 years ago

lesfen commented 9 years ago

When modpagespeed is set to off in the global config and only enabled for some virtual domains, purging will not work.

To duplicate...

  1. set ModPagespeed off in the global config
  2. set ModPagespeed on in 1 or more virtual domains
  3. from the gui, under caches -> purge cache, attempt to enter anything that is cached and nothing happens.

The item will show up in /var/cache/mod_pagespeed/cache.purge (assuming that is where your cache is). But the item will not show up in the gui and the purge will not happen.

Then set ModPagespeed on in the global and test again and purging works normally.

jmarantz commented 9 years ago

I can see exactly what the problem is, and a fix should not be difficult. The problem is that InstawebHandler::instaweb_handler does: server_context->FlushCacheIfNecessary() this does not take into account the .htaccess settings for "mod_pagespeed on", and ultimately this code runs:

if (options_->enabled()) { purgecontext->PollFileSystem(); }

Here the options_ come from the ServerContext (aka VHOST) and not the options computed dynamically per-request based on .htaccess, or (for that matter) request headers or query-params.

lesfen commented 9 years ago

I didn't fully understand your answer.. Are you saying I should put the pagespeed stuff in the vhost config instead of .htaccess? And if so, which items should be moved there?

------ Original Message ------ From: "jmarantz" notifications@github.com To: "pagespeed/mod_pagespeed" mod_pagespeed@noreply.github.com Cc: "lesfen" les@deltatechnicalservices.com Sent: 5/13/2015 10:18:06 AM Subject: Re: [mod_pagespeed] purging fails if ModPagespeed=off in global config (#1077)

I can see exactly what the problem is, and a fix should not be difficult. The problem is that InstawebHandler::instaweb_handler does: server_context->FlushCacheIfNecessary() this does not take into account the .htaccess settings for "mod_pagespeed on", and ultimately this code runs:

if (options_->enabled()) { purgecontext->PollFileSystem(); }

Here the options_ come from the ServerContext (aka VHOST) and not the options computed dynamically per-request based on .htaccess, or (for that matter) request headers or query-params.

— Reply to this email directly or view it on GitHub.

jmarantz commented 9 years ago

The workaround you already put in place is fine (turn MPS on in the root config) although it's not what you want for other reasons.

A better remedy -- and the one we test for -- is to define your hosts in VirtualHost blocks rather than an .htaccess file. I'm imagining there are reasons you didn't do that (e.g. the number of hosts is large or dynamic).

The best remedy is for us to ship a fix to this problem, which should be trivial -- I've already done it and just need to test it, get it reviewed, and it will be in a patch release. You could also apply the patch yourself and do a rebuild if that helped.

Sorry for missing this obvious testcase!

jmarantz commented 9 years ago

When possible, it is preferable to put the entire configuration in a vhost file rather than an .htaccess file. The vhosts are parsed and stored at server startup time, and PageSpeed can recycle a bunch of data structures allocated per vhost. When there is an htaccess file, the configuration is dynamically reparsed and merged on every request (this is an Apache architectural constraint), and PageSpeed cannot recycle as many data structures, but must burn them down and rebuild them on every request.

All of that overhead is small compared with image-optimization. It might consume order of 1 millisecond or less of extra time and CPU load per request. But all things being equal I usually prefer the faster strategy :)

-Josh

On Wed, May 13, 2015 at 1:31 PM, lesfen notifications@github.com wrote:

I just did a test.. I set ModPagespeed to off in the global and on in one o the vhost configs and the cache purging works. At least I can see in the GUI when I add something.

Is there any advantage to moving more than just ModPagespeed on to the vhost configs?

The only reason I had them in .htaccess is because I was just experimenting and I planned on moving them as I perfect the settings.

Les Fenison www.DeltaTechnicalServices.com les@DeltaTechnicalServices.com (503) 610-8747

------ Original Message ------ From: "jmarantz" notifications@github.com To: "pagespeed/mod_pagespeed" mod_pagespeed@noreply.github.com Cc: "lesfen" les@deltatechnicalservices.com Sent: 5/13/2015 10:26:57 AM Subject: Re: [mod_pagespeed] purging fails if ModPagespeed=off in global config (#1077)

The workaround you already put in place is fine (turn MPS on in the root config) although it's not what you want for other reasons.

A better remedy -- and the one we test for -- is to define your hosts in VirtualHost blocks rather than an .htaccess file. I'm imagining there are reasons you didn't do that (e.g. the number of hosts is large or dynamic).

The best remedy is for us to ship a fix to this problem, which should be trivial -- I've already done it and just need to test it, get it reviewed, and it will be in a patch release. You could also apply the patch yourself and do a rebuild if that helped.

Sorry for missing this obvious testcase!

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/pagespeed/mod_pagespeed/issues/1077#issuecomment-101753655 .

lesfen commented 9 years ago

Fantastic.. I just finished moving the entire contents of .htaccess for 15 sites to the vhost configs. I am also noticing that the ratio of hits to misses is much better already! Were you expecting that?

The only bad ratio I have is for the LRU. 80%misses 5% hits and 15% inserts..

Les Fenison www.DeltaTechnicalServices.com les@DeltaTechnicalServices.com (503) 610-8747

------ Original Message ------ From: "jmarantz" notifications@github.com To: "pagespeed/mod_pagespeed" mod_pagespeed@noreply.github.com Cc: "lesfen" les@deltatechnicalservices.com Sent: 5/13/2015 10:51:28 AM Subject: Re: [mod_pagespeed] purging fails if ModPagespeed=off in global config (#1077)

When possible, it is preferable to put the entire configuration in a vhost file rather than an .htaccess file. The vhosts are parsed and stored at server startup time, and PageSpeed can recycle a bunch of data structures allocated per vhost. When there is an htaccess file, the configuration is dynamically reparsed and merged on every request (this is an Apache architectural constraint), and PageSpeed cannot recycle as many data structures, but must burn them down and rebuild them on every request.

All of that overhead is small compared with image-optimization. It might consume order of 1 millisecond or less of extra time and CPU load per request. But all things being equal I usually prefer the faster strategy

-Josh

On Wed, May 13, 2015 at 1:31 PM, lesfen notifications@github.com wrote:

I just did a test.. I set ModPagespeed to off in the global and on in one o the vhost configs and the cache purging works. At least I can see in the GUI when I add something.

Is there any advantage to moving more than just ModPagespeed on to the vhost configs?

The only reason I had them in .htaccess is because I was just experimenting and I planned on moving them as I perfect the settings.

Les Fenison www.DeltaTechnicalServices.com les@DeltaTechnicalServices.com (503) 610-8747

------ Original Message ------ From: "jmarantz" notifications@github.com To: "pagespeed/mod_pagespeed" mod_pagespeed@noreply.github.com Cc: "lesfen" les@deltatechnicalservices.com Sent: 5/13/2015 10:26:57 AM Subject: Re: [mod_pagespeed] purging fails if ModPagespeed=off in global config (#1077)

The workaround you already put in place is fine (turn MPS on in the root config) although it's not what you want for other reasons.

A better remedy -- and the one we test for -- is to define your hosts in VirtualHost blocks rather than an .htaccess file. I'm imagining there are reasons you didn't do that (e.g. the number of hosts is large or dynamic).

The best remedy is for us to ship a fix to this problem, which should be trivial -- I've already done it and just need to test it, get it reviewed, and it will be in a patch release. You could also apply the patch yourself and do a rebuild if that helped.

Sorry for missing this obvious testcase!

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub

https://github.com/pagespeed/mod_pagespeed/issues/1077#issuecomment-101753655 .

— Reply to this email directly or view it on GitHub.

jmarantz commented 9 years ago

Sorry, this issue is still open for nginx.