Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>
---
This patch, as you can see, lack proper commit message: it is work in
progress.
As you can see we can almost do the same for the cache which supports
only get/set interface... but for replacing ->retrieve() with ->get()
in ->compute(). This is argument for generic_compute subroutine,
mentioned in previous patch.
Note that we actually can and do test that provided mechanism avoid
cache miss stampede (aka 'stampeding herd') problem... although the
test should probably be run only with --long (this would need update
to t/test-lib.sh to pass GIT_TEST_LONG to external tests in
test_external and the like), as we need to sleep at least one second
to ensure that we would have 'stampeding herd' problem.
gitweb/cache.pm | 27 ++++++++++++++--
t/t9503/test_cache_interface.pl | 66 +++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/gitweb/cache.pm b/gitweb/cache.pm
index 8dd4f39..f514ee9 100644
--- a/gitweb/cache.pm
+++ b/gitweb/cache.pm
@@ -24,6 +24,7 @@ use File::Path qw(make_path); # requires version >= 2.0
use File::Spec;
use File::Temp;
use Digest::MD5 qw(md5_hex);
+use Fcntl qw(:flock);
# by default, the cache nests all entries on the filesystem two
# directories deep
@@ -217,7 +218,7 @@ sub _path_to_key {
my ($self, $p_namespace, $p_key) = @_;
return $self->_path_to_hashed_key($p_namespace,
- _Build_Hashed_Key($p_key));
+ _Build_Hashed_Key($p_key));
}
# Take hashed key, and return file path
@@ -228,6 +229,13 @@ sub _path_to_hashed_key {
_Split_Word($p_hashed_key, $self->get_depth()));
}
+sub _lockfile_to_key {
+ my ($self, $p_namespace, $p_key) = @_;
+
+ return $self->_path_to_hashed_key($p_namespace,
+ _Build_Hashed_Key($p_key)) . '.lock';
+}
+
# Split word into N components, where each component but last is two-letter word
# e.g. _Split_Word("06b90e786e304a18fdfbd7c7bcc41a6b", 2) == qw(06 b90e786e304a18fdfbd7c7bcc41a6b);
# _Split_Word("06b90e786e304a18fdfbd7c7bcc41a6b", 3) == qw(06 b9 0e786e304a18fdfbd7c7bcc41a6b);
@@ -412,17 +420,30 @@ sub compute {
my ($self, $p_key, $p_coderef) = @_;
my $data = $self->get($p_key);
- if (!defined $data) {
+ return $data if defined $data;
+
+ my $lockfile = $self->_lockfile_to_key($self->get_namespace(), $p_key);
+ _Make_Path($lockfile);
+ open my $lock_fh, '+>', $lockfile;
+ # or die "Can't open lockfile '$lockfile': $!";
+ if (my $lock_state = flock($lock_fh, LOCK_EX | LOCK_NB)) {
+ # acquired writers lock
$data = $p_coderef->($self, $p_key);
$self->set($p_key, $data);
+ } else {
+ # get readers lock
+ flock($lock_fh, LOCK_SH);
+ $data = $self->restore($self->get_namespace(), $p_key);
}
-
+ close $lock_fh;
return $data;
}
1;
} # end of package GitwebCache::SimpleFileCache;
+# ======================================================================
+
# human readable key identifying gitweb output
sub gitweb_output_key {
return href(-replay => 1, -full => 1, -path_info => 0);
diff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl
index 0870b87..43b806d 100755
--- a/t/t9503/test_cache_interface.pl
+++ b/t/t9503/test_cache_interface.pl
@@ -65,6 +65,8 @@ $cache->set($key, $value);
is($cache->get($key), $value, 'get after set, returns cached value');
$cache->remove($key);
ok(!defined($cache->get($key)), 'get after remove, is undefined');
+eval { $cache->remove('Not-Existent Key'); };
+ok(!$@, 'remove on non-existent key doesn\'t die');
# Test the getting and setting of a cached value
# (CHI interface)
@@ -80,6 +82,70 @@ is($cache->compute($key, \&get_value), $value, 'compute 2nd time (get)');
is($cache->compute($key, \&get_value), $value, 'compute 3rd time (get)');
cmp_ok($call_count, '==', 1, 'get_value() is called once');
+# Test 'stampeding herd' / cache miss stampede problem
+# (probably should be run only if GIT_TEST_LONG)
+sub get_value_slow {
+ $call_count++;
+ sleep 1;
+ return $value;
+}
+my ($pid, $kid_fh);
+
+$call_count = 0;
+$cache->remove($key);
+$pid = open $kid_fh, '-|';
+SKIP: {
+ skip "cannot fork: $!", 1
+ unless defined $pid;
+
+ my $data = $cache->get($key);
+ if (!defined $data) {
+ $data = get_value_slow();
+ $cache->set($key, $data);
+ }
+
+ if ($pid) {
+ my $child_count = <$kid_fh>;
+ chomp $child_count;
+
+ waitpid $pid, 0;
+ close $kid_fh;
+
+ $call_count += $child_count;
+ } else {
+ print "$call_count\n";
+ exit 0;
+ }
+
+ cmp_ok($call_count, '==', 2, 'parallel get/set: get_value_slow() called twice');
+}
+
+$call_count = 0;
+$cache->remove($key);
+$pid = open $kid_fh, '-|';
+SKIP: {
+ skip "cannot fork: $!", 1
+ unless defined $pid;
+
+ my $data = $cache->compute($key, \&get_value_slow);
+
+ if ($pid) {
+ my $child_count = <$kid_fh>;
+ chomp $child_count;
+
+ waitpid $pid, 0;
+ close $kid_fh;
+
+ $call_count += $child_count;
+ } else {
+ print "$call_count\n";
+ exit 0;
+ }
+
+ cmp_ok($call_count, '==', 1, 'parallel compute: get_value_slow() called once');
+}
+
+
# Test cache expiration for 'expire now'
#
$cache->set_expires_min(0);
--
1.6.6.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html