git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: John 'Warthog9' Hawley <warthog9@kernel.org>,
	John 'Warthog9' Hawley <warthog9@eaglescrag.net>,
	Junio C Hamano <gitster@pobox.com>, demerphq <demerphq@gmail.com>,
	Aevar Arnfjord Bjarmason <avarab@gmail.com>,
	Thomas Adam <thomas@xteddy.org>,
	Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH 18/24] gitweb/lib - Configure running 'generating_info' when generating data
Date: Tue,  7 Dec 2010 00:11:03 +0100	[thread overview]
Message-ID: <1291677069-6559-19-git-send-email-jnareb@gmail.com> (raw)
In-Reply-To: <1291677069-6559-1-git-send-email-jnareb@gmail.com>

Add a new 'generating_info_is_safe' cache option; if true, then
process generating data (one that acquired exclusive writer's lock)
would run 'generating_info' if there is no stale data and background
cache generation is enabled.

If function generating (or printing) exits, which leads to cache entry
not being generated (for gitweb this means that there are pages which
are not cached, i.e. error pages), and 'generating_info' also exits,
this could result in bad behavior.  Therefore this new option is false
by default.


Updates t9503 test appropriately.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch was not present in previous version of this series.

This is one solution to the problem mentioned in commit message;
another is to use initial delay in "Generating..." page, as it is done
in next patch, and as it was done in previous version of this series.

 gitweb/lib/GitwebCache/FileCacheWithLocking.pm |   17 ++++++++++++++---
 t/t9503/test_cache_interface.pl                |    1 +
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
index 694c318..0823c55 100644
--- a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
+++ b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
@@ -80,26 +80,35 @@ use POSIX qw(setsid);
 #    instead), for other process (or bacground process).  It is passed
 #    $cache instance, $key, and opened $lock_fh filehandle to lockfile.
 #    Unset by default (which means no activity indicator).
+#  * 'generating_info_is_safe' (boolean)
+#    If true, run 'generating_info' subroutine also in the project that
+#    is generating data.  This has effect only when 'background_cache'
+#    is true (both 'background_cache' and 'generating_info_is_safe' must
+#    be true for project generating data to run 'generating_info').
+#    Defaults to false for safety.
 sub new {
 	my $class = shift;
 	my %opts = ref $_[0] ? %{ $_[0] } : @_;
 
 	my $self = $class->SUPER::new(\%opts);
 
-	my ($max_lifetime, $background_cache, $generating_info);
+	my ($max_lifetime, $background_cache, $generating_info, $gen_info_is_safe);
 	if (%opts) {
 		$max_lifetime =
 			$opts{'max_lifetime'} ||
 			$opts{'max_cache_lifetime'};
 		$background_cache = $opts{'background_cache'};
 		$generating_info  = $opts{'generating_info'};
+		$gen_info_is_safe = $opts{'generating_info_is_safe'};
 	}
 	$max_lifetime = -1 unless defined($max_lifetime);
 	$background_cache = 1 unless defined($background_cache);
+	$gen_info_is_safe = 0 unless defined($gen_info_is_safe);
 
 	$self->set_max_lifetime($max_lifetime);
 	$self->set_background_cache($background_cache);
 	$self->set_generating_info($generating_info);
+	$self->set_generating_info_is_safe($gen_info_is_safe);
 
 	return $self;
 }
@@ -110,7 +119,8 @@ sub new {
 # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern
 
 # creates get_depth() and set_depth($depth) etc. methods
-foreach my $i (qw(max_lifetime background_cache generating_info)) {
+foreach my $i (qw(max_lifetime background_cache
+                  generating_info generating_info_is_safe)) {
 	my $field = $i;
 	no strict 'refs';
 	*{"get_$field"} = sub {
@@ -214,7 +224,8 @@ sub _set_maybe_background {
 		# to regenerate/refresh the cache (generate data),
 		# or if main process would show progress indicator
 		$pid = fork()
-			if (@stale_result || $self->{'generating_info'});
+			if (@stale_result ||
+			    ($self->{'generating_info'} && $self->{'generating_info_is_safe'}));
 	}
 
 	if ($pid) {
diff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl
index 81f0b76..480cfbc 100755
--- a/t/t9503/test_cache_interface.pl
+++ b/t/t9503/test_cache_interface.pl
@@ -333,6 +333,7 @@ subtest 'serving stale data when (re)generating' => sub {
 
 	# with background generation
 	$cache->set_background_cache(1);
+	$cache->set_generating_info_is_safe(1);
 
 	$cache->set($key, $stale_value);
 	$cache->set_expires_in(0);    # set value is now expired
-- 
1.7.3

  parent reply	other threads:[~2010-12-06 23:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-06 23:10 [PATCHv6/RFC 00/24] gitweb: Simple file based output caching Jakub Narebski
2010-12-06 23:10 ` [PATCH 01/24] t/test-lib.sh: Export also GIT_BUILD_DIR in test_external Jakub Narebski
2010-12-06 23:10 ` [PATCH 02/24] gitweb: Prepare for splitting gitweb Jakub Narebski
2010-12-06 23:10 ` [PATCH 03/24] gitweb/lib - Very simple file based cache Jakub Narebski
2010-12-06 23:10 ` [PATCH 04/24] gitweb/lib - Stat-based cache expiration Jakub Narebski
2010-12-06 23:10 ` [PATCH 05/24] gitweb/lib - Regenerate entry if the cache file has size of 0 Jakub Narebski
2010-12-06 23:10 ` [PATCH 06/24] gitweb/lib - Simple output capture by redirecting STDOUT Jakub Narebski
2010-12-06 23:10 ` [PATCH 07/24] gitweb/lib - Cache captured output (using get/set) Jakub Narebski
2010-12-06 23:10 ` [PATCH 08/24] gitweb: Add optional output caching Jakub Narebski
2010-12-06 23:10 ` [PATCH 09/24] gitweb/lib - Adaptive cache expiration time Jakub Narebski
2010-12-06 23:10 ` [PATCH 10/24] gitweb/lib - Use CHI compatibile (compute method) caching interface Jakub Narebski
2010-12-06 23:10 ` [PATCH 11/24] gitweb/lib - capture output directly to cache entry file Jakub Narebski
2010-12-06 23:10 ` [PATCH 12/24] gitweb/lib - Use locking to avoid 'cache miss stampede' problem Jakub Narebski
2010-12-06 23:10 ` [PATCH 13/24] gitweb/lib - No need for File::Temp when locking Jakub Narebski
2010-12-06 23:10 ` [PATCH 14/24] gitweb/lib - Serve stale data when waiting for filling cache Jakub Narebski
2010-12-06 23:11 ` [PATCH 15/24] gitweb/lib - Regenerate (refresh) cache in background Jakub Narebski
2010-12-06 23:11 ` [PATCH 16/24] gitweb: Introduce %actions_info, gathering information about actions Jakub Narebski
2010-12-06 23:11 ` [PATCH 17/24] gitweb: Show appropriate "Generating..." page when regenerating cache Jakub Narebski
2010-12-06 23:11 ` Jakub Narebski [this message]
2010-12-06 23:11 ` [PATCH 19/24] gitweb: Add startup delay to activity indicator for cache Jakub Narebski
2010-12-06 23:11 ` [PATCH/RFC 20/24] gitweb/lib - Add support for setting error handler in cache Jakub Narebski
2010-12-06 23:11 ` [PATCH/RFC 21/24] gitweb: Wrap die_error to use as error handler for caching engine Jakub Narebski
2010-12-06 23:11 ` [PATCH/RFC 22/24] gitweb: Support legacy options used by kernel.org " Jakub Narebski
2010-12-06 23:11 ` [RFC/PATCH 23/24] gitweb/lib - Add clear() and size() methods to caching interface Jakub Narebski
2010-12-06 23:11 ` [RFC PATCH 24/24] gitweb: Add beginnings of cache administration page (proof of concept) Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1291677069-6559-19-git-send-email-jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=avarab@gmail.com \
    --cc=demerphq@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=thomas@xteddy.org \
    --cc=warthog9@eaglescrag.net \
    --cc=warthog9@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).