fixed roundrobin dead transport which should recover

This commit is contained in:
Fabien Potencier 2019-04-10 09:39:31 +02:00
parent ccbb171312
commit 5d4d4e7a71
3 changed files with 64 additions and 36 deletions

View File

@ -14,6 +14,7 @@ namespace Symfony\Component\Mailer\Tests\Transport;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Symfony\Component\Mailer\Exception\TransportException; use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Transport\FailoverTransport; use Symfony\Component\Mailer\Transport\FailoverTransport;
use Symfony\Component\Mailer\Transport\RoundRobinTransport;
use Symfony\Component\Mailer\Transport\TransportInterface; use Symfony\Component\Mailer\Transport\TransportInterface;
use Symfony\Component\Mime\RawMessage; use Symfony\Component\Mime\RawMessage;
@ -36,8 +37,11 @@ class FailoverTransportTest extends TestCase
$t2->expects($this->never())->method('send'); $t2->expects($this->never())->method('send');
$t = new FailoverTransport([$t1, $t2]); $t = new FailoverTransport([$t1, $t2]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
} }
public function testSendAllDead() public function testSendAllDead()
@ -50,6 +54,7 @@ class FailoverTransportTest extends TestCase
$this->expectException(TransportException::class); $this->expectException(TransportException::class);
$this->expectExceptionMessage('All transports failed.'); $this->expectExceptionMessage('All transports failed.');
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1, $t2]);
} }
public function testSendOneDead() public function testSendOneDead()
@ -60,27 +65,11 @@ class FailoverTransportTest extends TestCase
$t2->expects($this->exactly(3))->method('send'); $t2->expects($this->exactly(3))->method('send');
$t = new FailoverTransport([$t1, $t2]); $t = new FailoverTransport([$t1, $t2]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
} $this->assertTransports($t, 0, [$t1]);
public function testSendOneDeadAndRecoveryNotWithinRetryPeriod()
{
$t1 = $this->createMock(TransportInterface::class);
$t1->expects($this->at(0))->method('send')->will($this->throwException(new TransportException()));
$t1->expects($this->once())->method('send');
$t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->exactly(5))->method('send');
$t = new FailoverTransport([$t1, $t2], 40);
$t->send(new RawMessage(''));
sleep(4);
$t->send(new RawMessage(''));
sleep(4);
$t->send(new RawMessage(''));
sleep(4);
$t->send(new RawMessage(''));
sleep(4);
$t->send(new RawMessage(''));
} }
public function testSendOneDeadAndRecoveryWithinRetryPeriod() public function testSendOneDeadAndRecoveryWithinRetryPeriod()
@ -88,23 +77,26 @@ class FailoverTransportTest extends TestCase
$t1 = $this->createMock(TransportInterface::class); $t1 = $this->createMock(TransportInterface::class);
$t1->expects($this->at(0))->method('send')->will($this->throwException(new TransportException())); $t1->expects($this->at(0))->method('send')->will($this->throwException(new TransportException()));
$t1->expects($this->at(1))->method('send'); $t1->expects($this->at(1))->method('send');
$t1->expects($this->exactly(3))->method('send');
$t2 = $this->createMock(TransportInterface::class); $t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->at(0))->method('send'); $t2->expects($this->at(0))->method('send');
$t2->expects($this->at(1))->method('send'); $t2->expects($this->at(1))->method('send');
$t2->expects($this->at(2))->method('send'); $t2->expects($this->at(2))->method('send');
$t2->expects($this->at(3))->method('send')->will($this->throwException(new TransportException())); $t2->expects($this->at(3))->method('send')->will($this->throwException(new TransportException()));
$t2->expects($this->exactly(4))->method('send');
$t = new FailoverTransport([$t1, $t2], 6); $t = new FailoverTransport([$t1, $t2], 6);
$t->send(new RawMessage('')); // t1>fail - t2>sent $t->send(new RawMessage('')); // t1>fail - t2>sent
$this->assertTransports($t, 0, [$t1]);
sleep(4); sleep(4);
$t->send(new RawMessage('')); // t2>sent $t->send(new RawMessage('')); // t2>sent
$this->assertTransports($t, 0, [$t1]);
sleep(4); sleep(4);
$t->send(new RawMessage('')); // t2>sent $t->send(new RawMessage('')); // t2>sent
$this->assertTransports($t, 0, [$t1]);
sleep(4); sleep(4);
$t->send(new RawMessage('')); // t2>fail - t1>sent $t->send(new RawMessage('')); // t2>fail - t1>sent
$this->assertTransports($t, 1, [$t2]);
sleep(4); sleep(4);
$t->send(new RawMessage('')); // t1>sent $t->send(new RawMessage('')); // t1>sent
$this->assertTransports($t, 1, [$t2]);
} }
public function testSendAllDeadWithinRetryPeriod() public function testSendAllDeadWithinRetryPeriod()
@ -143,4 +135,15 @@ class FailoverTransportTest extends TestCase
sleep(1); sleep(1);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
} }
private function assertTransports(RoundRobinTransport $transport, int $cursor, array $deadTransports)
{
$p = new \ReflectionProperty(RoundRobinTransport::class, 'cursor');
$p->setAccessible(true);
$this->assertSame($cursor, $p->getValue($transport));
$p = new \ReflectionProperty(RoundRobinTransport::class, 'deadTransports');
$p->setAccessible(true);
$this->assertSame($deadTransports, iterator_to_array($p->getValue($transport)));
}
} }

