Skip to content

Commit

Permalink
fix multiple bugs in Minion::Backend::Pg related to the minion_locks …
Browse files Browse the repository at this point in the history 
…table only deleting expired locks when a new lock is requested
  • Loading branch information
@kraih
kraih committed Dec 10, 2017
1 parent 310e5e1 commit 1c5fb5f
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 60 deletions.
4 changes: 4 additions & 0 deletions Changes
View file
@@ -1,5 +1,9 @@

8.05 2017-12-10
- Removed active_locks field from stats methods again since it did not work
correctly and there is no efficient way to fix it.
- Fixed list_locks method in Minion::Backend::Pg to exclude already expired
locks.

8.04 2017-12-08
- Added list_locks method to Minion::Backend and Minion::Backend::Pg.
Expand Down
6 changes: 0 additions & 6 deletions lib/Minion.pm
View file
Expand Up @@ -633,12 +633,6 @@ These fields are currently available:
Number of jobs in C<active> state.
=item active_locks
active_locks => 100
Number of active named locks.
=item active_workers
active_workers => 100
Expand Down
6 changes: 0 additions & 6 deletions lib/Minion/Backend.pm
View file
Expand Up @@ -619,12 +619,6 @@ These fields are currently available:
Number of jobs in C<active> state.
=item active_locks
active_locks => 100
Number of active named locks.
=item active_workers
active_workers => 100
Expand Down
11 changes: 2 additions & 9 deletions lib/Minion/Backend/Pg.pm
View file
Expand Up @@ -77,8 +77,8 @@ sub list_locks {
my $locks = $self->pg->db->query(
'select name, extract(epoch from expires) as expires,
count(*) over() as total from minion_locks
where (name = $1 or $1 is null) order by id desc limit $2 offset $3',
$options->{name}, $limit, $offset
where expires > now() and (name = $1 or $1 is null)
order by id desc limit $2 offset $3', $options->{name}, $limit, $offset
)->hashes->to_array;
return _total('locks', $locks);
}
Expand Down Expand Up @@ -215,7 +215,6 @@ sub stats {
count(*) filter (where state = 'finished') as finished_jobs,
count(*) filter (where state = 'inactive'
and delayed > now()) as delayed_jobs,
(select count(*) from minion_locks) as active_locks,
count(distinct worker) filter (where state = 'active') as active_workers,
(select case when is_called then last_value else 0 end
from minion_jobs_id_seq) as enqueued_jobs,
Expand Down Expand Up @@ -855,12 +854,6 @@ These fields are currently available:
Number of jobs in C<active> state.
=item active_locks
active_locks => 100
Number of active named locks.
=item active_workers
active_workers => 100
Expand Down
40 changes: 5 additions & 35 deletions lib/Mojolicious/Plugin/Minion/resources/templates/minion/jobs.html.ep
View file
Expand Up @@ -37,37 +37,7 @@
% }
% end

<div class="row">
<div class="col-md-12">
% if (my $info = flash 'info') {
<div class="alert alert-dismissible alert-info" role="alert">
<button type="button" class="close" data-dismiss="alert"
aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
<%= $info %>
</div>
% }
% if (my $success = flash 'success') {
<div class="alert alert-dismissible alert-success" role="alert">
<button type="button" class="close" data-dismiss="alert"
aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
<%= $success %>
</div>
% }
% if (my $danger = flash 'danger') {
<div class="alert alert-dismissible alert-danger" role="alert">
<button type="button" class="close" data-dismiss="alert"
aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
<%= $danger %>
</div>
% }
</div>
</div>
%= include 'minion/_notifications'

%= form_for 'minion_manage_jobs' => begin
<div class="row center-md">
Expand Down Expand Up @@ -150,10 +120,10 @@
<span class="label label-info">inactive</span>
% }
</td>
<td class="accordion-toggle collapsed expand" data-toggle="collapse"
data-target="#job<%= $i %>">
<span class="fas fa-lg expand-icon"></span>
</td>
<td class="accordion-toggle collapsed expand"
data-toggle="collapse" data-target="#job<%= $i %>">
<span class="fas fa-lg expand-icon"></span>
</td>
</tr>
<tr>
<td colspan="8" class="hiddenRow">
Expand Down
11 changes: 7 additions & 4 deletions t/pg.t
View file
Expand Up @@ -168,7 +168,6 @@ ok $minion->unlock('baz'), 'unlocked';
ok !$minion->unlock('baz'), 'not unlocked again';

# List locks
is $minion->stats->{active_locks}, 1, 'one active lock';
$results = $minion->backend->list_locks(0, 2);
is $results->{locks}[0]{name}, 'yada', 'right name';
like $results->{locks}[0]{expires}, qr/^[\d.]+$/, 'expires';
Expand All @@ -178,7 +177,6 @@ $minion->unlock('yada');
$minion->lock('yada', 3600, {limit => 2});
$minion->lock('test', 3600, {limit => 1});
$minion->lock('yada', 3600, {limit => 2});
is $minion->stats->{active_locks}, 3, 'three active locks';
$results = $minion->backend->list_locks(1, 1);
is $results->{locks}[0]{name}, 'test', 'right name';
like $results->{locks}[0]{expires}, qr/^[\d.]+$/, 'expires';
Expand All @@ -191,7 +189,13 @@ is $results->{locks}[1]{name}, 'yada', 'right name';
like $results->{locks}[1]{expires}, qr/^[\d.]+$/, 'expires';
is $results->{locks}[2], undef, 'no more locks';
is $results->{total}, 2, 'two results';
$minion->unlock($_) for qw(yada yada test);
$minion->backend->pg->db->query(
"update minion_locks set expires = now() - interval '1 second' * 1
where name = 'yada'",
);
is $minion->backend->list_locks(0, 10, {name => 'yada'})->{total}, 0,
'no results';
$minion->unlock('test');
is $minion->backend->list_locks(0, 10)->{total}, 0, 'no results';

# Lock with guard
Expand Down Expand Up @@ -230,7 +234,6 @@ is $stats->{failed_jobs}, 0, 'no failed jobs';
is $stats->{finished_jobs}, 0, 'no finished jobs';
is $stats->{inactive_jobs}, 0, 'no inactive jobs';
is $stats->{delayed_jobs}, 0, 'no delayed jobs';
is $stats->{active_locks}, 0, 'no active locks';
ok $stats->{uptime}, 'has uptime';
$worker = $minion->worker->register;
is $minion->stats->{inactive_workers}, 1, 'one inactive worker';
Expand Down

0 comments on commit 1c5fb5f

Please sign in to comment.