Bug #66476 MySqlPool - use Stack instead of Queue for better idlepool cleanup
Submitted: 21 Aug 2012 2:54 Modified: 2 Aug 2022 14:49
Reporter: Poul Bak Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / NET Severity:S3 (Non-critical)
Version:6.6.1 OS:Any
Assigned to: CPU Architecture:Any
Tags: MySqlPool idle cleanup

[21 Aug 2012 2:54] Poul Bak
Description:
If the connection idle pool was a human retirement home, .net connector would get a reward for keeping everybody young :-)

The idle pool is currently a queue. This means that when a connection is taken from the pool, the oldest one (the one that has been idle for the longest time) is returned, effectively  refreshing this item.
A stack will return the newest and let the old ones grow even older - and then we can remove them.

This is especially important to remove connections created from spikes, while still keeping the necessary number of connections in the pool.
The term 'necessary' here means connections having been used within 3 minutes.

How to repeat:
(To see opened connections, use phpMyAdmin or similar tool)

Consider this (asp.net) example (for ease, we say one connection is made per request):
A request comes in and is sent to the client. The browser now requests all images and scripts, several at a time, making several connections(lets say 5 connections). 
Now lets say, that one request (without images, scripts)  comes in
every 1/2 minute (requiring 1 connection).
With the queue returning the oldest connection, all 5 will have been refreshed after 3 minutes and none will be removed.
With a stack the newest will be reused every time and the 4 others will keep growing older and can be removed, because only one connection is enough to serve that request every half minute.

To test this, have a full page with images, scripts and one request for a service or similar that doesn't cause sub-requests. Then try repeating the above.

Suggested fix:
In MySqlPool.cs, line 38, replace Queue with Stack:

        private Stack<Driver> idlePool;

Replace 'EnQueue' with 'Push' and 'DeQueue' with 'Pop'.
The biggest change is in 'RemoveOldIdleConnections()', because we have have to do the cleaning up in reverse order:

        internal List<Driver> RemoveOldIdleConnections()
        {
            DateTime now = DateTime.Now;

            lock ((idlePool as ICollection).SyncRoot)
            {
                // Rebuild the Stack with only newly used drivers, 
                // and return old idle drivers
                List<Driver> oldDrivers = new List<Driver>(idlePool.ToArray());
                idlePool.Clear();
                // Newest driver at index 0, iterate in reverse order
                for (int i = oldDrivers.Count - 1; i >= 0; i--)
                {
                    Driver d = oldDrivers[i];
                    DateTime expirationTime = d.IdleSince.Add(
                        TimeSpan.FromSeconds(MySqlPoolManager.maxConnectionIdleTime));
                    if ((i < minSize) || (expirationTime > now))
                    {
                        idlePool.Push(d);
                        oldDrivers.Remove(d);
                    }
                }
                return oldDrivers;
            }
        }

Finally, renamed 'EnQueueIdle' to 'PushIdle'.
[21 Aug 2012 2:55] Poul Bak
MySqlPool.cs after change

Attachment: MySqlPool.cs (text/plain), 11.68 KiB.

[21 Aug 2012 3:33] Poul Bak
The change should be able to go into earlier versions of .Net connector too.
[28 Aug 2012 14:55] Poul Bak
I should mention that I have run this code for a week now in production.
[15 Jul 2016 11:06] Chiranjeevi Battula
Hello Poul Bak,

Thank you for the bug report.
Verified based on internal discussion with dev's.

Thanks,
Chiranjeevi.
[3 Feb 2022 13:48] Bradley Grainger
See #106368 for an updated version of this suggestion that uses LinkedList<T>.
[2 Aug 2022 1:52] Bradley Grainger
This is now fixed in 8.0.30; see bug #106368.
[2 Aug 2022 14:47] Daniel Valdez
Posted by developer:
 
Fixed in Connector/NET v8.0.30. (see 33935441)