Merge pull request #247 from clue-labs/round-robin-dns

Use round robin for happy eyeballs DNS responses (load balancing)
reactphp · Sep 6, 2020 · 28fac70 · 28fac70
2 parents e2b96b2 + 16ff9ad
commit 28fac70
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 50 deletions.
diff --git a/src/HappyEyeBallsConnectionBuilder.php b/src/HappyEyeBallsConnectionBuilder.php
@@ -316,6 +316,7 @@ public function hasBeenResolved()
      */
     public function mixIpsIntoConnectQueue(array $ips)
     {
+        \shuffle($ips);
         $this->ipsCount += \count($ips);
         $connectQueueStash = $this->connectQueue;
         $this->connectQueue = array();

diff --git a/tests/HappyEyeBallsConnectionBuilderTest.php b/tests/HappyEyeBallsConnectionBuilderTest.php
@@ -302,8 +302,8 @@ public function testConnectWillStartConnectingWithAlternatingIPv6AndIPv4WhenReso
         $connector->expects($this->exactly(4))->method('connect')->withConsecutive(
             array('tcp://[::1]:80?hostname=reactphp.org'),
             array('tcp://127.0.0.1:80?hostname=reactphp.org'),
-            array('tcp://[::2]:80?hostname=reactphp.org'),
-            array('tcp://127.0.0.2:80?hostname=reactphp.org')
+            array('tcp://[::1]:80?hostname=reactphp.org'),
+            array('tcp://127.0.0.1:80?hostname=reactphp.org')
         )->willReturnOnConsecutiveCalls(
             $deferred->promise(),
             $deferred->promise(),
@@ -316,8 +316,8 @@ public function testConnectWillStartConnectingWithAlternatingIPv6AndIPv4WhenReso
             array('reactphp.org', Message::TYPE_AAAA),
             array('reactphp.org', Message::TYPE_A)
         )->willReturnOnConsecutiveCalls(
-            \React\Promise\resolve(array('::1', '::2')),
-            \React\Promise\resolve(array('127.0.0.1', '127.0.0.2'))
+            \React\Promise\resolve(array('::1', '::1')),
+            \React\Promise\resolve(array('127.0.0.1', '127.0.0.1'))
         );
 
         $uri = 'tcp://reactphp.org:80';
@@ -341,7 +341,7 @@ public function testConnectWillStartConnectingWithAttemptTimerWhenOnlyIpv6Resolv
         $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
         $connector->expects($this->exactly(2))->method('connect')->withConsecutive(
             array('tcp://[::1]:80?hostname=reactphp.org'),
-            array('tcp://[::2]:80?hostname=reactphp.org')
+            array('tcp://[::1]:80?hostname=reactphp.org')
         )->willReturnOnConsecutiveCalls(
             \React\Promise\reject(new \RuntimeException()),
             new Promise(function () { })
@@ -352,7 +352,7 @@ public function testConnectWillStartConnectingWithAttemptTimerWhenOnlyIpv6Resolv
             array('reactphp.org', Message::TYPE_AAAA),
             array('reactphp.org', Message::TYPE_A)
         )->willReturnOnConsecutiveCalls(
-            \React\Promise\resolve(array('::1', '::2')),
+            \React\Promise\resolve(array('::1', '::1')),
             \React\Promise\reject(new \RuntimeException())
         );
 
@@ -799,4 +799,56 @@ public function testCleanUpCancelsAllPendingConnectionAttemptsWithoutStartingNew
 
         $builder->cleanUp();
     }
+
+    public function testMixIpsIntoConnectQueueSometimesAssignsInOriginalOrder()
+    {
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
+        $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
+
+        $uri = 'tcp://reactphp.org:80/path?test=yes#start';
+        $host = 'reactphp.org';
+        $parts = parse_url($uri);
+
+        for ($i = 0; $i < 100; ++$i) {
+            $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
+            $builder->mixIpsIntoConnectQueue(array('::1', '::2'));
+
+            $ref = new \ReflectionProperty($builder, 'connectQueue');
+            $ref->setAccessible(true);
+            $value = $ref->getValue($builder);
+
+            if ($value === array('::1', '::2')) {
+                break;
+            }
+        }
+
+        $this->assertEquals(array('::1', '::2'), $value);
+    }
+
+    public function testMixIpsIntoConnectQueueSometimesAssignsInReverseOrder()
+    {
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
+        $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
+
+        $uri = 'tcp://reactphp.org:80/path?test=yes#start';
+        $host = 'reactphp.org';
+        $parts = parse_url($uri);
+
+        for ($i = 0; $i < 100; ++$i) {
+            $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
+            $builder->mixIpsIntoConnectQueue(array('::1', '::2'));
+
+            $ref = new \ReflectionProperty($builder, 'connectQueue');
+            $ref->setAccessible(true);
+            $value = $ref->getValue($builder);
+
+            if ($value === array('::2', '::1')) {
+                break;
+            }
+        }
+
+        $this->assertEquals(array('::2', '::1'), $value);
+    }
 }
diff --git a/tests/HappyEyeBallsConnectorTest.php b/tests/HappyEyeBallsConnectorTest.php
@@ -270,50 +270,6 @@ public function testCancelDuringTcpConnectionCancelsTcpConnectionIfGivenIp()
         $this->loop->run();
     }
 
-    /**
-     * @dataProvider provideIpvAddresses
-     */
-    public function testShouldConnectOverIpv4WhenIpv6LookupFails(array $ipv6, array $ipv4)
-    {
-        $this->resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive(
-            array($this->equalTo('example.com'), Message::TYPE_AAAA),
-            array($this->equalTo('example.com'), Message::TYPE_A)
-        )->willReturnOnConsecutiveCalls(
-            Promise\reject(new \Exception('failure')),
-            Promise\resolve($ipv4)
-        );
-        $this->tcp->expects($this->exactly(1))->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn(Promise\resolve($this->connection));
-
-        $promise = $this->connector->connect('example.com:80');;
-        $resolvedConnection = Block\await($promise, $this->loop);
-
-        self::assertSame($this->connection, $resolvedConnection);
-    }
-
-    /**
-     * @dataProvider provideIpvAddresses
-     */
-    public function testShouldConnectOverIpv6WhenIpv4LookupFails(array $ipv6, array $ipv4)
-    {
-        if (count($ipv6) === 0) {
-            $ipv6[] = '1:2:3:4';
-        }
-
-        $this->resolver->expects($this->exactly(2))->method('resolveAll')->withConsecutive(
-            array($this->equalTo('example.com'), Message::TYPE_AAAA),
-            array($this->equalTo('example.com'), Message::TYPE_A)
-        )->willReturnOnConsecutiveCalls(
-            Promise\resolve($ipv6),
-            Promise\reject(new \Exception('failure'))
-        );
-        $this->tcp->expects($this->exactly(1))->method('connect')->with($this->equalTo('[1:2:3:4]:80?hostname=example.com'))->willReturn(Promise\resolve($this->connection));
-
-        $promise = $this->connector->connect('example.com:80');;
-        $resolvedConnection = Block\await($promise, $this->loop);
-
-        self::assertSame($this->connection, $resolvedConnection);
-    }
-
     /**
      * @internal
      */