Merge pull request #2 from utopia-php/feat-optimize-filling

Optimize Filling Logic
utopia-php · Nov 2, 2022 · 3c41ab2 · 3c41ab2
2 parents 5a467a5 + f2caf0b
commit 3c41ab2
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 136 deletions.
diff --git a/README.md b/README.md
@@ -46,8 +46,6 @@ $pool = new Pool('mysql-pool', 1 /* number of connections */, function() {
 $pool->setReconnectAttempts(3); // number of attempts to reconnect
 $pool->setReconnectSleep(5); // seconds to sleep between reconnect attempts
 
-$pool->fill(); // Populate the pool with connections
-
 $connection = $pool->pop(); // Get a connection from the pool
 $connection->getID(); // Get the connection ID
 $connection->getResource(); // Get the connection resource

diff --git a/composer.json b/composer.json
@@ -21,7 +21,8 @@
         "php": ">=8.0",
         "ext-pdo": "*",
         "ext-redis": "*",
-        "ext-mongodb": "*"
+        "ext-mongodb": "*",
+        "utopia-php/cli": "^0.11.0"
     },
     "require-dev": {
         "phpunit/phpunit": "^9.4",

diff --git a/src/Pools/Connection.php b/src/Pools/Connection.php
@@ -9,6 +9,11 @@ class Connection {
      */
     protected string $id = '';
 
+    /**
+     * @var Pool
+     */
+    protected ?Pool $pool = null;
+
     /**
      * @var mixed $resource
      */
@@ -51,4 +56,30 @@ public function setResource(mixed $resource): self
         $this->resource = $resource;
         return $this;
     }
+
+    /**
+     * @return Pool
+     */
+    public function getPool(): ?Pool
+    {
+        return $this->pool;
+    }
+
+    /**
+     * @param Pool $pool
+     * @return self
+     */
+    public function setPool(Pool &$pool): self
+    {
+        $this->pool = $pool;
+        return $this;
+    }
+
+    /**
+     * @return Pool
+     */
+    public function reclaim(): Pool
+    {
+        return $this->pool->reclaim($this);
+    }
 }
diff --git a/src/Pools/Group.php b/src/Pools/Group.php
@@ -27,7 +27,7 @@ public function add(Pool $pool): self
      */
     public function get(string $name): Pool
     {
-        return $this->pools[$name] ??  throw new Exception('Pool not found');
+        return $this->pools[$name] ??  throw new Exception("Pool '{$name}'  not found");
     }
 
     /**
@@ -39,19 +39,7 @@ public function remove(string $name): self
         unset($this->pools[$name]);
         return $this;
     }
-
-    /**
-     * @return self
-     */
-    public function fill(): self
-    {
-        foreach ($this->pools as $pool) {
-            $pool->fill();
-        }
-
-        return $this;
-    }
-
+
     /**
      * @return self
      */

diff --git a/src/Pools/Pool.php b/src/Pools/Pool.php
@@ -51,6 +51,7 @@ public function __construct(string $name, int $size, callable $init)
         $this->name = $name;
         $this->size = $size;
         $this->init = $init;
+        $this->pool = array_fill(0, $size, true);
     }
 
     /**
@@ -106,13 +107,29 @@ public function setReconnectSleep(int $reconnectSleep): self
     }
 
     /**
-     * @return self
+     * Summary: 
+     *  1. Try to get a connection from the pool
+     *  2. If no connection is available, wait for one to be released
+     *  3. If still no connection is available, throw an exception
+     *  4. If a connection is available, return it
+     * 
+     * @return Connection
      */
