Handle connections from a pool

README.md

@@ -200,6 +200,14 @@ Update the connection authentication settings.
 
 Throws an error on failure.
 
+### `conn:close()`
+
+Close the individual connection or return it to a pool.
+
+Throws an error on failure.
+
+*Returns*: `true`
+
 ### `pool = mysql.pool_create(opts)`
 
 Create a connection pool with count of size established connections.
@@ -247,6 +255,12 @@ Return a connection to connection pool.
 
  - `conn` - a connection
 
+### `pool:close()`
+
+Close all connections in pool.
+
+*Returns*: `true`
+
 ## Comments
 
 All calls to connections api will be serialized, so it should to be safe to

mysql/init.lua

@@ -9,9 +9,6 @@ local conn_mt
 
 -- The marker for empty slots in a connection pool.
 --
--- When a user puts a connection that is in an unusable state to a
--- pool, we put this marker to a pool's internal connection queue.
---
 -- Note: It should not be equal to `nil`, because fiber channel's
 -- `get` method returns `nil` when a timeout is reached.
 local POOL_EMPTY_SLOT = true
@@ -35,8 +32,8 @@ local function conn_get(pool, timeout)
     -- A timeout was reached.
     if mysql_conn == nil then return nil end
 
-    local status
     if mysql_conn == POOL_EMPTY_SLOT then
+        local status
         status, mysql_conn = driver.connect(pool.host, pool.port or 0,
                                             pool.user, pool.pass,
                                             pool.db, pool.use_numeric_result)
@@ -52,6 +49,17 @@ local function conn_get(pool, timeout)
                 mysql_conn:close()
                 pool.queue:put(POOL_EMPTY_SLOT)
             end)
+    -- If the connection belongs to a connection pool, it must be returned to
+    -- the pool when calling "close" without actually closing the connection.
+    -- In the case of a double "close", the behavior is the same as with a
+    -- simple connection.
+    conn.close = function(self)
+        if not self.usable then
+            error('Connection is not usable')
+        end
+        pool:put(self)
+        return true
+    end
     return conn
 end
 
@@ -163,9 +171,7 @@ local function pool_create(opts)
                 local mysql_conn = queue:get()
                 mysql_conn:close()
             end
-            if status < 0 then
-                error(conn)
-            end
+            error(conn)
         end
         queue:put(conn)
     end
@@ -195,7 +201,7 @@ local function pool_close(self)
             mysql_conn:close()
         end
     end
-    return 1
+    return true
 end
 
 -- Returns connection
@@ -219,7 +225,7 @@ local function pool_put(self, conn)
     if conn.usable then
         self.queue:put(conn_put(conn))
     else
-        self.queue:put(POOL_EMPTY_SLOT)
+        error('Connection is not usable')
     end
 end
 

test/mysql.test.lua

@@ -171,7 +171,7 @@ local function test_mysql_int64(t, p)
 end
 
 local function test_connection_pool(test, pool)
