skipover now implicit from deferrals/actionset; fix sql where logic with better correlation

lib/App/Netdisco/DB/Result/Admin.pm

@@ -58,21 +58,29 @@ __PACKAGE__->set_primary_key("job");
 
 =head1 RELATIONSHIPS
 
-=head2 skipped
+=head2 device_skips( $backend?, $max_deferrals? )
 
-Retuns the set of C<device_skip> entries which may apply to this job. They
-match the device IP and job action, and may refer to one or more backends.
+Retuns the set of C<device_skip> entries which apply to this job. They match
+the device IP, current backend, and job action.
+
+You probably want to use the ResultSet method C<skipped> which completes this
+query with a C<backend> host and C<max_deferrals> parameters (or sensible
+defaults).
 
 =cut
 
-__PACKAGE__->has_many( skipped => 'App::Netdisco::DB::Result::DeviceSkip',
+__PACKAGE__->has_many( device_skips => 'App::Netdisco::DB::Result::DeviceSkip',
   sub {
     my $args = shift;
     return {
+      "$args->{foreign_alias}.backend" => { '=' => \'?' },
       "$args->{foreign_alias}.device"
         => { -ident => "$args->{self_alias}.device" },
-      "$args->{foreign_alias}.actionset"
-        => { '@>' => \"string_to_array($args->{self_alias}.action,'')" },
+      -or => [
+        { "$args->{foreign_alias}.actionset"
+            => { '@>' => \"string_to_array($args->{self_alias}.action,'')" } },
+        { "$args->{foreign_alias}.deferrals" => { '>=' => \'?' } },
+      ],
     };
   },
   { cascade_copy => 0, cascade_update => 0, cascade_delete => 0 }

lib/App/Netdisco/DB/Result/DeviceSkip.pm

@@ -14,11 +14,9 @@ __PACKAGE__->add_columns(
   "device",
   { data_type => "inet", is_nullable => 0 },
   "actionset",
-  { data_type => "text[]", is_nullable => 0 },
+  { data_type => "text[]", is_nullable => 0, default_value => '{}' },
   "deferrals",
   { data_type => "integer", is_nullable => 1, default_value => '0' },
-  "skipover",
-  { data_type => "boolean", is_nullable => 1, default_value => \'false' },
 );
 
 __PACKAGE__->set_primary_key("backend", "device");
@@ -38,7 +36,7 @@ There is a race in the update, but this is not worrying for now.
 sub increment_deferrals {
   my $row = shift;
   return unless $row->in_storage;
-  return $row->update({ deferrals => ($row->deferrals + 1) });
+  return $row->update({ deferrals => (($row->deferrals || 0) + 1) });
 }
 
 =head2 add_to_actionset
@@ -49,7 +47,7 @@ sub add_to_actionset {
   my ($row, @badactions) = @_;
   return unless $row->in_storage;
   return unless scalar @badactions;
-  return $row->update({ skipover => \'true', actionset =>
+  return $row->update({ actionset =>
     [ sort (List::MoreUtils::uniq( @{ $row->actionset || [] }, @badactions )) ]
   });
 }

lib/App/Netdisco/DB/ResultSet/Admin.pm

@@ -4,12 +4,34 @@ use base 'App::Netdisco::DB::ResultSet';
 use strict;
 use warnings;
 
+use Net::Domain 'hostfqdn';
+
 __PACKAGE__->load_components(qw/
   +App::Netdisco::DB::ExplicitLocking
 /);
 
 =head1 ADDITIONAL METHODS
 
+=head2 skipped
+
+Retuns a correlated subquery for the set of C<device_skip> entries that apply
+to some jobs. They match the device IP, current backend, and job action.
+
+Pass the C<backend> FQDN (or the current host will be used as a default), and
+the C<max_deferrals> (or 10 will be used as the default).
+
+=cut
+
+sub skipped {
+  my ($rs, $backend, $max_deferrals) = @_;
+  $backend ||= (hostfqdn || 'localhost');
+  $max_deferrals ||= 10;
+
+  return $rs->correlate('device_skips')->search(undef, {
+    bind => [[deferrals => $max_deferrals], [backend => $backend]],
+  });
+}
+
 =head2 with_times
 
 This is a modifier for any C<search()> (including the helpers below) which

lib/App/Netdisco/JobQueue/PostgreSQL.pm

@@ -40,10 +40,7 @@ sub _getsome {
 
   my $rs = $jobs->search({
     status => 'queued',
-    device => { '-not_in' => $jobs->correlate('skipped')->search({
-      backend => $fqdn,
-      -or => [{ deferrals => { '>=', 10 } },{ '-bool' => 'skipover' }],
-    }, { columns => 'device' })->as_query },
+    device => { '-not_in' => $jobs->skipped->columns('device')->as_query },
     %$where,
   }, { order_by => 'random()', rows => $num_slots });
 
@@ -130,7 +127,6 @@ sub jq_prime_skiplist {
         backend => $fqdn,
         device  => $_,
         actionset => $actionset{$_},
-        skipover => \'true',
       }} keys %actionset
     ]);
   });

share/schema_versions/App-Netdisco-DB-41-42-PostgreSQL.sql

@@ -5,7 +5,6 @@ CREATE TABLE "device_skip" (
   "device" inet NOT NULL,
   "actionset" text[] DEFAULT '{}',
   "deferrals" integer DEFAULT 0,
-  "skipover" boolean DEFAULT false,
   PRIMARY KEY ("backend", "device")
 );