From: John Napiorkowski Date: Fri, 9 May 2008 15:53:45 +0000 (+0000) Subject: changed the balancer to a role, created a new class to define the default balancer... X-Git-Tag: v0.08240~402^2~44 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=17b05c131035f73964c434c1a9c8b28e46aebeeb;p=dbsrgits%2FDBIx-Class.git changed the balancer to a role, created a new class to define the default balancer behavior, finished the autovalidate code and added tests for all the above --- diff --git a/lib/DBIx/Class/Storage/DBI/Replicated.pm b/lib/DBIx/Class/Storage/DBI/Replicated.pm index 7323b42..a70a8b5 100644 --- a/lib/DBIx/Class/Storage/DBI/Replicated.pm +++ b/lib/DBIx/Class/Storage/DBI/Replicated.pm @@ -73,18 +73,6 @@ has 'pool_type' => ( }, ); -=head2 pool_args - -Contains a hashref of initialized information to pass to the Pool object. See -L for available arguments. - -=cut - -has 'pool_args' => ( - is=>'ro', - isa=>'HashRef', -); - =head2 balancer_type The replication pool requires a balance class to provider the methods for @@ -101,6 +89,18 @@ has 'balancer_type' => ( }, ); +=head2 balancer_args + +Contains a hashref of initialized information to pass to the Balancer object. +See L for available arguments. + +=cut + +has 'balancer_args' => ( + is=>'ro', + isa=>'HashRef', +); + =head2 pool Is a or derived class. This is a @@ -116,9 +116,6 @@ has 'pool' => ( connect_replicants replicants has_replicants - num_replicants - delete_replicant - validate_replicants /], ); @@ -133,6 +130,7 @@ has 'balancer' => ( is=>'ro', isa=>'DBIx::Class::Storage::DBI::Replicated::Balancer', lazy_build=>1, + handles=>[qw/auto_validate_every/], ); =head2 master @@ -272,7 +270,7 @@ Lazy builder for the L attribute. sub _build_pool { my $self = shift @_; - $self->create_pool($self->pool_args); + $self->create_pool; } =head2 _build_balancer_type @@ -282,7 +280,7 @@ Lazy builder for the L attribute. =cut sub _build_balancer_type { - return 'DBIx::Class::Storage::DBI::Replicated::Balancer'; + return 'DBIx::Class::Storage::DBI::Replicated::Balancer::First'; } =head2 _build_balancer @@ -296,7 +294,8 @@ sub _build_balancer { my $self = shift @_; $self->create_balancer( pool=>$self->pool, - master=>$self->master); + master=>$self->master, + %{$self->balancer_args},); } =head2 _build_write_handler diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm index f69ce8c..ed7007d 100644 --- a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm +++ b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm @@ -1,6 +1,7 @@ package DBIx::Class::Storage::DBI::Replicated::Balancer; -use Moose; +use Moose::Role; +requires 'next_storage'; =head1 NAME @@ -8,8 +9,7 @@ DBIx::Class::Storage::DBI::Replicated::Balancer; A Software Load Balancer =head1 SYNOPSIS -This class is used internally by L. You -shouldn't need to create instances of this class. +This role is used internally by L. =head1 DESCRIPTION @@ -32,26 +32,7 @@ validating every query. has 'auto_validate_every' => ( is=>'rw', isa=>'Int', - predicate=>'had_auto_validate_every', -); - -=head2 last_validated - -This is an integer representing a time since the last time the replicants were -validated. It's nothing fancy, just an integer provided via the perl time -builtin. - -=cut - -has 'last_validated' => ( - is=>'rw', - isa=>'Int', - reader=>'last_validated', - writer=>'_last_validated', - lazy=>1, - default=>sub { - time; - }, + predicate=>'has_auto_validate_every', ); =head2 master @@ -122,6 +103,8 @@ sub _build_current_replicant { =head2 next_storage +This method should be defined in the class which consumes this role. + Given a pool object, return the next replicant that will serve queries. The default behavior is to grap the first replicant it finds but you can write your own subclasses of L to @@ -130,26 +113,32 @@ support other balance systems. This returns from the pool of active replicants. If there are no active replicants, then you should have it return the master as an ultimate fallback. -TODO this needs to wrap for the subclasses better. Maybe good use of INNER? +=head2 around: next_storage + +Advice on next storage to add the autovalidation. We have this broken out so +that it's easier to break out the auto validation into a role. + +This also returns the master in the case that none of the replicants are active +or just just forgot to create them :) =cut -sub next_storage { - my $self = shift @_; - - ## Do we need to validate the replicants? - if( - $self->had_auto_validate_every && - ($self->auto_validate_every + $self->last_validated) > time - ) { - $self->pool->validate_replicants; - $self->_last_validated(time); - } +around 'next_storage' => sub { + my ($next_storage, $self, @args) = @_; + my $now = time; - ## Get a replicant, or the master if none - my $next = ($self->pool->active_replicants)[0]; - return $next ? $next:$self->master; -} + ## Do we need to validate the replicants? + if( + $self->has_auto_validate_every && + ($self->auto_validate_every + $self->pool->last_validated) <= $now + ) { + $self->pool->validate_replicants; + } + + ## Get a replicant, or the master if none + my $next = $self->$next_storage(@args); + return $next ? $next:$self->master; +}; =head2 before: select diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/First.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/First.pm new file mode 100644 index 0000000..495b41d --- /dev/null +++ b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/First.pm @@ -0,0 +1,52 @@ +package DBIx::Class::Storage::DBI::Replicated::Balancer::First; + +use List::Util qw(shuffle); +use Moose; +with 'DBIx::Class::Storage::DBI::Replicated::Balancer'; + +=head1 NAME + +DBIx::Class::Storage::DBI::Replicated::Balancer::First; Just get the First Balancer + +=head1 SYNOPSIS + +This class is used internally by L. You +shouldn't need to create instances of this class. + +=head1 DESCRIPTION + +Given a pool (L) of replicated +database's (L), defines a +method by which query load can be spread out across each replicant in the pool. + +This Balancer just get's whatever is the first replicant in the pool + +=head1 ATTRIBUTES + +This class defines the following attributes. + +=head1 METHODS + +This class defines the following methods. + +=head2 next_storage + +Just get the first storage. Probably only good when you have one replicant. + +=cut + +sub next_storage { + return (shift->pool->active_replicants)[0]; +} + +=head1 AUTHOR + +John Napiorkowski + +=head1 LICENSE + +You may distribute this code under the same terms as Perl itself. + +=cut + +1; \ No newline at end of file diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm index 6c66ea8..e37b291 100644 --- a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm +++ b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm @@ -2,11 +2,11 @@ package DBIx::Class::Storage::DBI::Replicated::Balancer::Random; use List::Util qw(shuffle); use Moose; -extends 'DBIx::Class::Storage::DBI::Replicated::Balancer'; +with 'DBIx::Class::Storage::DBI::Replicated::Balancer'; =head1 NAME -DBIx::Class::Storage::DBI::Replicated::Balancer; A Software Load Balancer +DBIx::Class::Storage::DBI::Replicated::Balancer::Random; A 'random' Balancer =head1 SYNOPSIS @@ -40,9 +40,7 @@ be requested several times in a row. =cut sub next_storage { - my $self = shift @_; - my $next = (shuffle($self->pool->active_replicants))[0]; - return $next ? $next : $self->master; + return (shuffle(shift->pool->active_replicants))[0]; } =head1 AUTHOR diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Pool.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Pool.pm index 4f406c3..123cc47 100644 --- a/lib/DBIx/Class/Storage/DBI/Replicated/Pool.pm +++ b/lib/DBIx/Class/Storage/DBI/Replicated/Pool.pm @@ -43,6 +43,25 @@ has 'maximum_lag' => ( default=>0, ); +=head2 last_validated + +This is an integer representing a time since the last time the replicants were +validated. It's nothing fancy, just an integer provided via the perl time +builtin. + +=cut + +has 'last_validated' => ( + is=>'rw', + isa=>'Int', + reader=>'last_validated', + writer=>'_last_validated', + lazy=>1, + default=>sub { + time; + }, +); + =head2 replicant_type ($classname) Base class used to instantiate replicants that are in the pool. Unless you @@ -229,6 +248,9 @@ sub validate_replicants { $replicant->active(0); } } + + ## Mark that we completed this validation. + $self->_last_validated(time); } =head1 AUTHOR diff --git a/t/93storage_replication.t b/t/93storage_replication.t index 20739da..1156a24 100644 --- a/t/93storage_replication.t +++ b/t/93storage_replication.t @@ -8,7 +8,7 @@ BEGIN { eval "use Moose; use Test::Moose"; plan $@ ? ( skip_all => 'needs Moose for testing' ) - : ( tests => 46 ); + : ( tests => 50 ); } use_ok 'DBIx::Class::Storage::DBI::Replicated::Pool'; @@ -64,6 +64,9 @@ TESTSCHEMACLASSES: { storage_type=>[ '::DBI::Replicated' => { balancer_type=>'::Random', + balancer_args=>{ + auto_validate_every=>100, + }, } ], deploy_args=>{ @@ -195,7 +198,7 @@ isa_ok $replicated->schema->storage->master isa_ok $replicated->schema->storage->pool => 'DBIx::Class::Storage::DBI::Replicated::Pool'; -isa_ok $replicated->schema->storage->balancer +does_ok $replicated->schema->storage->balancer => 'DBIx::Class::Storage::DBI::Replicated::Balancer'; ok my @replicant_connects = $replicated->generate_replicant_connect_info @@ -210,7 +213,7 @@ isa_ok $replicated->schema->storage->balancer->current_replicant ok $replicated->schema->storage->pool->has_replicants => 'does have replicants'; -is $replicated->schema->storage->num_replicants => 2 +is $replicated->schema->storage->pool->num_replicants => 2 => 'has two replicants'; does_ok $replicated_storages[0] @@ -369,7 +372,7 @@ SKIP: { ## We skip this tests unless you have a custom replicants, since the default ## sqlite based replication tests don't support these functions. - skip 'Cannot Test Replicant Status on Non Replicating Database', 3 + skip 'Cannot Test Replicant Status on Non Replicating Database', 9 unless DBICTest->has_custom_dsn && $ENV{"DBICTEST_SLAVE0_DSN"}; $replicated->replicate; ## Give the slaves a chance to catchup. @@ -403,6 +406,31 @@ SKIP: { is $replicated->schema->storage->pool->active_replicants, 2 => 'Both replicants in good standing again'; + + ## Check auto validate + + is $replicated->schema->storage->balancer->auto_validate_every, 100 + => "Got the expected value for auto validate"; + + ## This will make sure we auto validatge everytime + $replicated->schema->storage->balancer->auto_validate_every(0); + + ## set all the replicants to inactive, and make sure the balancer falls back to + ## the master. + + $replicated->schema->storage->replicants->{$replicant_names[0]}->active(0); + $replicated->schema->storage->replicants->{$replicant_names[1]}->active(0); + + ## Ok, now when we go to run a query, autovalidate SHOULD reconnect + + is $replicated->schema->storage->pool->active_replicants => 0 + => "both replicants turned off"; + + ok $replicated->schema->resultset('Artist')->find(5) + => 'replicant reactivated'; + + is $replicated->schema->storage->pool->active_replicants => 2 + => "both replicants reactivated"; } ## Delete the old database files