From 1c12930c036adfa479e8ec2165853942f03c4ed3 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 28 Oct 2014 00:44:03 +0000 Subject: [PATCH] Fix recent regression where adaptive throttles stopped adjusting. Extends regression tests to test response of adaptive throttles to ack'ed and expired packets. --- .../ClientStack/Linden/UDP/LLUDPClient.cs | 34 +++++++------- .../Linden/UDP/Tests/ThrottleTests.cs | 44 ++++++++++++------- .../ClientStack/Linden/UDP/TokenBucket.cs | 34 +++++++------- 3 files changed, 62 insertions(+), 50 deletions(-) diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPClient.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPClient.cs index c7686621bf..b70d861279 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 912c9940f5..0560b9bc32 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; - - // 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; + // 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, - 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); + + // 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); + + // Test a decrease in target throttle + udpClient.FlowThrottle.ExpirePackets(1); + commitRatio = 0.1; + + AssertThrottles( + udpClient, + LLUDPServer.MTU, landBytes * commitRatio, windBytes * commitRatio, cloudBytes * commitRatio, taskBytes * commitRatio, + textureBytes * commitRatio, assetBytes * commitRatio, udpClient.FlowThrottle.AdjustedDripRate, totalBytes, 0); } /// @@ -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 ab8d2685d5..d215595c7b 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 /// protected Dictionary m_children = new Dictionary(); - -#region Properties /// /// 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. /// - 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; // // 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. /// public Int64 TotalDripRequest { get; protected set; } - -#endregion Properties - -#region Constructor /// /// Default constructor @@ -200,20 +194,20 @@ namespace OpenSim.Region.ClientStack.LindenUDP /// Identifier for this token bucket /// Parent bucket if this is a child bucket, or /// null if this is a root bucket - /// Rate that the bucket fills, in bytes per - /// second. If zero, the bucket always remains full - public TokenBucket(string identifier, TokenBucket parent, Int64 dripRate, Int64 maxDripRate) + /// + /// Requested rate that the bucket fills, in bytes per + /// second. If zero, the bucket always remains full. + /// + 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 - /// /// 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(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 /// 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; } }