git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 15/24] gitweb/lib - Regenerate (refresh) cache in background

From
Jakub Narebski <jnareb@gmail.com>
Date
Dec 6, 2010, 23:11 UTC
Message-ID
<1291677069-6559-16-git-send-email-jnareb@gmail.com>
In-Reply-To
<1291677069-6559-1-git-send-email-jnareb@gmail.com>

This commit removes asymmetry in serving stale data (if stale data exists) when regenerating cache in GitwebCache::FileCacheWithLocking. The process that acquired exclusive (writers) lock, and is therefore selected to be the one that (re)generates data to fill the cache, can now generate data in background, while serving stale data.

Those background processes are daemonized, i.e. detached from the main
process (the one returning data or stale data).  Otherwise there might be a
problem when gitweb is running as (part of) long-lived process, for example
from mod_perl or from FastCGI: it would leave unreaped children as zombies
(entries in process table).  We don't want to wait for background process,
and we can't set $SIG{CHLD} to 'IGNORE' in gitweb to automatically reap
child processes, because this interferes with using
  open my $fd, '-|', git_cmd(), 'param', ...
    or die_error(...)
  # read from <$fd>
  close $fd
    or die_error(...)
In the above code "close" for magic "-|" open calls waitpid...  and we
would would die with "No child processes".  Removing 'or die' would
possibly remove ability to react to other errors.

This feature can be enabled or disabled on demand via 'background_cache' cache parameter. It is turned on by default.

The t9503 test got updated to test both case with background generation enabled and case with background generation disabled.

Inspired-by-code-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Compared to previous version of this series ->_set_maybe_background
was extracted (refactored) from ->_compute_generic (in earlier version
it would be just ->compute).  This hopefully makes code easier to
understand.
Differences to approach taken in "Gitweb caching v7" by J.H.
* It is made explicit that background generation depends on using
  locking.  It doesn't matter for J.H. series, as you canot turn off
  using locking there.
* Forking (running generating process in background) is done only if
  there is stale data to serve (and if background cache is turned on).
  In J.H. series forking was done unconditionally, only generation or
  exit depended on $backgroundCache (and technical/for debugging
  $cacheDoFork).
* Locking is done before forking, as forking background process is done
  only for the process regenerating cache.
* Daemonizes background process, detaching it from parent (using
  setsid).  This way whether main process is short-lived (gitweb as CGI)
  or long-lived (mod_perl, PSGI or FastCGI), there would be no need to
  wait and no zombies.
 gitweb/gitweb.perl                             |    9 +++
 gitweb/lib/GitwebCache/FileCacheWithLocking.pm |   64 ++++++++++++++++++++++--
 t/t9503/test_cache_interface.pl                |   40 ++++++++++++++-
 3 files changed, 106 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 454766c..f202d6b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -335,6 +335,15 @@ our %cache_options = (
 	# Set it to -1 to always serve existing data if it exists,
 	# set it to 0 to turn off serving stale data - always wait.
 	'max_lifetime' => 5*60*60, # 5 hours
+
+	# This enables/disables background caching.  If it is set to true value,
+	# caching engine would return stale data (if it is not older than
+	# 'max_lifetime' seconds) if it exists, and launch process if regenerating
+	# (refreshing) cache into the background.  If it is set to false value,
+	# the process that fills cache must always wait for data to be generated.
+	# In theory this will make gitweb seem more responsive at the price of
+	# serving possibly stale data.
+	'background_cache' => 1,
 );
 # Set to _initialized_ instance of GitwebCache::Capture compatibile capturing
 # engine, i.e. one implementing ->new() constructor, and ->capture($code)
diff --git a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
index 1d32810..82e88f1 100644
--- a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
+++ b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
@@ -23,6 +23,7 @@ use warnings;
 
 use File::Path qw(mkpath);
 use Fcntl qw(:flock);
+use POSIX qw(setsid);
 
 # ......................................................................
 # constructor
@@ -70,21 +71,27 @@ use Fcntl qw(:flock);
 #    than it, serve stale data when waiting for cache entry to be 
 #    regenerated (refreshed).  Non-adaptive.
 #    Defaults to -1 (never expire / always serve stale).
