Skip to content

Commit

Permalink
Ability to ignore long waits for queue (#1172)
Browse files Browse the repository at this point in the history
* Boyscouting

* WIP: TDD

* Code style

* Remove unused event

* Ignore queues with wait time of 0
  • Loading branch information
jasonmccreary committed Aug 11, 2022
1 parent 4f67df3 commit 8cbdf15
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 13 deletions.
6 changes: 3 additions & 3 deletions src/Listeners/MonitorWaitTimes.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use Carbon\CarbonImmutable;
use Laravel\Horizon\Contracts\MetricsRepository;
use Laravel\Horizon\Events\LongWaitDetected;
use Laravel\Horizon\Events\SupervisorLooped;
use Laravel\Horizon\WaitTimeCalculator;

class MonitorWaitTimes
Expand Down Expand Up @@ -41,7 +40,7 @@ public function __construct(MetricsRepository $metrics)
* @param \Laravel\Horizon\Events\SupervisorLooped $event
* @return void
*/
public function handle(SupervisorLooped $event)
public function handle()
{
if (! $this->dueToMonitor()) {
return;
Expand All @@ -53,7 +52,8 @@ public function handle(SupervisorLooped $event)
$results = app(WaitTimeCalculator::class)->calculate();

$long = collect($results)->filter(function ($wait, $queue) {
return $wait > (config("horizon.waits.{$queue}") ?? 60);
return config("horizon.waits.{$queue}") !== 0
&& $wait > (config("horizon.waits.{$queue}") ?? 60);
});

// Once we have determined which queues have long wait times we will raise the
Expand Down
33 changes: 23 additions & 10 deletions tests/Feature/MonitorWaitTimesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
namespace Laravel\Horizon\Tests\Feature;

use Carbon\CarbonImmutable;
use Illuminate\Contracts\Redis\Factory as RedisFactory;
use Illuminate\Support\Facades\Event;
use Laravel\Horizon\Contracts\MetricsRepository;
use Laravel\Horizon\Events\LongWaitDetected;
use Laravel\Horizon\Events\SupervisorLooped;
use Laravel\Horizon\Listeners\MonitorWaitTimes;
use Laravel\Horizon\Supervisor;
use Laravel\Horizon\Tests\IntegrationTest;
use Laravel\Horizon\WaitTimeCalculator;
use Mockery;
Expand All @@ -20,24 +17,40 @@ public function test_queues_with_long_waits_are_found()
{
Event::fake();

$redis = Mockery::mock(RedisFactory::class);
$redis->shouldReceive('setnx')->with('monitor:time-to-clear', 1)->andReturn(1);
$redis->shouldReceive('expire')->with('monitor:time-to-clear', 60);

$calc = Mockery::mock(WaitTimeCalculator::class);
$calc->shouldReceive('calculate')->andReturn([
'redis:test-queue' => 10,
'redis:test-queue-2' => 80,
]);
$this->app->instance(WaitTimeCalculator::class, $calc);

$listener = new MonitorWaitTimes(resolve(MetricsRepository::class), $redis);
$listener->lastMonitoredAt = CarbonImmutable::now()->subDays(1);
$listener = new MonitorWaitTimes(resolve(MetricsRepository::class));
$listener->lastMonitoredAt = CarbonImmutable::now()->subDay();

$listener->handle(new SupervisorLooped(Mockery::mock(Supervisor::class)));
$listener->handle();

Event::assertDispatched(LongWaitDetected::class, function ($event) {
return $event->connection == 'redis' && $event->queue == 'test-queue-2';
});
}

public function test_queue_ignores_long_waits()
{
config(['horizon.waits' => ['redis:ignore-queue' => 0]]);

Event::fake();

$calc = Mockery::mock(WaitTimeCalculator::class);
$calc->expects('calculate')->andReturn([
'redis:ignore-queue' => 10,
]);
$this->app->instance(WaitTimeCalculator::class, $calc);

$listener = new MonitorWaitTimes(resolve(MetricsRepository::class));
$listener->lastMonitoredAt = CarbonImmutable::now()->subDays(1);

$listener->handle();

Event::assertNotDispatched(LongWaitDetected::class);
}
}

0 comments on commit 8cbdf15

Please sign in to comment.