use row lock not table lock

Netdisco/lib/App/Netdisco/Core/Arpnip.pm

@@ -3,7 +3,6 @@ package App::Netdisco::Core::Arpnip;
 use Dancer qw/:syntax :script/;
 use Dancer::Plugin::DBIC 'schema';
 
-use App::Netdisco::DB::ExplicitLocking ':modes';
 use App::Netdisco::Util::PortMAC ':all';
 use App::Netdisco::Util::DNS ':all';
 use NetAddr::IP::Lite ':lower';
@@ -21,7 +20,7 @@ App::Netdisco::Core::Arpnip
 
 =head1 DESCRIPTION
 
-Helper subroutine to support parts of the Netdisco application.
+Helper subroutines to support parts of the Netdisco application.
 
 There are no default exports, however the C<:all> tag will export all
 subroutines.
@@ -60,12 +59,9 @@ sub do_arpnip {
   # would be possible just to use now() on updated records, but by using this
   # same value for them all, we _can_ if we want add a job at the end to
   # select and do something with the updated set (no reason to yet, though)
-  my $now = join '.', gettimeofday;
+  my $now = 'to_timestamp('. (join '.', gettimeofday) .')';
 
   # update node_ip with ARP and Neighbor Cache entries
-  # TODO: ORDER BY ... FOR UPDATE will allow us to avoid the table lock
-  schema('netdisco')->resultset('NodeIp')->txn_do_locked(
-    EXCLUSIVE, sub {
   _store_arp(@$_, $now) for @v4;
   debug sprintf ' [%s] arpnip - processed %s ARP Cache entries',
     $device->ip, scalar @v4;
@@ -74,19 +70,18 @@ sub do_arpnip {
   debug sprintf ' [%s] arpnip - processed %s IPv6 Neighbor Cache entries',
     $device->ip, scalar @v6;
 
-      $device->update({last_arpnip => \"to_timestamp($now)"});
-    });
-
   # update subnets with new networks
   foreach my $cidr (@subnets) {
       schema('netdisco')->txn_do(sub {
           schema('netdisco')->resultset('Subnet')->update_or_create(
           {
             net => $cidr,
-            last_discover => \"to_timestamp($now)",
+            last_discover => \$now,
           },
-          # update_or_create doesn't seem to lock the row
+          {
-          { for => 'update' });
+            order_by => 'net',
+            for => 'update',
+          });
       });
   }
   debug sprintf ' [%s] arpnip - processed %s Subnet entries',
@@ -159,18 +154,21 @@ sub _check_arp {
 sub _store_arp {
   my ($mac, $ip, $name, $now) = @_;
 
-  schema('netdisco')->resultset('NodeIp')
+  schema('netdisco')->txn_do(sub {
+    my $current = schema('netdisco')->resultset('NodeIp')
       ->search({ip => $ip, -bool => 'active'})
-    ->update({active => \'false'});
+      ->search(undef, {order_by => [qw/mac ip/], for => 'update'});
+    my $count = scalar $current->all;
+    $current->update({active => \'false'});
 
     schema('netdisco')->resultset('NodeIp')
-    ->search({mac => $mac, ip => $ip})
+      ->search({'me.mac' => $mac, 'me.ip' => $ip})
+      ->search(undef, {order_by => [qw/mac ip/], for => 'update'})
       ->update_or_create({
-      mac => $mac,
-      ip => $ip,
         dns => $name,
         active => \'true',
-      time_last => \"to_timestamp($now)",
+        time_last => \$now,
+      });
   });
 }
 

Netdisco/lib/App/Netdisco/Core/Discover.pm

@@ -106,7 +106,7 @@ sub store_device {
     my $gone = $device->device_ips->delete;
     debug sprintf ' [%s] device - removed %s aliases',
       $device->ip, $gone;
-    $device->update_or_insert({}, {for => 'update'});
+    $device->update_or_insert(undef, {for => 'update'});
     $device->device_ips->populate(\@aliases);
     debug sprintf ' [%s] device - added %d new aliases',
       $device->ip, scalar @aliases;
@@ -253,7 +253,7 @@ sub store_interfaces {
     my $gone = $device->ports->delete;
     debug sprintf ' [%s] interfaces - removed %s interfaces',
       $device->ip, $gone;
-    $device->update_or_insert({}, {for => 'update'});
+    $device->update_or_insert(undef, {for => 'update'});
     $device->ports->populate(\@interfaces);
     debug sprintf ' [%s] interfaces - added %d new interfaces',
       $device->ip, scalar @interfaces;
@@ -744,7 +744,7 @@ sub _set_manual_topology {
               return unless ($left->in_storage and $right->in_storage);
 
               $left->ports
-                ->single({port => $link->port1})
+                ->single({port => $link->port1}, {for => 'update'})
                 ->update({
                   remote_ip => $right->ip,
                   remote_port => $link->port2,
@@ -752,7 +752,7 @@ sub _set_manual_topology {
                 });
 
               $right->ports
-                ->single({port => $link->port2})
+                ->single({port => $link->port2}, {for => 'update'})
                 ->update({
                   remote_ip => $left->ip,
                   remote_port => $link->port1,

Netdisco/lib/App/Netdisco/Core/Macsuck.pm

@@ -3,7 +3,6 @@ package App::Netdisco::Core::Macsuck;
 use Dancer qw/:syntax :script/;
 use Dancer::Plugin::DBIC 'schema';
 
-use App::Netdisco::DB::ExplicitLocking ':modes';
 use App::Netdisco::Util::PortMAC ':all';
 use Time::HiRes 'gettimeofday';
 
@@ -18,7 +17,7 @@ App::Netdisco::Core::Macsuck
 
 =head1 DESCRIPTION
 
-Helper subroutine to support parts of the Netdisco application.
+Helper subroutines to support parts of the Netdisco application.
 
 There are no default exports, however the C<:all> tag will export all
 subroutines.
@@ -52,7 +51,7 @@ sub do_macsuck {
   # would be possible just to use now() on updated records, but by using this
   # same value for them all, we _can_ if we want add a job at the end to
   # select and do something with the updated set (no reason to yet, though)
-  my $now = join '.', gettimeofday;
+  my $now = 'to_timestamp('. (join '.', gettimeofday) .')';
   my $total_nodes = 0;
 
   # do this before we start messing with the snmp community string
@@ -109,7 +108,7 @@ sub do_macsuck {
 
   debug sprintf ' [%s] macsuck - %s forwarding table entries',
     $device->ip, $total_nodes;
-  $device->update({last_macsuck => \"to_timestamp($now)"});
+  $device->update({last_macsuck => \$now});
 }
 
 sub _wireless_client_info {
@@ -160,7 +159,7 @@ sub _wireless_client_info {
             sigqual => $cd11_sigqual->{$idx},
             sigstrength => $cd11_sigstrength->{$idx},
             ssid    => ($cd11_ssid->{$idx} || 'unknown'),
-            time_last => \"to_timestamp($now)",
+            time_last => \$now,
           }, { for => 'update' });
       });
   }
@@ -217,8 +216,8 @@ sub _store_node {
       vlan => $vlan,
       active => \'true',
       oui => substr($mac,0,8),
-      time_last => \"to_timestamp($now)",
+      time_last => \$now,
-      ($old_count ? (time_recent => \"to_timestamp($now)") : ()),
+      ($old_count ? (time_recent => \$now) : ()),
     });
   });
 }