+#  * 'background_cache' (boolean)
+#    This enables/disables regenerating cache in background process.
+#    Defaults to true.
 sub new {
 	my $class = shift;
 	my %opts = ref $_[0] ? %{ $_[0] } : @_;
 
 	my $self = $class->SUPER::new(\%opts);
 
-	my ($max_lifetime);
+	my ($max_lifetime, $background_cache);
 	if (%opts) {
 		$max_lifetime =
 			$opts{'max_lifetime'} ||
 			$opts{'max_cache_lifetime'};
+		$background_cache = $opts{'background_cache'};
 	}
 	$max_lifetime = -1 unless defined($max_lifetime);
+	$background_cache = 1 unless defined($background_cache);
 
 	$self->set_max_lifetime($max_lifetime);
+	$self->set_background_cache($background_cache);
 
 	return $self;
 }
@@ -95,7 +102,7 @@ sub new {
 # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern
 
 # creates get_depth() and set_depth($depth) etc. methods
-foreach my $i (qw(max_lifetime)) {
+foreach my $i (qw(max_lifetime background_cache)) {
 	my $field = $i;
 	no strict 'refs';
 	*{"get_$field"} = sub {
@@ -146,6 +153,52 @@ sub _tempfile_to_path {
 # ......................................................................
 # interface methods
 
+sub _set_maybe_background {
+	my ($self, $key, $fetch_code, $set_code) = @_;
+
+	my $pid;
+	my (@result, @stale_result);
+
+	if ($self->{'background_cache'}) {
+		# try to retrieve stale data
+		@stale_result = $fetch_code->()
+			if $self->is_valid($key, $self->get_max_lifetime());
+
+		# fork if there is stale data, for background process
+		# to regenerate/refresh the cache (generate data)
+		$pid = fork() if (@stale_result);
+	}
+
+	if ($pid) {
+		## forked and are in parent process
+		# reap child, which spawned grandchild process (detaching it)
+		waitpid $pid, 0;
+
+	}	else {
+		## didn't fork, or are in background process
+
+		# daemonize background process, detaching it from parent
+		# see also Proc::Daemonize, Apache2::SubProcess
+		if (defined $pid) {
+			## in background process
+			POSIX::setsid(); # or setpgrp(0, 0);
+			fork() && CORE::exit(0);
+		}
+
+		@result = $set_code->();
+
+		if (defined $pid) {
+			## in background process; parent will serve stale data
+
+			# lockfile will be automatically closed on exit,
+			# and therefore lockfile would be unlocked
+			CORE::exit(0);
+		}
+	}
+
+	return @result > 0 ? @result : @stale_result;
+}
+
 sub _compute_generic {
 	my ($self, $key,
 	    $get_code, $fetch_code, $set_code, $fetch_locked) = @_;
@@ -162,16 +215,19 @@ sub _compute_generic {
 	do {
 		open my $lock_fh, '+>', $lockfile
 			or die "Could't open lockfile '$lockfile': $!";
+
 		$lock_state = flock($lock_fh, LOCK_EX | LOCK_NB);
 		if ($lock_state) {
-			# acquired writers lock
-			@result = $set_code->();
+			## acquired writers lock, have to generate data
+			@result = $self->_set_maybe_background($key, $fetch_code, $set_code);
 
 			# closing lockfile releases lock
 			close $lock_fh
 				or die "Could't close lockfile '$lockfile': $!";
 
 		} else {
+			## didn't acquire writers lock, get stale data or wait for regeneration
+
 			# try to retrieve stale data
 			@result = $fetch_code->()
 				if $self->is_valid($key, $self->get_max_lifetime());
diff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl
index 8a52261..7f08863 100755
--- a/t/t9503/test_cache_interface.pl
+++ b/t/t9503/test_cache_interface.pl
@@ -24,9 +24,13 @@ diag("Testing '$INC{'GitwebCache/FileCacheWithLocking.pm'}'");
 
 my $cache = new_ok('GitwebCache::FileCacheWithLocking', [ {
 	'max_lifetime' => 0, # turn it off
+	'background_cache' => 0,
 } ]);
 isa_ok($cache, 'GitwebCache::SimpleFileCache');
 
+# compute can fork, don't generate zombies
+#local $SIG{CHLD} = 'IGNORE';
+
 # Test that default values are defined
 #
 ok(defined $GitwebCache::SimpleFileCache::DEFAULT_CACHE_ROOT,
@@ -303,6 +307,9 @@ subtest 'parallel access' => sub {
 my $stale_value = 'Stale Value';
 
 subtest 'serving stale data when (re)generating' => sub {
+	# without background generation
+	$cache->set_background_cache(0);
+
 	$cache->set($key, $stale_value);
 	$call_count = 0;
 	$cache->set_expires_in(0);    # expire now
@@ -312,12 +319,39 @@ subtest 'serving stale data when (re)generating' => sub {
 		my $data = cache_compute($cache, $key, \&get_value_slow);
 		print $data;
 	};
-	ok(scalar(grep { $_ eq $stale_value } @output),
-	   'stale data in at least one process when expired');
+	# returning stale data works
+	is_deeply(
+		[sort @output],
+		[sort ($value, $stale_value)],
+		'no background: stale data returned by one process'
+	);
+
+	$cache->set_expires_in(-1); # never expire for next ->get
+	is($cache->get($key), $value,
+	   'no background: value got set correctly, even if stale data returned');
+
+
+	# with background generation
+	$cache->set_background_cache(1);
+
+	$cache->set($key, $stale_value);
+	$cache->set_expires_in(0);    # set value is now expired
+	@output = parallel_run {
+		my $data = cache_compute($cache, $key, \&get_value_slow);
+		print $data;
+	};
+	# returning stale data works
+	is_deeply(
+		\@output,
+		[ ($stale_value) x 2 ],
+		'background: stale data returned by both process when expired'
+	);
 
 	$cache->set_expires_in(-1); # never expire for next ->get
+	note('waiting for background process to have time to set data');
+	sleep $slow_time; # wait for background process to have chance to set data
 	is($cache->get($key), $value,
-	   'value got set correctly, even if stale data returned');
+	   'background: value got set correctly by background process');
 
 
 # 	$cache->set($key, $stale_value);

-- 
1.7.3
Previous: Jakub NarebskiNext: Jakub Narebski
Message 16 of 25 in “[PATCHv6/RFC 00/24] gitweb: Simple file based output caching”
  1. Jakub NarebskiDec 6, 2010
  2. 01/24 t/test-lib.sh: Export also GIT_BUILD_DIR in test_externalJakub Narebski, Dec 6, 2010
  3. 02/24 gitweb: Prepare for splitting gitwebJakub Narebski, Dec 6, 2010
  4. 03/24 gitweb/lib - Very simple file based cacheJakub Narebski, Dec 6, 2010
  5. 04/24 gitweb/lib - Stat-based cache expirationJakub Narebski, Dec 6, 2010
  6. 05/24 gitweb/lib - Regenerate entry if the cache file has size of 0Jakub Narebski, Dec 6, 2010
  7. 06/24 gitweb/lib - Simple output capture by redirecting STDOUTJakub Narebski, Dec 6, 2010
  8. 07/24 gitweb/lib - Cache captured output (using get/set)Jakub Narebski, Dec 6, 2010
  9. 08/24 gitweb: Add optional output cachingJakub Narebski, Dec 6, 2010
  10. 09/24 gitweb/lib - Adaptive cache expiration timeJakub Narebski, Dec 6, 2010
  11. 10/24 gitweb/lib - Use CHI compatibile (compute method) caching interfaceJakub Narebski, Dec 6, 2010
  12. 11/24 gitweb/lib - capture output directly to cache entry fileJakub Narebski, Dec 6, 2010
  13. 12/24 gitweb/lib - Use locking to avoid 'cache miss stampede' problemJakub Narebski, Dec 6, 2010
  14. 13/24 gitweb/lib - No need for File::Temp when lockingJakub Narebski, Dec 6, 2010
  15. 14/24 gitweb/lib - Serve stale data when waiting for filling cacheJakub Narebski, Dec 6, 2010
  16. 15/24 gitweb/lib - Regenerate (refresh) cache in backgroundJakub Narebski, Dec 6, 2010
  17. 16/24 gitweb: Introduce %actions_info, gathering information about actionsJakub Narebski, Dec 6, 2010
  18. 17/24 gitweb: Show appropriate "Generating..." page when regenerating cacheJakub Narebski, Dec 6, 2010
  19. 18/24 gitweb/lib - Configure running 'generating_info' when generating dataJakub Narebski, Dec 6, 2010
  20. 19/24 gitweb: Add startup delay to activity indicator for cacheJakub Narebski, Dec 6, 2010
  21. 20/24 gitweb/lib - Add support for setting error handler in cacheJakub Narebski, Dec 6, 2010
  22. 21/24 gitweb: Wrap die_error to use as error handler for caching engineJakub Narebski, Dec 6, 2010
  23. 22/24 gitweb: Support legacy options used by kernel.org caching engineJakub Narebski, Dec 6, 2010
  24. 23/24 gitweb/lib - Add clear() and size() methods to caching interfaceJakub Narebski, Dec 6, 2010
  25. 24/24 gitweb: Add beginnings of cache administration page (proof of concept)Jakub Narebski, Dec 6, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.