changed the balancer to a role, created a new class to define the default balancer...
John Napiorkowski [Fri, 9 May 2008 15:53:45 +0000 (15:53 +0000)]
lib/DBIx/Class/Storage/DBI/Replicated.pm
lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm
lib/DBIx/Class/Storage/DBI/Replicated/Balancer/First.pm [new file with mode: 0644]
lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm
lib/DBIx/Class/Storage/DBI/Replicated/Pool.pm
t/93storage_replication.t

index 7323b42..a70a8b5 100644 (file)
@@ -73,18 +73,6 @@ has 'pool_type' => (
     },
 );
 
-=head2 pool_args
-
-Contains a hashref of initialized information to pass to the Pool object.  See
-L<DBIx::Class::Storage::Replicated::Pool> 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<DBIx::Class::Storage::Replicated::Pool> for available arguments.
+
+=cut
+
+has 'balancer_args' => (
+    is=>'ro',
+    isa=>'HashRef',
+);
+
 =head2 pool
 
 Is a <DBIx::Class::Storage::DBI::Replicated::Pool> 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</pool> 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</balancer_type> 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
index f69ce8c..ed7007d 100644 (file)
@@ -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<DBIx::Class::Storage::DBI::Replicated>.  You
-shouldn't need to create instances of this class.
+This role is used internally by L<DBIx::Class::Storage::DBI::Replicated>.
     
 =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<DBIx::Class::Storage::DBI::Replicated::Balancer> 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 (file)
index 0000000..495b41d
--- /dev/null
@@ -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<DBIx::Class::Storage::DBI::Replicated>.  You
+shouldn't need to create instances of this class.
+    
+=head1 DESCRIPTION
+
+Given a pool (L<DBIx::Class::Storage::DBI::Replicated::Pool>) of replicated
+database's (L<DBIx::Class::Storage::DBI::Replicated::Replicant>), 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 <john.napiorkowski@takkle.com>
+
+=head1 LICENSE
+
+You may distribute this code under the same terms as Perl itself.
+
+=cut
+
+1;
\ No newline at end of file
index 6c66ea8..e37b291 100644 (file)
@@ -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
index 4f406c3..123cc47 100644 (file)
@@ -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
index 20739da..1156a24 100644 (file)
@@ -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