-    public function fill(): self
+    public function pop(): Connection
     {
-        $this->pool = [];
+        $connection = array_pop($this->pool);
+
+        if(is_null($connection)) { // pool is empty, wait an if still empty throw exception
+            usleep(50000); // 50ms TODO: make this configurable
 
-        for ($i=0; $i < $this->size; $i++) { 
+            $connection = array_pop($this->pool);
+
+            if(is_null($connection)) {
+                throw new Exception('Pool is empty');
+            }
+        }
+
+        if($connection === true) { // Pool has space, create connection
             $attempts = 0;
 
             do {
@@ -128,27 +145,18 @@ public function fill(): self
                 }
             } while ($attempts < $this->getReconnectAttempts());
 
-            $connection->setID($this->getName().'-'.$i);
-
-            $this->pool[$i] = $connection;
+            $connection
+                ->setID($this->getName().'-'.uniqid())
+                ->setPool($this)
+            ;
         }
-
-        return $this;
-    }
-
-    /**
-     * @return Connection
-     */
-    public function pop(): Connection
-    {
-        if (empty($this->pool)) {
-            throw new Exception('Pool is empty');
+
+        if($connection instanceof Connection) { // connection is available, return it
+            $this->active[$connection->getID()] = $connection;
+            return $connection;
         }
 
-        $connection = array_pop($this->pool);
-        $this->active[$connection->getID()] = $connection;
-
-        return $connection;
+        throw new Exception('Failed to get a connection from the pool');
     }
 
     /**

diff --git a/tests/Pools/ConnectionTest.php b/tests/Pools/ConnectionTest.php
@@ -4,6 +4,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Utopia\Pools\Connection;
+use Utopia\Pools\Pool;
 
 class ConnectionTest extends TestCase
 {
@@ -50,4 +51,43 @@ public function testSetResource()
 
         $this->assertEquals('y', $this->object->getResource());
     }
+
+    public function testSetPool()
+    {
+        $pool = new Pool('test', 1, function () {
+            return 'x';
+        });
+
+        $this->assertNull($this->object->getPool());
+        $this->assertInstanceOf(Connection::class, $this->object->setPool($pool));
+    }
+
+    public function testGetPool()
+    {
+        $pool = new Pool('test', 1, function () {
+            return 'x';
+        });
+
+        $this->assertNull($this->object->getPool());
+        $this->assertInstanceOf(Connection::class, $this->object->setPool($pool));
+        $this->assertInstanceOf(Pool::class, $this->object->getPool());
+        $this->assertEquals('test', $this->object->getPool()->getName());
+    }
+
+    public function testReclame()
+    {
+        $pool = new Pool('test', 1, function () {
+            return 'x';
+        });
+
+        $this->assertEquals(1, $pool->count());
+
+        $connection = $pool->pop();
+
+        $this->assertEquals(0, $pool->count());
+
+        $this->assertInstanceOf(Pool::class, $connection->reclaim());
+
+        $this->assertEquals(1, $pool->count());
+    }
 }
diff --git a/tests/Pools/GroupTest.php b/tests/Pools/GroupTest.php
@@ -59,35 +59,12 @@ public function testRemove()
         $this->assertInstanceOf(Pool::class, $this->object->get('test'));
     }
 
-    public function testFill()
-    {
-        $this->object->add(new Pool('test', 1, function() {
-            return 'x';
-        }));
-
-        // Pool should be empty
-        try {
-            $this->object->get('test')->pop();
-            $this->fail('Exception not thrown');
-        } catch (Exception $e) {
-            $this->assertInstanceOf(Exception::class, $e);
-        }
-
-        $this->object->fill();
-
-        $this->assertInstanceOf(Connection::class, $this->object->get('test')->pop());
-    }
-
     public function testReset()
     {
         $this->object->add(new Pool('test', 5, function() {
             return 'x';
         }));
 
-        $this->assertEquals(0, $this->object->get('test')->count());
-
-        $this->object->fill();
-
         $this->assertEquals(5, $this->object->get('test')->count());
 
         $this->object->get('test')->pop();

diff --git a/tests/Pools/PoolTest.php b/tests/Pools/PoolTest.php
@@ -61,49 +61,8 @@ public function testSetReconnectSleep()
         $this->assertEquals(20, $this->object->getReconnectSleep());
     }
 
-    public function testFill()
-    {
-        $this->assertEquals(0, $this->object->count());
-
-        $this->object->fill();
-
-        $this->assertEquals(5, $this->object->count());
-    }
-
-    public function testFillFailure()
-    {
-        $this->object = new Pool('test', 5, function() {
-            throw new Exception();
-        });
-
-        $start = microtime(true);
-
-        $this->assertEquals(0, $this->object->count());
-
-        try {
-            $this->object->fill();
-            $this->fail('Exception not thrown');
-        } catch (Exception $e) {
-            $this->assertInstanceOf(Exception::class, $e);
-        }
-
-        $time = microtime(true) - $start;
-
-        $this->assertGreaterThan(2, $time);
-    }
-
     public function testPop()
     {
-        // Pool should be empty
-        try {
-            $this->object->pop();
-            $this->fail('Exception not thrown');
-        } catch (Exception $e) {
-            $this->assertInstanceOf(Exception::class, $e);
-        }
-
-        $this->object->fill();
-
         $this->assertEquals(5, $this->object->count());
 
         $connection = $this->object->pop();
@@ -112,20 +71,19 @@ public function testPop()
 
         $this->assertInstanceOf(Connection::class, $connection);
         $this->assertEquals('x', $connection->getResource());
+
+        // // Pool should be empty
+        $this->expectException(Exception::class);
+
+        $this->assertInstanceOf(Connection::class, $this->object->pop());
+        $this->assertInstanceOf(Connection::class, $this->object->pop());
+        $this->assertInstanceOf(Connection::class, $this->object->pop());
+        $this->assertInstanceOf(Connection::class, $this->object->pop());
+        $this->assertInstanceOf(Connection::class, $this->object->pop());
     }
 
     public function testPush()
     {
-        // Pool should be empty
-        try {
-            $this->object->pop();
-            $this->fail('Exception not thrown');
-        } catch (Exception $e) {
-            $this->assertInstanceOf(Exception::class, $e);
-        }
-
-        $this->object->fill();
-
         $this->assertEquals(5, $this->object->count());
 
         $connection = $this->object->pop();
@@ -142,10 +100,6 @@ public function testPush()
 
     public function testCount()
     {
-        $this->assertEquals(0, $this->object->count());
-
-        $this->object->fill();
-
         $this->assertEquals(5, $this->object->count());
 
         $connection = $this->object->pop();
@@ -157,12 +111,8 @@ public function testCount()
         $this->assertEquals(5, $this->object->count());
     }
 
-    public function testReset()
+    public function testReclaim()
     {
-        $this->assertEquals(0, $this->object->count());
-
-        $this->object->fill();
-
         $this->assertEquals(5, $this->object->count());
 
         $this->object->pop();
@@ -178,12 +128,6 @@ public function testReset()
 
     public function testIsEmpty()
     {
-        $this->assertEquals(true, $this->object->isEmpty());
-
-        $this->object->fill();
-
-        $this->assertEquals(false, $this->object->isEmpty());
-
         $this->object->pop();
         $this->object->pop();
         $this->object->pop();
@@ -195,10 +139,6 @@ public function testIsEmpty()
 
     public function testIsFull()
     {
-        $this->assertEquals(false, $this->object->isFull());
-
-        $this->object->fill();
-
         $this->assertEquals(true, $this->object->isFull());
 
         $connection = $this->object->pop();
@@ -228,9 +168,5 @@ public function testIsFull()
         $this->object->pop();
 
         $this->assertEquals(false, $this->object->isFull());
-
-        $this->object->fill();
-
-        $this->assertEquals(true, $this->object->isFull());
     }
 }