From 4550b8a84cdf25ec54e468061db3bfba19e8a676 Mon Sep 17 00:00:00 2001 From: Oliver Gorwits Date: Tue, 23 May 2017 17:27:43 +0100 Subject: [PATCH] skipover now implicit from deferrals/actionset; fix sql where logic with better correlation --- lib/App/Netdisco/DB/Result/Admin.pm | 20 ++++++++++++----- lib/App/Netdisco/DB/Result/DeviceSkip.pm | 8 +++---- lib/App/Netdisco/DB/ResultSet/Admin.pm | 22 +++++++++++++++++++ lib/App/Netdisco/JobQueue/PostgreSQL.pm | 6 +---- .../App-Netdisco-DB-41-42-PostgreSQL.sql | 1 - 5 files changed, 40 insertions(+), 17 deletions(-) diff --git a/lib/App/Netdisco/DB/Result/Admin.pm b/lib/App/Netdisco/DB/Result/Admin.pm index 3a5c7bc2..85f75dec 100644 --- a/lib/App/Netdisco/DB/Result/Admin.pm +++ b/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 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 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 which completes this +query with a C host and C 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 } diff --git a/lib/App/Netdisco/DB/Result/DeviceSkip.pm b/lib/App/Netdisco/DB/Result/DeviceSkip.pm index 76621aa8..bed6f17b 100644 --- a/lib/App/Netdisco/DB/Result/DeviceSkip.pm +++ b/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 )) ] }); } diff --git a/lib/App/Netdisco/DB/ResultSet/Admin.pm b/lib/App/Netdisco/DB/ResultSet/Admin.pm index 1d7d8bec..9b1ec4b7 100644 --- a/lib/App/Netdisco/DB/ResultSet/Admin.pm +++ b/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 entries that apply +to some jobs. They match the device IP, current backend, and job action. + +Pass the C FQDN (or the current host will be used as a default), and +the C (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 (including the helpers below) which diff --git a/lib/App/Netdisco/JobQueue/PostgreSQL.pm b/lib/App/Netdisco/JobQueue/PostgreSQL.pm index c143d52e..1429ce7a 100644 --- a/lib/App/Netdisco/JobQueue/PostgreSQL.pm +++ b/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 ]); }); diff --git a/share/schema_versions/App-Netdisco-DB-41-42-PostgreSQL.sql b/share/schema_versions/App-Netdisco-DB-41-42-PostgreSQL.sql index 6314e962..0b64289b 100644 --- a/share/schema_versions/App-Netdisco-DB-41-42-PostgreSQL.sql +++ b/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") );