View File

@ -36,8 +36,11 @@ class RoundRobinTransportTest extends TestCase
$t2->expects($this->once())->method('send'); $t2->expects($this->once())->method('send');
$t = new RoundRobinTransport([$t1, $t2]); $t = new RoundRobinTransport([$t1, $t2]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, []);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
} }
public function testSendAllDead() public function testSendAllDead()
@ -50,6 +53,7 @@ class RoundRobinTransportTest extends TestCase
$this->expectException(TransportException::class); $this->expectException(TransportException::class);
$this->expectExceptionMessage('All transports failed.'); $this->expectExceptionMessage('All transports failed.');
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, [$t1, $t2]);
} }
public function testSendOneDead() public function testSendOneDead()
@ -60,8 +64,11 @@ class RoundRobinTransportTest extends TestCase
$t2->expects($this->exactly(3))->method('send'); $t2->expects($this->exactly(3))->method('send');
$t = new RoundRobinTransport([$t1, $t2]); $t = new RoundRobinTransport([$t1, $t2]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 0, [$t1]);
} }
public function testSendOneDeadAndRecoveryNotWithinRetryPeriod() public function testSendOneDeadAndRecoveryNotWithinRetryPeriod()
@ -69,13 +76,16 @@ class RoundRobinTransportTest extends TestCase
$t1 = $this->createMock(TransportInterface::class); $t1 = $this->createMock(TransportInterface::class);
$t1->expects($this->exactly(4))->method('send'); $t1->expects($this->exactly(4))->method('send');
$t2 = $this->createMock(TransportInterface::class); $t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->at(0))->method('send')->will($this->throwException(new TransportException())); $t2->expects($this->once())->method('send')->will($this->throwException(new TransportException()));
$t2->expects($this->once())->method('send');
$t = new RoundRobinTransport([$t1, $t2], 60); $t = new RoundRobinTransport([$t1, $t2], 60);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, [$t2]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, [$t2]);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, [$t2]);
} }
public function testSendOneDeadAndRecoveryWithinRetryPeriod() public function testSendOneDeadAndRecoveryWithinRetryPeriod()
@ -85,14 +95,26 @@ class RoundRobinTransportTest extends TestCase
$t2 = $this->createMock(TransportInterface::class); $t2 = $this->createMock(TransportInterface::class);
$t2->expects($this->at(0))->method('send')->will($this->throwException(new TransportException())); $t2->expects($this->at(0))->method('send')->will($this->throwException(new TransportException()));
$t2->expects($this->at(1))->method('send'); $t2->expects($this->at(1))->method('send');
$t2->expects($this->exactly(2))->method('send');
$t = new RoundRobinTransport([$t1, $t2], 3); $t = new RoundRobinTransport([$t1, $t2], 3);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
$t->send(new RawMessage(''));
$this->assertTransports($t, 1, [$t2]);
sleep(5); sleep(5);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
sleep(5); $this->assertTransports($t, 0, []);
$t->send(new RawMessage(''));
sleep(5);
$t->send(new RawMessage('')); $t->send(new RawMessage(''));
$this->assertTransports($t, 1, []);
}
private function assertTransports(RoundRobinTransport $transport, int $cursor, array $deadTransports)
{
$p = new \ReflectionProperty($transport, 'cursor');
$p->setAccessible(true);
$this->assertSame($cursor, $p->getValue($transport));
$p = new \ReflectionProperty($transport, 'deadTransports');
$p->setAccessible(true);
$this->assertSame($deadTransports, iterator_to_array($p->getValue($transport)));
} }
} }

View File

@ -29,6 +29,7 @@ class RoundRobinTransport implements TransportInterface
private $deadTransports; private $deadTransports;
private $transports = []; private $transports = [];
private $retryPeriod; private $retryPeriod;
private $cursor = 0;
/** /**
* @param TransportInterface[] $transports * @param TransportInterface[] $transports
@ -62,7 +63,10 @@ class RoundRobinTransport implements TransportInterface
*/ */
protected function getNextTransport(): ?TransportInterface protected function getNextTransport(): ?TransportInterface
{ {
while ($transport = array_shift($this->transports)) { $cursor = $this->cursor;
while (true) {
$transport = $this->transports[$cursor];
if (!$this->isTransportDead($transport)) { if (!$this->isTransportDead($transport)) {
break; break;
} }
@ -73,18 +77,12 @@ class RoundRobinTransport implements TransportInterface
break; break;
} }
if ($transport) { if ($this->cursor === $cursor = $this->moveCursor($cursor)) {
$this->transports[] = $transport;
}
if ($this->deadTransports->count() >= \count($this->transports)) {
return null; return null;
} }
} }
if ($transport) { $this->cursor = $this->moveCursor($cursor);
$this->transports[] = $transport;
}
return $transport; return $transport;
} }
@ -93,4 +91,9 @@ class RoundRobinTransport implements TransportInterface
{ {
return $this->deadTransports->contains($transport); return $this->deadTransports->contains($transport);
} }
private function moveCursor(int $cursor): int
{
return ++$cursor >= \count($this->transports) ? 0 : $cursor;
}
} }