-    test:plan(6)
+    test:plan(11)
 
     -- {{{ Case group: all connections are consumed initially.
 
@@ -302,13 +302,13 @@ local function test_connection_pool(test, pool)
 
         -- Put the connection back and verify that the pool is full.
         pool:put(conn)
-        test:ok(pool.queue:is_full(), 'a broken connection was given back')
+        test:ok(pool.queue:is_full(), 'a connection was given back')
     end)
 
     -- Case: the same, but loss and collect a connection after
     -- put.
     test:test('get, put and loss a connection', function(test)
-        test:plan(2)
+        test:plan(1)
 
         assert(pool.size >= 1, 'test case precondition fails')
 
@@ -321,14 +321,7 @@ local function test_connection_pool(test, pool)
         collectgarbage('collect')
 
         -- Verify that the pool is full.
-        test:ok(pool.queue:is_full(), 'a broken connection was given back')
-
-        -- Verify the pool will not be populated by a connection's
-        -- GC callback. Otherwise :put() will hang.
-        local item = pool.queue:get()
-        pool.queue:put(item)
-        test:ok(true, 'GC callback does not put back a connection that was ' ..
-            'put manually')
+        test:ok(pool.queue:is_full(), 'a connection was given back')
     end)
 
     -- Case: get a connection, broke it and put back.
@@ -350,7 +343,7 @@ local function test_connection_pool(test, pool)
     -- Case: the same, but loss and collect a connection after
     -- put.
     test:test('get, broke, put and loss a connection', function(test)
-        test:plan(3)
+        test:plan(2)
 
         assert(pool.size >= 1, 'test case precondition fails')
 
@@ -366,24 +359,10 @@ local function test_connection_pool(test, pool)
 
         -- Verify that the pool is full
         test:ok(pool.queue:is_full(), 'a broken connection was given back')
-
-        -- Verify the pool will not be populated by a connection's
-        -- GC callback. Otherwise :put() will hang.
-        local item = pool.queue:get()
-        pool.queue:put(item)
-        test:ok(true, 'GC callback does not put back a connection that was ' ..
-            'put manually')
     end)
 
-    --[[
-
-    -- It is unclear for now whether putting of closed connection
-    -- should be allowed. The second case, where GC collects lost
-    -- connection after :put(), does not work at the moment. See
-    -- gh-33.
-
-    -- Case: get a connection, close it and put back.
-    test:test('get, close and put a connection', function(test)
+    -- Case: get a connection and close it.
+    test:test('get and close a connection', function(test)
         test:plan(1)
 
         assert(pool.size >= 1, 'test case precondition fails')
@@ -392,39 +371,78 @@ local function test_connection_pool(test, pool)
         local conn = pool:get()
         conn:close()
 
-        -- Put a connection back and verify that the pool is full.
-        pool:put(conn)
-        test:ok(pool.queue:is_full(), 'a broken connection was given back')
+        test:ok(pool.queue:is_full(), 'a connection was given back')
     end)
 
     -- Case: the same, but loss and collect a connection after
-    -- put.
-    test:test('get, close, put and loss a connection', function(test)
-        test:plan(2)
+    -- close.
+    test:test('get, close and loss a connection', function(test)
+        test:plan(1)
 
         assert(pool.size >= 1, 'test case precondition fails')
 
         -- Get a connection and close it.
         local conn = pool:get()
         conn:close()
 
-        -- Put the connection back, loss it and trigger GC.
-        pool:put(conn)
-        conn = nil
+        conn = nil -- luacheck: no unused
         collectgarbage('collect')
 
         -- Verify that the pool is full
         test:ok(pool.queue:is_full(), 'a broken connection was given back')
+    end)
+
+    -- Case: get a connection, close and put it back.
+    test:test('get, close and put a connection', function(test)
+        test:plan(2)
 
-        -- Verify the pool will not be populated by a connection's
-        -- GC callback. Otherwise :put() will hang.
-        local item = pool.queue:get()
-        pool.queue:put(item)
-        test:ok(true, 'GC callback does not put back a connection that was ' ..
-            'put manually')
+        assert(pool.size >= 1, 'test case precondition fails')
+
+        -- Get a connection.
+        local conn = pool:get()
+
+        conn:close()
+        test:ok(pool.queue:is_full(), 'a connection was given back')
+
+        -- Put must throw an error.
+        local res = pcall(pool.put, pool, conn)
+        test:ok(not res, 'an error is thrown on "put" after "close"')
+    end)
+
+     -- Case: close the same connection twice.
+    test:test('close a connection twice', function(test)
+        test:plan(2)
+
+        assert(pool.size >= 1, 'test case precondition fails')
+
+        local conn = pool:get()
+        conn:close()
+        test:ok(pool.queue:is_full(), 'a connection was given back')
+
+        local res = pcall(conn.close, conn)
+        test:ok(not res, 'an error is thrown on double "close"')
     end)
 
-    --]]
+    -- Case: put the same connection twice.
+    test:test('put a connection twice', function(test)
+        test:plan(3)
+
+        assert(pool.size >= 2, 'test case precondition fails')
+
+        local conn_1 = pool:get()
+        local conn_2 = pool:get()
+        pool:put(conn_1)
+
+        test:ok(not pool.queue:is_full(),
+                'the same connection has not been "put" twice')
+
+        local res = pcall(pool.put, pool, conn_1)
+        test:ok(not res, 'an error is thrown on double "put"')
+
+        pool:put(conn_2)
+        test:ok(pool.queue:is_full(),
+                'all connections were returned to the pool')
+    end)
 
     assert(pool.queue:is_full(), 'case group postcondition fails')
 
@@ -469,7 +487,6 @@ test:plan(7)
 test:test('connection old api', test_old_api, conn)
 local pool_conn = p:get()
 test:test('connection old api via pool', test_old_api, pool_conn)
-p:put(pool_conn)
 test:test('garbage collection', test_gc, p)
 test:test('concurrent connections', test_conn_concurrent, p)
 test:test('int64', test_mysql_int64, p)