Connector/NET commit: r605 - in branches/5.0: . Driver/Source TestSuite

[rburnett]

Modified:
   branches/5.0/CHANGES
   branches/5.0/Driver/Source/MySqlPool.cs
   branches/5.0/Driver/Source/Resources.Designer.cs
   branches/5.0/Driver/Source/Resources.resx
   branches/5.0/TestSuite/PoolingTests.cs
Log:
Bug #24373 High CPU utilization when no idle connection 

Fixed this and improved the overall pooling code by using a semaphore to control access to
the pooled connections.

Modified: branches/5.0/CHANGES
===================================================================
--- branches/5.0/CHANGES	2007-02-21 18:46:39 UTC (rev 604)
+++ branches/5.0/CHANGES	2007-02-22 20:06:20 UTC (rev 605)
@@ -18,6 +18,7 @@
   Bug #26430 Will not install under Vista   
   Bug #25605 BINARY and VARBINARY is returned as a string 
   Bug #26152 Opening a connection is really slow 
+  Bug #24373 High CPU utilization when no idle connection 
 
   Other changes
   -------------

Modified: branches/5.0/Driver/Source/MySqlPool.cs
===================================================================
--- branches/5.0/Driver/Source/MySqlPool.cs	2007-02-21 18:46:39 UTC (rev 604)
+++ branches/5.0/Driver/Source/MySqlPool.cs	2007-02-22 20:06:20 UTC (rev 605)
@@ -22,6 +22,8 @@
 using MySql.Data.Common;
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
+using System.Threading;
 
 namespace MySql.Data.MySqlClient
 {
@@ -41,11 +43,15 @@
 		private uint minSize;
 		private uint maxSize;
         private ProcedureCache procedureCache;
+        private Object lockObject;
+        private Semaphore poolGate;
 
 		public MySqlPool(MySqlConnectionStringBuilder settings)
 		{
 			minSize = settings.MinimumPoolSize;
 			maxSize = settings.MaximumPoolSize;
+            if (minSize > maxSize)
+                minSize = maxSize;
 			this.settings = settings;
 #if NET20
             inUsePool = new List<Driver>((int)maxSize);
@@ -60,9 +66,15 @@
 				CreateNewPooledConnection();
 
             procedureCache = new ProcedureCache(settings.ProcedureCacheSize);
-		}
+            poolGate = new Semaphore((int)maxSize, (int)maxSize);
 
-		public MySqlConnectionStringBuilder	Settings 
+            // we don't really need to create this but it makes the code a bit cleaner
+            lockObject = new Object();
+        }
+
+        #region Properties
+
+        public MySqlConnectionStringBuilder	Settings 
 		{
 			get { return settings; }
 			set { settings = value; }
@@ -73,94 +85,101 @@
             get { return procedureCache; }
         }
 
-/*		private int CheckConnections() 
+        /// <summary>
+        /// It is assumed that this property will only be used from inside an active
+        /// lock.
+        /// </summary>
+        private bool HasIdleConnections
+        {
+            get { return idlePool.Count > 0; }
+        }
+
+        /// <summary>
+        /// It is assumed that this property will only be used from inside an active
+        /// lock.
+        /// </summary>
+        private bool HasRoomForConnections
+        {
+            get
+            {
+                if ((inUsePool.Count + idlePool.Count) == maxSize)
+                    return false;
+                return true;
+            }
+
+        }
+
+        #endregion
+
+        /// <summary>
+        /// CheckoutConnection handles the process of pulling a driver
+        /// from the idle pool, possibly resetting its state,
+        /// and adding it to the in use pool.  We assume that this method is only
+        /// called inside an active lock so there is no need to acquire a new lock.
+        /// </summary>
+        /// <returns>An idle driver object</returns>
+		private Driver CheckoutConnection()
 		{
-			int freed = 0;
-			lock (inUsePool.SyncRoot) 
+			Driver driver = (Driver)idlePool.Dequeue();
+
+			// if the user asks us to ping/reset pooled connections
+			// do so now
+			if (settings.ConnectionReset)
 			{
-				for (int i=inUsePool.Count-1; i >= 0; i--) 
+				if (!driver.Ping())
 				{
-					Driver d = (inUsePool[i] as Driver);
-					if (! d.Ping()) 
-					{
-						inUsePool.RemoveAt(i);
-						freed++;
-					}
+					driver.Close();
+					return null;
 				}
+				driver.Reset();
 			}
-			return freed;
-		}
-*/
-		private Driver CheckoutConnection()
-		{
-			lock((idlePool as ICollection).SyncRoot)
-			{
-				if (idlePool.Count == 0) return null;
-				Driver driver = (Driver)idlePool.Dequeue();
 
-				// if the user asks us to ping/reset pooled connections
-				// do so now
-				if (settings.ConnectionReset)
-				{
-					if (!driver.Ping())
-					{
-						driver.Close();
-						return null;
-					}
-					driver.Reset();
-				}
+			inUsePool.Add(driver);
 
-                lock ((inUsePool as ICollection).SyncRoot)
-                {
-					inUsePool.Add(driver);
-				}
-				return driver;
-			}
+			return driver;
 		}
 
-		private Driver GetPooledConnection()
+        /// <summary>
+        /// It is assumed that this method is only called from inside an active lock.
+        /// </summary>
+        private Driver GetPooledConnection()
 		{
-			while (true)
-			{
-				if (idlePool.Count > 0)
-					return CheckoutConnection();
+            // if we don't have an idle connection but we have room for a new
+            // one, then create it here.
+            if (!HasIdleConnections)
+                CreateNewPooledConnection();
 
-				// if idlepool == 0 and inusepool == max, then we can't create a new one
-				if (inUsePool.Count == maxSize)
-					return null;
-
-				CreateNewPooledConnection();
-			}
+            return CheckoutConnection();
 		}
 
