[mb-commits] branch, mbs-5741, created. Allow page timeouts to be configured per page, rather than per app
MusicBrainz Git Server <[email protected]> Tue, 08 Jan 2013 13:43:54 +0000
| Newsgroups | gmane.comp.audio.musicbrainz.cvs |
|---|---|
| Message-ID | <E1TsZT8-0004zQ-4L@wiley> |
The branch, mbs-5741 has been created
at ddf62ca934f2916fa4eeaeac9e21bd3faf0f3d15 (commit)
- Log -----------------------------------------------------------------
commit ddf62ca934f2916fa4eeaeac9e21bd3faf0f3d15
Author: Oliver Charles <[email protected]>
Date: Tue Jan 8 13:42:25 2013 +0000
Allow page timeouts to be configured per page, rather than per app
MAX_REQUEST_TIME is now DETERMINE_MAX_REQUEST_TIME, which is a function from
a Catalyst::Request to a duration in a seconds, that defines the amount of
time it can take for a response to be generated. This allows us to match
the request path to timeout specific pages at a different rate than others.
diff --git a/lib/DBDefs/Default.pm b/lib/DBDefs/Default.pm
index 5ec1059..a180ef8 100644
--- a/lib/DBDefs/Default.pm
+++ b/lib/DBDefs/Default.pm
@@ -391,12 +391,11 @@ sub AUTO_RESTART {
# }
}
-# The maximum amount of time a process can be serving a single request
-# If undef, the process is never killed
-# If set to a positive integer, the process can server a single request
-# for MAX_REQUEST_TIME seconds, and if it is still not done the process
-# will be killed (and log a message about the request it was serving).
-sub MAX_REQUEST_TIME { undef }
+# The maximum amount of time a process can be serving a single request. This
+# function takes a Catalyst::Request as input, and should return the amount of time
+# in seconds that it should take to respond to this request.
+# If undef, the process is never killed.
+sub DETERMINE_MAX_REQUEST_TIME { undef }
sub LOGGER_ARGUMENTS {
return (
diff --git a/lib/MusicBrainz/Server.pm b/lib/MusicBrainz/Server.pm
index 7e65979..c7468b4 100644
--- a/lib/MusicBrainz/Server.pm
+++ b/lib/MusicBrainz/Server.pm
@@ -320,9 +320,18 @@ after dispatch => sub {
# Timeout long running requests
-if(my $max_request_time = DBDefs->MAX_REQUEST_TIME) {
- around dispatch => sub {
- my ($orig, $c, @args) = @_;
+around dispatch => sub {
+ my ($orig, $c, @args) = @_;
+
+ my $max_request_time = DBDefs->DETERMINE_MAX_REQUEST_TIME($c->req);
+
+ if (defined($max_request_time) && $max_request_time > 0) {
+ my $context = $c->model('MB')->context;
+
+ if ($context->connector->conn->connected) {
+ $context->sql->do("SET statement_timeout = " .
+ ($max_request_time * 1000));
+ }
alarm($max_request_time);
POSIX::sigaction(
@@ -333,19 +342,19 @@ if(my $max_request_time = DBDefs->MAX_REQUEST_TIME) {
$c->log->error(Devel::StackTrace->new->as_string);
$c->log->_flush;
- if (my $sth = $c->model('MB')->context->sql->sth) {
+ if (my $sth = $context->sql->sth) {
$sth->cancel;
}
- $c->model('MB')->context->connector->disconnect;
+ $context->connector->disconnect;
exit(42)
}));
+ }
- $c->$orig(@args);
+ $c->$orig(@args);
- alarm(0);
- };
+ alarm(0);
};
around 'finalize_error' => sub {
diff --git a/lib/MusicBrainz/Server/Connector.pm b/lib/MusicBrainz/Server/Connector.pm
index d1ef746..d44d9c0 100644
--- a/lib/MusicBrainz/Server/Connector.pm
+++ b/lib/MusicBrainz/Server/Connector.pm
@@ -1,7 +1,6 @@
package MusicBrainz::Server::Connector;
use Moose;
-use DBDefs;
use DBIx::Connector;
use Sql;
@@ -50,10 +49,6 @@ sub _build_conn
my $dbh = shift;
$dbh->do("SET TIME ZONE 'UTC'");
$dbh->do("SET CLIENT_ENCODING = 'UNICODE'");
- $dbh->do("SET statement_timeout = " .
- (DBDefs->MAX_REQUEST_TIME() * 1000))
- if (defined(DBDefs->MAX_REQUEST_TIME)
- && DBDefs->MAX_REQUEST_TIME > 0);
if ($schema) {
$dbh->do("SET search_path=$schema,public");
commit bc195f8980b361395a2632ef6cc77c593eaed690
Author: Oliver Charles <[email protected]>
Date: Tue Jan 8 11:25:43 2013 +0000
Consolidate connection opening and closing
Ensure's that each web node opens one database connection and closes it at the
end of the request. This has required me to slightly change how things are laid
out in Server.pm - I've chosen to abstract each wrapper into its own method
modifier, so we can easily compose these wrappers and understand what each part
is trying to do.
I've moved the database statement timeout set up code into the Connector itself,
as previously doing it in Model::MB was causing the FastCGI process manager to
have a database connection open throughout the entire server lifetime, which is
wasteful.
The disconnection logic of the Connector has changed, as it would previously
open a connection if there was nothing to disconnect.
diff --git a/lib/MusicBrainz/Server.pm b/lib/MusicBrainz/Server.pm
index ac69ca7..7e65979 100644
--- a/lib/MusicBrainz/Server.pm
+++ b/lib/MusicBrainz/Server.pm
@@ -7,6 +7,7 @@ use Class::MOP;
use DBDefs;
use Encode;
use MusicBrainz::Server::Log qw( logger );
+use POSIX qw(SIGALRM);
use aliased 'MusicBrainz::Server::Translation';
@@ -240,43 +241,6 @@ sub relative_uri
return $uri;
}
-use POSIX qw(SIGALRM);
-
-around 'dispatch' => sub {
- my $orig = shift;
- my $c = shift;
-
- $c->model('MB')->context->connector->refresh;
-
- with_translations($c, sub {
- my $c = shift;
- my $orig = shift;
-
- if(my $max_request_time = DBDefs->MAX_REQUEST_TIME) {
- alarm($max_request_time);
- POSIX::sigaction(
- SIGALRM, POSIX::SigAction->new(sub {
- $c->log->error(sprintf("Request for %s took over %d seconds. Killing process",
- $c->req->uri,
- $max_request_time));
- $c->log->error(Devel::StackTrace->new->as_string);
- $c->log->_flush;
- if (my $sth = $c->model('MB')->context->sql->sth) {
- $sth->cancel;
- }
- exit(42)
- }));
-
- $c->$orig(@_);
-
- alarm(0);
- }
- else {
- $c->$orig(@_);
- }
- }, $orig, @_);
-};
-
sub gettext { shift; Translation->instance->gettext(@_) }
sub pgettext { shift; Translation->instance->pgettext(@_) }
sub ngettext { shift; Translation->instance->ngettext(@_) }
@@ -300,45 +264,9 @@ sub _handle_param_unicode_decoding {
};
}
-sub execute {
- my $c = shift;
- return do {
- local $SIG{__WARN__} = sub {
- my $warning = shift;
- chomp $warning;
- $c->log->warn($c->req->method . " " . $c->req->uri . " caused a warning: " . $warning);
- };
- $c->next::method(@_);
- };
-}
-
-around 'finalize_error' => sub {
- my $orig = shift;
- my $c = shift;
-
- with_translations($c, sub {
- my $c = shift;
- my $orig = shift;
-
- $c->$orig(@_);
-
- $c->model('MB')->context->connector->disconnect;
-
- if (!$c->debug && scalar @{ $c->error }) {
- $c->stash->{errors} = $c->error;
- $c->stash->{template} = 'main/500.tt';
- $c->stash->{stack_trace} = $c->_stacktrace;
- $c->clear_errors;
- $c->res->{body} = 'clear';
- $c->view('Default')->process($c);
- $c->res->{body} = encode('utf-8', $c->res->{body});
- }
- }, $orig, @_);
-};
-
-sub with_translations {
- my $c = shift;
- my $orig = shift;
+# Set and unset translation language
+around dispatch => sub {
+ my ($orig, $c, @args) = @_;
$_->instance->build_languages_from_header($c->req->headers)
for qw( MusicBrainz::Server::Translation
@@ -363,10 +291,83 @@ sub with_translations {
use_languages => scalar @{ Translation->instance->all_languages() }
);
- &$orig($c, @_);
+ $c->$orig(@args);
Translation->instance->unset_language();
-}
+};
+
+# All warnings should be logged
+around dispatch => sub {
+ my ($orig, $c, @args) = @_;
+
+ local $SIG{__WARN__} = sub {
+ my $warning = shift;
+ chomp $warning;
+ $c->log->warn($c->req->method . " " . $c->req->uri . " caused a warning: " . $warning);
+ };
+
+ $c->$orig(@args);
+};
+
+# Use a fresh database connection for every request, and remember to disconnect at the end
+before dispatch => sub {
+ shift->model('MB')->context->connector->refresh;
+};
+
+after dispatch => sub {
+ shift->model('MB')->context->connector->disconnect;
+};
+
+# Timeout long running requests
+
+if(my $max_request_time = DBDefs->MAX_REQUEST_TIME) {
+ around dispatch => sub {
+ my ($orig, $c, @args) = @_;
+
+ alarm($max_request_time);
+ POSIX::sigaction(
+ SIGALRM, POSIX::SigAction->new(sub {
+ $c->log->error(sprintf("Request for %s took over %d seconds. Killing process",
+ $c->req->uri,
+ $max_request_time));
+ $c->log->error(Devel::StackTrace->new->as_string);
+ $c->log->_flush;
+
+ if (my $sth = $c->model('MB')->context->sql->sth) {
+ $sth->cancel;
+ }
+
+ $c->model('MB')->context->connector->disconnect;
+
+ exit(42)
+ }));
+
+ $c->$orig(@args);
+
+ alarm(0);
+ };
+};
+
+around 'finalize_error' => sub {
+ my $orig = shift;
+ my $c = shift;
+ my @args = @_;
+
+ $c->with_translations(sub {
+ $c->$orig(@args);
+
+ if (!$c->debug && scalar @{ $c->error }) {
+ $c->stash->{errors} = $c->error;
+ $c->stash->{template} = 'main/500.tt';
+ $c->stash->{stack_trace} = $c->_stacktrace;
+ $c->clear_errors;
+ $c->res->{body} = 'clear';
+ $c->view('Default')->process($c);
+ $c->res->{body} = encode('utf-8', $c->res->{body});
+ }
+ });
+};
+
=head1 NAME
MusicBrainz::Server - Catalyst-based MusicBrainz server
@@ -377,6 +378,7 @@ MusicBrainz::Server - Catalyst-based MusicBrainz server
=head1 LICENSE
+Copyright (C) 2012 MetaBrainz Foundation
Copyright (C) 2008 Oliver Charles
Copyright (C) 2009 Lukas Lalinsky
diff --git a/lib/MusicBrainz/Server/Connector.pm b/lib/MusicBrainz/Server/Connector.pm
index ca14bad..d1ef746 100644
--- a/lib/MusicBrainz/Server/Connector.pm
+++ b/lib/MusicBrainz/Server/Connector.pm
@@ -50,6 +50,10 @@ sub _build_conn
my $dbh = shift;
$dbh->do("SET TIME ZONE 'UTC'");
$dbh->do("SET CLIENT_ENCODING = 'UNICODE'");
+ $dbh->do("SET statement_timeout = " .
+ (DBDefs->MAX_REQUEST_TIME() * 1000))
+ if (defined(DBDefs->MAX_REQUEST_TIME)
+ && DBDefs->MAX_REQUEST_TIME > 0);
if ($schema) {
$dbh->do("SET search_path=$schema,public");
@@ -69,7 +73,7 @@ sub _build_conn
sub _disconnect {
my ($self) = @_;
if (my $conn = $self->conn) {
- $conn->dbh->disconnect;
+ $conn->disconnect;
}
$self->_clear_conn;
diff --git a/lib/MusicBrainz/Server/Model/MB.pm b/lib/MusicBrainz/Server/Model/MB.pm
index 26ef183..f874777 100644
--- a/lib/MusicBrainz/Server/Model/MB.pm
+++ b/lib/MusicBrainz/Server/Model/MB.pm
@@ -31,12 +31,6 @@ sub _build_context {
my $c = MusicBrainz::Server::Context->new(
cache_manager => MusicBrainz::Server::CacheManager->new($cache_opts)
);
-
- $c->dbh->do("SET statement_timeout = " .
- (DBDefs->MAX_REQUEST_TIME() * 1000))
- if (defined(DBDefs->MAX_REQUEST_TIME)
- && DBDefs->MAX_REQUEST_TIME > 0);
-
return $c;
}
}
-----------------------------------------------------------------------
hooks/post-receive
--
mb_server