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

purging fails if ModPagespeed=off in global config #1077

Open
lesfen opened this Issue May 13, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@lesfen
Copy link

lesfen commented May 13, 2015

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 jmarantz self-assigned this May 13, 2015

@jmarantz

This comment has been minimized.

Copy link
Contributor

jmarantz commented May 13, 2015

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()) {
purge_context_->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

This comment has been minimized.

Copy link

lesfen commented May 13, 2015

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()) {
purge_context_->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

This comment has been minimized.

Copy link
Contributor

jmarantz commented May 13, 2015

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

This comment has been minimized.

Copy link
Contributor

jmarantz commented May 13, 2015

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
#1077 (comment)
.

@lesfen

This comment has been minimized.

Copy link

lesfen commented May 13, 2015

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

#1077 (comment)
.


Reply to this email directly or view it on GitHub.

jmarantz added a commit to apache/incubator-pagespeed-ngx that referenced this issue May 18, 2015

jmarantz added a commit that referenced this issue May 19, 2015

Use 'unplugged' rather than 'on/off' to gate cache-purge processing.
Incorporate more config data into the key used for sharing CachePath
objects so that we don't share cache objects between vhosts that have
purging enabled/disabled, and we don't share between those that have
different cache-flush filenames.

#1077

@jmarantz jmarantz closed this May 19, 2015

@jmarantz jmarantz reopened this May 19, 2015

@jmarantz

This comment has been minimized.

Copy link
Contributor

jmarantz commented May 19, 2015

Sorry, this issue is still open for nginx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment