Fix recent regression where adaptive throttles stopped adjusting.

Extends regression tests to test response of adaptive throttles to ack'ed and expired packets.

3 files changed, 61 insertions, 49 deletions

diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPClient.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPClient.cs
index c768662..b70d861 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPClient.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPClient.cs
@@ -447,31 +447,29 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 long total = resend + land + wind + cloud + task + texture + asset;
                 m_throttleClient.TargetDripRate = total;
             }
-            else
-            {
-                TokenBucket bucket;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Resend];
-                bucket.RequestedDripRate = resend;
+            TokenBucket bucket;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Land];
-                bucket.RequestedDripRate = land;
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Resend];
+            bucket.RequestedDripRate = resend;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Wind];
-                bucket.RequestedDripRate = wind;
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Land];
+            bucket.RequestedDripRate = land;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Cloud];
-                bucket.RequestedDripRate = cloud;
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Wind];
+            bucket.RequestedDripRate = wind;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Asset];
-                bucket.RequestedDripRate = asset;
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Cloud];
+            bucket.RequestedDripRate = cloud;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Task];
-                bucket.RequestedDripRate = task;
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Asset];
+            bucket.RequestedDripRate = asset;
 
-                bucket = m_throttleCategories[(int)ThrottleOutPacketType.Texture];
-                bucket.RequestedDripRate = texture;
-            }
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Task];
+            bucket.RequestedDripRate = task;
+
+            bucket = m_throttleCategories[(int)ThrottleOutPacketType.Texture];
+            bucket.RequestedDripRate = texture;
 
             // Reset the packed throttles cached data
             m_packedThrottles = null;
diff --git a/OpenSim/Region/ClientStack/Linden/UDP/Tests/ThrottleTests.cs b/OpenSim/Region/ClientStack/Linden/UDP/Tests/ThrottleTests.cs
index 912c994..0560b9b 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/Tests/ThrottleTests.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/Tests/ThrottleTests.cs
@@ -184,7 +184,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests
             udpServer.Throttle.DebugLevel = 1;
             udpClient.ThrottleDebugLevel = 1;
 
-            // Total is 28000
+            // Total is 280000
             int resendBytes = 10000;
             int landBytes = 20000;
             int windBytes = 30000;
@@ -192,24 +192,38 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests
             int taskBytes = 50000;
             int textureBytes = 60000;
             int assetBytes = 70000;
+            int totalBytes = resendBytes + landBytes + windBytes + cloudBytes + taskBytes + textureBytes + assetBytes;
 
             SetThrottles(
                 udpClient, resendBytes, landBytes, windBytes, cloudBytes, taskBytes, textureBytes, assetBytes);
 
-            // We expect individual throttle changes to currently have no effect under adaptive, since this is managed
-            // purely by that throttle.  However, we expect the max to change.
-            // XXX: At the moment we check against defaults, but at some point there should be a better test to 
-            // active see change over time.
-            ThrottleRates defaultRates = udpServer.ThrottleRates;
+            // Ratio of current adaptive drip rate to requested bytes
+            // XXX: Should hard code this as below so we don't rely on values given by tested code to construct
+            // expected values.
+            double commitRatio = (double)udpClient.FlowThrottle.AdjustedDripRate / udpClient.FlowThrottle.TargetDripRate;
+
+            AssertThrottles(
+                udpClient, 
+                LLUDPServer.MTU, landBytes * commitRatio, windBytes * commitRatio, cloudBytes * commitRatio, taskBytes * commitRatio,
+                textureBytes * commitRatio, assetBytes * commitRatio, udpClient.FlowThrottle.AdjustedDripRate, totalBytes, 0);
+
+            // Test an increase in target throttle
+            udpClient.FlowThrottle.AcknowledgePackets(35000);
+            commitRatio = 0.2;
+
+            AssertThrottles(
+                udpClient, 
+                resendBytes * commitRatio, landBytes * commitRatio, windBytes * commitRatio, cloudBytes * commitRatio, taskBytes * commitRatio,
+                textureBytes * commitRatio, assetBytes * commitRatio, udpClient.FlowThrottle.AdjustedDripRate, totalBytes, 0);
 
-            // Current total is 66750
-            int totalBytes = defaultRates.Resend + defaultRates.Land + defaultRates.Wind + defaultRates.Cloud + defaultRates.Task + defaultRates.Texture + defaultRates.Asset;
-            int totalMaxBytes = resendBytes + landBytes + windBytes + cloudBytes + taskBytes + textureBytes + assetBytes;
+            // Test a decrease in target throttle
+            udpClient.FlowThrottle.ExpirePackets(1);
+            commitRatio = 0.1;
 
             AssertThrottles(
                 udpClient, 
-                defaultRates.Resend, defaultRates.Land, defaultRates.Wind, defaultRates.Cloud, defaultRates.Task, 
-                defaultRates.Texture, defaultRates.Asset, totalBytes, totalMaxBytes, 0);
+                LLUDPServer.MTU, landBytes * commitRatio, windBytes * commitRatio, cloudBytes * commitRatio, taskBytes * commitRatio,
+                textureBytes * commitRatio, assetBytes * commitRatio, udpClient.FlowThrottle.AdjustedDripRate, totalBytes, 0);
         }
 
         /// <summary>
@@ -376,9 +390,9 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests
         {
             ClientInfo ci = udpClient.GetClientInfo();
 
-            //                Console.WriteLine(
-            //                    "Resend={0}, Land={1}, Wind={2}, Cloud={3}, Task={4}, Texture={5}, Asset={6}, TOTAL = {7}", 
-            //                    ci.resendThrottle, ci.landThrottle, ci.windThrottle, ci.cloudThrottle, ci.taskThrottle, ci.textureThrottle, ci.assetThrottle, ci.totalThrottle);
+//                            Console.WriteLine(
+//                                "Resend={0}, Land={1}, Wind={2}, Cloud={3}, Task={4}, Texture={5}, Asset={6}, TOTAL = {7}", 
+//                                ci.resendThrottle, ci.landThrottle, ci.windThrottle, ci.cloudThrottle, ci.taskThrottle, ci.textureThrottle, ci.assetThrottle, ci.totalThrottle);
 
             Assert.AreEqual((int)resendBytes, ci.resendThrottle, "Resend");
             Assert.AreEqual((int)landBytes, ci.landThrottle, "Land");
diff --git a/OpenSim/Region/ClientStack/Linden/UDP/TokenBucket.cs b/OpenSim/Region/ClientStack/Linden/UDP/TokenBucket.cs
index ab8d268..d215595 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/TokenBucket.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/TokenBucket.cs
@@ -76,8 +76,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// Map of children buckets and their requested maximum burst rate
         /// </summary>
         protected Dictionary<TokenBucket,Int64> m_children = new Dictionary<TokenBucket,Int64>();
-        
-#region Properties
 
         /// <summary>
         /// The parent bucket of this bucket, or null if this bucket has no
@@ -123,7 +121,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// RequestedDripRate is set to 0.  Really, this should always return m_dripRate and then we can get
         /// (m_dripRate == 0 ? TotalDripRequest : m_dripRate) on some other properties.
         /// </remarks>
-        protected Int64 m_dripRate;
         public virtual Int64 RequestedDripRate
         {
             get { return (m_dripRate == 0 ? TotalDripRequest : m_dripRate); }
@@ -179,6 +176,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 return (Int64)rate;
             }
         }
+        protected Int64 m_dripRate;
 
         // <summary>
         // The maximum rate for flow control. Drip rate can never be greater than this.
@@ -189,10 +187,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// The current total of the requested maximum burst rates of children buckets.
         /// </summary>
         public Int64 TotalDripRequest { get; protected set; }
-        
-#endregion Properties
-
-#region Constructor
 
         /// <summary>
         /// Default constructor
@@ -200,20 +194,20 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// <param name="identifier">Identifier for this token bucket</param>
         /// <param name="parent">Parent bucket if this is a child bucket, or
         /// null if this is a root bucket</param>
-        /// <param name="dripRate">Rate that the bucket fills, in bytes per
-        /// second. If zero, the bucket always remains full</param>
-        public TokenBucket(string identifier, TokenBucket parent, Int64 dripRate, Int64 maxDripRate) 
+        /// <param name="requestedDripRate">
+        /// Requested rate that the bucket fills, in bytes per
+        /// second. If zero, the bucket always remains full.
+        /// </param>
+        public TokenBucket(string identifier, TokenBucket parent, Int64 requestedDripRate, Int64 maxDripRate) 
         {
             Identifier = identifier;
 
             Parent = parent;
-            RequestedDripRate = dripRate;
+            RequestedDripRate = requestedDripRate;
             MaxDripRate = maxDripRate;
             m_lastDrip = Util.EnvironmentTickCount();
         }
 
-#endregion Constructor
-
         /// <summary>
         /// Compute a modifier for the MaxBurst rate. This is 1.0, meaning
         /// no modification if the requested bandwidth is less than the
@@ -374,7 +368,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         public Int64 TargetDripRate 
         { 
             get { return m_targetDripRate; }
-            set { m_targetDripRate = Math.Max(0, value); }
+            set 
+            {
+                m_targetDripRate = Math.Max(value, m_minimumFlow);
+            }
         }
         protected Int64 m_targetDripRate;
 
@@ -384,9 +381,11 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         public virtual Int64 AdjustedDripRate
         {
             get { return m_dripRate; }
-            set {
+            set 
+            {
                 m_dripRate = OpenSim.Framework.Util.Clamp<Int64>(value, m_minimumFlow, TargetDripRate);
                 m_burstRate = (Int64)((double)m_dripRate * m_quantumsPerBurst);
+
                 if (Parent != null)
                     Parent.RegisterRequest(this, m_dripRate);
             }
@@ -399,14 +398,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP
         /// </summary>
         protected const Int64 m_minimumFlow = m_minimumDripRate * 15;
 
-        public AdaptiveTokenBucket(string identifier, TokenBucket parent, Int64 dripRate, Int64 maxDripRate, bool enabled) 
-            : base(identifier, parent, dripRate, maxDripRate)
+        public AdaptiveTokenBucket(string identifier, TokenBucket parent, Int64 requestedDripRate, Int64 maxDripRate, bool enabled) 
+            : base(identifier, parent, requestedDripRate, maxDripRate)
         {
             AdaptiveEnabled = enabled;
 
             if (AdaptiveEnabled)
             {
 //                m_log.DebugFormat("[TOKENBUCKET]: Adaptive throttle enabled");
+                TargetDripRate = m_minimumFlow;
                 AdjustedDripRate = m_minimumFlow;
             }
         }