+        /// <summary>
+        /// It is assumed that this method is only called from inside an active lock.
+        /// </summary>
 		private void CreateNewPooledConnection()
 		{
-            lock ((idlePool as ICollection).SyncRoot)
-                lock ((inUsePool as ICollection).SyncRoot)
-                {
-					// first we check if we are allowed to create another
-					if ((inUsePool.Count + idlePool.Count) == maxSize)
-						return;
-
-					Driver driver = Driver.Create(settings);
-
-					idlePool.Enqueue(driver);
-				}
+			Driver driver = Driver.Create(settings);
+			idlePool.Enqueue(driver);
 		}
 
 		public void ReleaseConnection(Driver driver)
 		{
-            lock ((idlePool as ICollection).SyncRoot)
-                lock ((inUsePool as ICollection).SyncRoot)
-                {
-					inUsePool.Remove(driver);
-					if (driver.IsTooOld())
-						driver.Close();
-					else
-						idlePool.Enqueue(driver);
-				}
-		}
+            lock (lockObject)
+            {
+                if (!inUsePool.Contains(driver))
+                    return;
 
+                inUsePool.Remove(driver);
+                if (driver.IsTooOld())
+                    driver.Close();
+                else
+                    idlePool.Enqueue(driver);
+
+                // we now either have a connection available or have room to make
+                // one so we release one slot in our semaphore
+                poolGate.Release();
+            }
+        }
+
         /// <summary>
         /// Removes a connection from the in use pool.  The only situations where this
method 
         /// would be called are when a connection that is in use gets some type of fatal
exception
@@ -170,32 +189,31 @@
         /// <param name="driver"></param>
         public void RemoveConnection(Driver driver)
         {
-            lock ((inUsePool as ICollection).SyncRoot)
+            lock (lockObject)
             {
-                inUsePool.Remove(driver);
+                if (inUsePool.Contains(driver))
+                {
+                    inUsePool.Remove(driver);
+                    poolGate.Release();
+                }
             }
         }
 
 		public Driver GetConnection() 
 		{
-			Driver driver = null;
+			int ticks = (int)settings.ConnectionTimeout * 1000;
 
-			int start = Environment.TickCount;
-			uint ticks = settings.ConnectionTimeout * 1000;
+            // wait till we are allowed in
+            bool allowed = poolGate.WaitOne(ticks, false);
+            if (! allowed)
+                throw new TimeoutException(Resources.TimeoutGettingConnection);
 
-			// wait timeOut seconds at most to get a connection
-			while (driver == null && (Environment.TickCount - start) < ticks)
-				driver = GetPooledConnection();
-					 
-			// if pool size is at maximum, then we must have reached our timeout so we simply
-			// throw our exception
-			if (driver == null)
-				throw new MySqlException("error connecting: Timeout expired.  The timeout period
elapsed " + 
-					"prior to obtaining a connection from the pool.  This may have occurred because all
" +
-					"pooled connections were in use and max pool size was reached.");
-
-			return driver;
+            // if we get here, then it means that we either have an idle connection
+            // or room to make a new connection
+            lock (lockObject)
+            {
+                return GetPooledConnection();
+            }
 		}
-
 	}
 }

Modified: branches/5.0/Driver/Source/Resources.Designer.cs
===================================================================
--- branches/5.0/Driver/Source/Resources.Designer.cs	2007-02-21 18:46:39 UTC (rev 604)
+++ branches/5.0/Driver/Source/Resources.Designer.cs	2007-02-22 20:06:20 UTC (rev 605)
@@ -574,6 +574,15 @@
         }
         
         /// <summary>
+        ///   Looks up a localized string similar to error connecting: Timeout expired. 
The timeout period elapsed prior to obtaining a connection from the pool.  This may have
occurred because all pooled connections were in use and max pool size was reached..
+        /// </summary>
+        internal static string TimeoutGettingConnection {
+            get {
+                return ResourceManager.GetString("TimeoutGettingConnection",
resourceCulture);
+            }
+        }
+        
+        /// <summary>
         ///   Looks up a localized string similar to Unable to connect to any of the
specified MySQL hosts..
         /// </summary>
         internal static string UnableToConnectToHost {

Modified: branches/5.0/Driver/Source/Resources.resx
===================================================================
--- branches/5.0/Driver/Source/Resources.resx	2007-02-21 18:46:39 UTC (rev 604)
+++ branches/5.0/Driver/Source/Resources.resx	2007-02-22 20:06:20 UTC (rev 605)
@@ -309,4 +309,7 @@
   <data name="NoBodiesAndTypeNotSet" xml:space="preserve">
     <value>When calling stored procedures and 'Use Procedure Bodies' is false, all
parameters must have their type explicitly set.</value>
   </data>
+  <data name="TimeoutGettingConnection" xml:space="preserve">
+    <value>error connecting: Timeout expired.  The timeout period elapsed prior to
obtaining a connection from the pool.  This may have occurred because all pooled
connections were in use and max pool size was reached.</value>
+  </data>
 </root>
\ No newline at end of file

Modified: branches/5.0/TestSuite/PoolingTests.cs
===================================================================
--- branches/5.0/TestSuite/PoolingTests.cs	2007-02-21 18:46:39 UTC (rev 604)
+++ branches/5.0/TestSuite/PoolingTests.cs	2007-02-22 20:06:20 UTC (rev 605)
@@ -223,6 +223,40 @@
 
 
 
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 
 
\ No newline at end of file