[Catalyst-commits] r13687 - in trunk/Catalyst-Plugin-PageCache: .
lib/Catalyst/Plugin t t/lib t/lib/TestApp/Controller
timbunce at dev.catalyst.perl.org
timbunce at dev.catalyst.perl.org
Wed Nov 3 16:01:23 GMT 2010
Author: timbunce
Date: 2010-11-03 16:01:23 +0000 (Wed, 03 Nov 2010)
New Revision: 13687
Modified:
trunk/Catalyst-Plugin-PageCache/Changes
trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm
trunk/Catalyst-Plugin-PageCache/t/04cache.t
trunk/Catalyst-Plugin-PageCache/t/lib/TestApp.pm
trunk/Catalyst-Plugin-PageCache/t/lib/TestApp/Controller/Cache.pm
trunk/Catalyst-Plugin-PageCache/t/lib/TestAppI18N.pm
Log:
Major update to page index and clear_cached_page() logic.
+ - Documented race-hazard risks with page index concept.
+ - Reduced the race-hazard risk in one scenario.
+ - clear_cached_page() now returns the number of entries matched.
+ - page index keys are original /path?query not altered by key_maker or sha1.
+ - Now warns if disable_index is not explicitly set in config.
+ - disable_index may default true in a future release.
Modified: trunk/Catalyst-Plugin-PageCache/Changes
===================================================================
--- trunk/Catalyst-Plugin-PageCache/Changes 2010-11-03 15:09:37 UTC (rev 13686)
+++ trunk/Catalyst-Plugin-PageCache/Changes 2010-11-03 16:01:23 UTC (rev 13687)
@@ -12,7 +12,14 @@
- Refactored page cache storage logic, inspired by RT#53303.
- Use SHA1 to create cache key to limit length and charset. RT#62343.
- Treat the order of repeated URL parameter values as significant.
+ Changes to page index logic used by clear_cached_page():
+ - Documented race-hazard risks with page index concept.
+ - Reduced the race-hazard risk in one scenario.
- Added app class name to index key for added safety.
+ - clear_cached_page() now returns the number of entries matched.
+ - page index keys are original /path?query not altered by key_maker or sha1.
+ - Now warns if disable_index is not explicitly set in config.
+ - disable_index may default true in a future release.
0.22 2009-06-25 10:38:00
- Update to use MRO::Compat
Modified: trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm
===================================================================
--- trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm 2010-11-03 15:09:37 UTC (rev 13686)
+++ trunk/Catalyst-Plugin-PageCache/lib/Catalyst/Plugin/PageCache.pm 2010-11-03 16:01:23 UTC (rev 13687)
@@ -41,38 +41,44 @@
$c->_cache_page( \%options );
}
+
sub clear_cached_page {
my ( $c, $uri ) = @_;
- return unless ( $c->can( 'cache' ) );
+ return undef unless $c->can( 'cache' );
my $pc_config = $c->config->{'Plugin::PageCache'};
- my $is_debug = $pc_config->{debug};
+ my $is_debug = $pc_config->{debug};
# Warn if index was disabled
- if ( $pc_config->{disable_index} ) {
+ my $index_page_key = $pc_config->{index_page_key} or do {
$c->log->warn("clear_cached_page($uri) did not clear the cache because disable_index is set");
return;
- }
+ };
- my $removed = 0;
-
my $cache = $c->cache; # curry the cache just once, here
- my $index = $cache->get( $pc_config->{index_page_key} ) || {};
+ my $index = $cache->get( $index_page_key ) || {};
- foreach my $key ( keys %{$index} ) {
- if ( $key =~ /^(?::[^:]+:)?$uri$/xms ) {
- $cache->remove( $key );
- delete $index->{$key};
- $removed++;
-
- $c->log->debug( "Removed $key from page cache" ) if $is_debug;
- }
+ my $removed = 0;
+ foreach my $index_key ( keys %$index ) {
+
+ # find matching entries, ignoring the language prefix
+ next unless $index_key =~ /^ $uri (?:\#.*)? $/x;
+
+ # delete from the index and remove the corresponding cache entry
+ # (which may have a different key, e.g., sha1 encoded)
+ my $cache_key = delete $index->{$index_key};
+ $cache->remove( $cache_key );
+ $removed++;
+
+ $c->log->debug( "Removed $index_key from page cache" ) if $is_debug;
}
- $cache->set( $pc_config->{index_page_key}, $index, $pc_config->{no_expire} )
+ $cache->set( $index_page_key, $index, $pc_config->{no_expire} )
if $removed;
+
+ $removed;
}
@@ -153,11 +159,11 @@
$cache->remove( $key );
- if ( !$pc_config->{disable_index} ) {
- my $index = $cache->get( $pc_config->{index_page_key} ) || {};
- delete $index->{$key};
- $cache->set( $pc_config->{index_page_key}, $index,
- $pc_config->{no_expire});
+ if ( my $index_page_key = $pc_config->{index_page_key} ) {
+ my $index = $cache->get( $index_page_key ) || {};
+ my $found = delete $index->{ $data->{index_key} };
+ $cache->set( $index_page_key, $index, $pc_config->{no_expire})
+ if $found;
}
}
@@ -338,26 +344,37 @@
if ( $pc_config->{cache_headers} ) {
$data->{headers} = {
- map { $_ => $headers->header($_) }
- $headers->header_field_names
+ map { $_ => $headers->header($_) } $headers->header_field_names
};
}
- $data->{expires} = $options->{expires} if exists $options->{expires};
+ if ($pc_config->{index_page_key}) {
+ # We can't simply use $key for $index_key because $key may have been
+ # mangled by sha1 and/or a key_maker hook. We include $key to ensure
+ # unique entries in the index in cases where a given uri might produce
+ # different results eg due to headers like language.
+ $data->{index_key} = '/'.$c->req->path;
+ $data->{index_key} .= '?'.$c->req->uri->query if $c->req->uri->query;
+ $data->{index_key} .= '#'.$key;
+ }
+ if (exists $options->{expires}) {
+ $data->{expires} = $options->{expires}
+ }
+
my $cache = $c->cache; # curry the cache just once, here
$cache->set( $key, $data );
$c->_set_page_cache_headers( $data ); # don't forget the first time
- if ( !$pc_config->{disable_index} ) {
+ if ( $data->{index_key} ) {
# Keep an index cache of all pages that have been cached, for use
- # with clear_cached_page
- my $index = $cache->get($pc_config->{index_page_key}) || {};
- $index->{$key} = 1;
- # Save data in cache
- $cache->set($pc_config->{index_page_key}, $index, $pc_config->{no_expire});
+ # with clear_cached_page. This is error prone. See KNOWN ISSUES.
+ my $index_page_key = $pc_config->{index_page_key};
+ my $index = $cache->get($index_page_key) || {};
+ $index->{ $data->{index_key} } = $key;
+ $cache->set($index_page_key, $index, $pc_config->{no_expire});
}
return $data;
@@ -380,7 +397,6 @@
$pc_config->{expires} ||= 60 * 5;
$pc_config->{cache_headers} ||= 0;
$pc_config->{set_http_headers} ||= 0;
- $pc_config->{disable_index} ||= 0;
$pc_config->{busy_lock} ||= 0;
$pc_config->{debug} ||= $c->debug;
@@ -389,6 +405,13 @@
$pc_config->{index_page_key} = "$c._page_cache_index"
unless defined $pc_config->{index_page_key};
+ if (not defined $pc_config->{disable_index}) {
+ warn "Plugin::PageCache config does not include disable_index, which currently defaults false but may default true in future\n";
+ $pc_config->{disable_index} = 0;
+ }
+ $pc_config->{index_page_key} = undef
+ if $pc_config->{disable_index};
+
# detect the cache plugin being used and set appropriate
# never-expires syntax
if ( $c->can('cache') ) {
@@ -589,11 +612,16 @@
disable_index => 1
-To support the C<clear_cached_page> method, PageCache attempts keep an index of
-all cached pages. If you don't intend to use C<clear_cached_page>, you may
-enable this config option to avoid the overhead of creating and updating the
-cache index. This option is disabled by default (i.e. the page index is enabled).
+To support the L</clear_cached_page> method, PageCache attempts keep an index
+of all cached pages. This adds overhead by performing extra cache reads and
+writes to maintain the (possibly very large) page index. It's also not
+reliable, see L</KNOWN ISSUES>.
+If you don't intend to use C<clear_cached_page>, you should enable this config
+option to avoid the overhead of creating and updating the cache index. This
+option is currently disabled (i.e. the page index is enabled) by default but
+that may change in a future release.
+
index_page_key => '...'
The key string used for the index, Defaults to a string that includes the name
@@ -737,17 +765,23 @@
=head2 clear_cached_page
-To clear the cached value for a URI, you may call clear_cached_page.
+To clear the cached pages for a URI, you may call clear_cached_page.
$c->clear_cached_page( '/view/userlist' );
$c->clear_cached_page( '/view/.*' );
+ $c->clear_cached_page( '/view/.*\?.*\bparam=value\b' );
-This method takes an absolute path or regular expression. For obvious reasons,
-this must be called from a different controller than the cached controller. You
-may for example wish to build an admin page that lets you clear page caches.
+This method takes an absolute path or regular expression.
+Returns the number of matching entries found in the index.
+A warning will be generated if the page index is disabled (see L</CONFIGURATION>).
-Note that clear_cached_page will generate a warning if disable_index is enabled.
+The argument is matched against all the keys in the page index.
+The page index keys include the request path and query string, if any.
+Typically you'd call this from a different controller than the cached
+controller. You may for example wish to build an admin page that lets you clear
+page caches.
+
=head1 INTERNAL EXTENDED METHODS
=head2 dispatch
@@ -769,6 +803,10 @@
=head1 KNOWN ISSUES
+The page index, used to support the L</clear_cached_page> method is unreliable
+because it uses a read-modify-write approach which will loose data if more than
+one process attempts to update the page index at the same time.
+
It is not currently possible to cache pages served from the Static plugin. If
you're concerned enough about performance to use this plugin, you should be
serving static files directly from your web server anyway.
Modified: trunk/Catalyst-Plugin-PageCache/t/04cache.t
===================================================================
--- trunk/Catalyst-Plugin-PageCache/t/04cache.t 2010-11-03 15:09:37 UTC (rev 13686)
+++ trunk/Catalyst-Plugin-PageCache/t/04cache.t 2010-11-03 16:01:23 UTC (rev 13687)
@@ -8,13 +8,6 @@
use Test::More;
use File::Path;
-BEGIN {
- eval "use Catalyst::Plugin::Cache";
- plan $@
- ? ( skip_all => 'needs Catalyst::Plugin::Cache for testing' )
- : ( tests => 20 );
-}
-
use Catalyst::Test 'TestApp';
# cache a page
@@ -27,6 +20,7 @@
# clear the cached page
ok( $res = request('http://localhost/cache/clear_cache'), 'request ok' );
+is( $res->content, 1, 'clear_cache removed 1 entry' );
# request again
ok( $res = request('http://localhost/cache/count'), 'request ok' );
@@ -38,6 +32,7 @@
# clear the cached page with the regex format
ok( $res = request('http://localhost/cache/clear_cache_regex'), 'request ok' );
+is( $res->content, 1, 'clear_cache removed 1 entry' );
# request again
ok( $res = request('http://localhost/cache/count'), 'request ok' );
@@ -61,3 +56,5 @@
# test cache of a page with a parameter-less query string
ok( $res = request('http://localhost/cache/count/2?foo'), 'request ok' );
is( $res->content, 8, 'count after query string is 8' );
+
+done_testing();
Modified: trunk/Catalyst-Plugin-PageCache/t/lib/TestApp/Controller/Cache.pm
===================================================================
--- trunk/Catalyst-Plugin-PageCache/t/lib/TestApp/Controller/Cache.pm 2010-11-03 15:09:37 UTC (rev 13686)
+++ trunk/Catalyst-Plugin-PageCache/t/lib/TestApp/Controller/Cache.pm 2010-11-03 16:01:23 UTC (rev 13687)
@@ -49,17 +49,17 @@
sub clear_cache : Local {
my ( $self, $c ) = @_;
- $c->clear_cached_page( '/cache/count' );
+ my $removed = $c->clear_cached_page( '/cache/count' );
- $c->res->output( 'ok' );
+ $c->res->output( $removed );
}
sub clear_cache_regex : Local {
my ( $self, $c ) = @_;
- $c->clear_cached_page( '/cache/.*' );
+ my $removed = $c->clear_cached_page( '/cache/.*' );
- $c->res->output( 'ok' );
+ $c->res->output( $removed );
}
sub test_datetime : Local {
Modified: trunk/Catalyst-Plugin-PageCache/t/lib/TestApp.pm
===================================================================
--- trunk/Catalyst-Plugin-PageCache/t/lib/TestApp.pm 2010-11-03 15:09:37 UTC (rev 13686)
+++ trunk/Catalyst-Plugin-PageCache/t/lib/TestApp.pm 2010-11-03 16:01:23 UTC (rev 13687)
@@ -14,6 +14,7 @@
name => 'TestApp',
counter => 0,
'Plugin::Cache' => {
+ disable_index => 0,
backend => {
class => 'Cache::FileCache',
cache_root => $cache_root,
Modified: trunk/Catalyst-Plugin-PageCache/t/lib/TestAppI18N.pm
===================================================================
--- trunk/Catalyst-Plugin-PageCache/t/lib/TestAppI18N.pm 2010-11-03 15:09:37 UTC (rev 13686)
+++ trunk/Catalyst-Plugin-PageCache/t/lib/TestAppI18N.pm 2010-11-03 16:01:23 UTC (rev 13687)
@@ -14,6 +14,7 @@
name => 'TestApp-I18N',
counter => 0,
'Plugin::Cache' => {
+ disable_index => 0,
backend => {
class => 'Cache::FileCache',
cache_root => $cache_root,
More information about the Catalyst-commits
mailing list