From 8c8a1906a5105dbae1e92b5154b2917324a6069e Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 13 Jan 2025 21:51:01 +0300 Subject: [PATCH 1/7] #2246 Add draft reproducing test --- test/Ocelot.AcceptanceTests/Core/LoadTests.cs | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 test/Ocelot.AcceptanceTests/Core/LoadTests.cs diff --git a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs new file mode 100644 index 000000000..5f1d42079 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs @@ -0,0 +1,116 @@ +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration.File; +using Ocelot.LoadBalancer.LoadBalancers; +using System.Diagnostics; + +namespace Ocelot.AcceptanceTests.Core; + +public sealed class LoadTests : ConcurrentSteps, IDisposable +{ + private readonly ServiceHandler _serviceHandler; + private string _downstreamPath; + private string _downstreamQuery; + + public LoadTests() + { + _serviceHandler = new(); + } + + public override void Dispose() + { + _serviceHandler.Dispose(); + base.Dispose(); + } + + [Fact] + [Trait("Feat", "1348")] + [Trait("Bug", "2246")] + public async Task ShouldLoadRegexCachingHeavily_NoOutOfMemoryExceptions_NoMemoryLeaks() + { + var limit = Environment.GetEnvironmentVariable("DOTNET_GCHeapHardLimit"); + var port = PortFinder.GetRandomPort(); + var route = GivenRoute(port, "/my-gateway/order/{orderNumber}", "/order/{orderNumber}"); + var configuration = GivenConfiguration(route); + GivenThereIsAConfiguration(configuration); + GivenOcelotIsRunning(); + GivenThereIsAServiceRunningOn(port, "/order/", "Hello from Donny"); + + var currentProcess = Process.GetCurrentProcess(); + long memoryUsage = currentProcess.WorkingSet64; + float memoryUsageMB = (float)memoryUsage / (1024 * 1024); + + // Step 1: Measure memory consumption for constant upstream URL + GC.Collect(); + var a = GC.GetGCMemoryInfo(); + long step1TotalMemory = GC.GetTotalMemory(true); + long step1TotalBytes = GC.GetTotalAllocatedBytes(); + long step1ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); + + //.When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 50)) + //.When(x => RunParallelRequests(100_000, (i) => "/my-gateway/order/" + i)) + await WhenIDoActionMultipleTimes(10_000, (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/1")); // const url + + GC.Collect(); + step1TotalMemory = GC.GetTotalMemory(true); + step1TotalBytes = GC.GetTotalAllocatedBytes(); + step1ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); + long memoryUsage1 = currentProcess.WorkingSet64; + float memoryUsageMB1 = (float)memoryUsage / (1024 * 1024); + + await WhenIGetUrlOnTheApiGateway("/my-gateway/order/1"); + ThenTheStatusCodeShouldBe(HttpStatusCode.OK); + ThenTheResponseBodyShouldBe("Hello from Donny"); + + // Step 2: Measure memory consumption for varying upstream URL + GC.Collect(); + long step2TotalMemory = GC.GetTotalMemory(true); + long step2TotalBytes = GC.GetTotalAllocatedBytes(); + long step2ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); + + await WhenIDoActionMultipleTimes(10_000, (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/" + i)); // varying url + + GC.Collect(); + step2TotalMemory = GC.GetTotalMemory(true); + step2TotalBytes = GC.GetTotalAllocatedBytes(); + step2ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); + long memoryUsage2 = currentProcess.WorkingSet64; + float memoryUsageMB2 = (float)memoryUsage / (1024 * 1024); + } + + private async Task WhenIGetUrlOnTheApiGatewaySequentially(int i) + { + //int count = i + 1; + await WhenIGetUrlOnTheApiGateway("/my-gateway/order/" + i); + + //ThenTheStatusCodeShouldBe(HttpStatusCode.OK); + //ThenServiceShouldHaveBeenCalledTimes(0, count); + //ThenTheResponseBodyShouldBe($"{count}:{Bug2119ServiceNames[0]}", $"i is {i}"); + } + + private FileRoute GivenRoute(int port, string upstream, string downstream) => new() + { + DownstreamPathTemplate = string.IsNullOrEmpty(downstream) ? "/" : downstream, + DownstreamScheme = Uri.UriSchemeHttp, + UpstreamPathTemplate = string.IsNullOrEmpty(upstream) ? "/" : upstream, + UpstreamHttpMethod = new() { HttpMethods.Get }, + LoadBalancerOptions = new() { Type = nameof(NoLoadBalancer) }, + DownstreamHostAndPorts = new() { Localhost(port) }, + }; + + private void GivenThereIsAServiceRunningOn(int port, string basePath, string responseBody) + { + var baseUrl = DownstreamUrl(port); + _serviceHandler.GivenThereIsAServiceRunningOn(baseUrl, basePath, MapGet); + + async Task MapGet(HttpContext context) + { + _downstreamPath = !string.IsNullOrEmpty(context.Request.PathBase.Value) + ? context.Request.PathBase.Value + context.Request.Path.Value + : context.Request.Path.Value; + _downstreamQuery = context.Request.QueryString.HasValue ? context.Request.QueryString.Value : string.Empty; + var isOK = _downstreamPath.StartsWith(basePath); + context.Response.StatusCode = isOK ? (int)HttpStatusCode.OK : (int)HttpStatusCode.NotFound; + await context.Response.WriteAsync(isOK ? responseBody : nameof(HttpStatusCode.NotFound)); + } + } +} From fa6dde60a242fc41962ef29d7be6a9d858ea9f96 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 14 Jan 2025 18:51:45 +0300 Subject: [PATCH 2/7] Refactor IUrlPathToUrlTemplateMatcher interface: Replace returning type Response with UrlMatch, but it should be simple bool actually --- .../Finder/DownstreamRouteFinder.cs | 3 +- .../IUrlPathToUrlTemplateMatcher.cs | 3 +- .../UrlMatcher/RegExUrlMatcher.cs | 20 +++------- .../DownstreamRouteFinderTests.cs | 40 +++++++++---------- .../UrlMatcher/RegExUrlMatcherTests.cs | 6 +-- 5 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index f74e94ce4..2dff92dde 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -37,8 +37,7 @@ public Response Get(string upstreamUrlPath, string upstre { var urlMatch = _urlMatcher.Match(upstreamUrlPath, upstreamQueryString, route.UpstreamTemplatePattern); var headersMatch = _headerMatcher.Match(upstreamHeaders, route.UpstreamHeaderTemplates); - - if (urlMatch.Data.Match && headersMatch) + if (urlMatch.Match && headersMatch) { downstreamRoutes.Add(GetPlaceholderNamesAndValues(upstreamUrlPath, upstreamQueryString, route, upstreamHeaders)); } diff --git a/src/Ocelot/DownstreamRouteFinder/UrlMatcher/IUrlPathToUrlTemplateMatcher.cs b/src/Ocelot/DownstreamRouteFinder/UrlMatcher/IUrlPathToUrlTemplateMatcher.cs index 00088e02e..2d4b68a77 100644 --- a/src/Ocelot/DownstreamRouteFinder/UrlMatcher/IUrlPathToUrlTemplateMatcher.cs +++ b/src/Ocelot/DownstreamRouteFinder/UrlMatcher/IUrlPathToUrlTemplateMatcher.cs @@ -1,9 +1,8 @@ -using Ocelot.Responses; using Ocelot.Values; namespace Ocelot.DownstreamRouteFinder.UrlMatcher; public interface IUrlPathToUrlTemplateMatcher { - Response Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate); + UrlMatch Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate); } diff --git a/src/Ocelot/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcher.cs b/src/Ocelot/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcher.cs index e854508bc..663a034a1 100644 --- a/src/Ocelot/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcher.cs +++ b/src/Ocelot/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcher.cs @@ -1,21 +1,11 @@ -using Ocelot.Responses; -using Ocelot.Values; +using Ocelot.Values; namespace Ocelot.DownstreamRouteFinder.UrlMatcher; public class RegExUrlMatcher : IUrlPathToUrlTemplateMatcher { - public Response Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate) - { - if (!pathTemplate.ContainsQueryString) - { - return pathTemplate.Pattern.IsMatch(upstreamUrlPath) - ? new OkResponse(new UrlMatch(true)) - : new OkResponse(new UrlMatch(false)); - } - - return pathTemplate.Pattern.IsMatch($"{upstreamUrlPath}{upstreamQueryString}") - ? new OkResponse(new UrlMatch(true)) - : new OkResponse(new UrlMatch(false)); - } + public UrlMatch Match(string upstreamUrlPath, string upstreamQueryString, UpstreamPathTemplate pathTemplate) + => !pathTemplate.ContainsQueryString + ? new(pathTemplate.Pattern.IsMatch(upstreamUrlPath)) + : new(pathTemplate.Pattern.IsMatch($"{upstreamUrlPath}{upstreamQueryString}")); } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 5c7e3079e..abd0c575d 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -20,7 +20,7 @@ public class DownstreamRouteFinderTests : UnitTest private Response _result; private List _routesConfig; private InternalConfiguration _config; - private Response _match; + private UrlMatch _match; private string _upstreamHttpMethod; private string _upstreamHost; private Dictionary _upstreamHeaders; @@ -65,7 +65,7 @@ public void should_return_highest_priority_when_first() .WithUpstreamPathTemplate(new UpstreamPathTemplate("test", 0, false, "someUpstreamPath")) .Build(), }, string.Empty, serviceProviderConfig)) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) .When(x => x.WhenICallTheFinder()) @@ -113,7 +113,7 @@ public void should_return_highest_priority_when_lowest() .WithUpstreamPathTemplate(new UpstreamPathTemplate("test", 1, false, "someUpstreamPath")) .Build(), }, string.Empty, serviceProviderConfig)) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) .When(x => x.WhenICallTheFinder()) @@ -154,7 +154,7 @@ public void should_return_route() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -199,7 +199,7 @@ public void should_not_append_slash_to_upstream_url_path() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -244,7 +244,7 @@ public void should_return_route_if_upstream_path_and_upstream_template_are_the_s .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -296,7 +296,7 @@ public void should_return_correct_route_for_http_verb() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) .When(x => x.WhenICallTheFinder()) @@ -334,7 +334,7 @@ public void should_not_return_route() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(false)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(false))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -368,7 +368,7 @@ public void should_return_correct_route_for_http_verb_setting_multiple_upstream_ .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) .When(x => x.WhenICallTheFinder()) @@ -411,7 +411,7 @@ public void should_return_correct_route_for_http_verb_setting_all_upstream_http_ .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) .When(x => x.WhenICallTheFinder()) @@ -454,7 +454,7 @@ public void should_not_return_route_for_http_verb_not_setting_in_upstream_http_m .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Post")) .When(x => x.WhenICallTheFinder()) @@ -488,7 +488,7 @@ public void should_return_route_when_host_matches() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -533,7 +533,7 @@ public void should_return_route_when_upstreamhost_is_null() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -587,7 +587,7 @@ public void should_not_return_route_when_host_doesnt_match() .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -619,7 +619,7 @@ public void should_not_return_route_when_host_doesnt_match_with_empty_upstream_h .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -651,7 +651,7 @@ public void should_return_route_when_host_does_match_with_empty_upstream_http_me .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -693,7 +693,7 @@ public void should_return_route_when_host_matches_but_null_host_on_same_path_fir .Build(), }, string.Empty, serviceProviderConfig )) - .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true))) .And(x => x.GivenTheHeadersMatcherReturns(true)) .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) .When(x => x.WhenICallTheFinder()) @@ -756,7 +756,7 @@ public void Should_return_route_when_upstream_headers_match() }, string.Empty, serviceProviderConfig); - GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true))); + GivenTheUrlMatcherReturns(new UrlMatch(true)); GivenTheHeadersMatcherReturns(true); GivenTheUpstreamHttpMethodIs("Get"); @@ -820,7 +820,7 @@ public void Should_not_return_route_when_upstream_headers_dont_match() .Build(), }, string.Empty, serviceProviderConfig ); - GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true))); + GivenTheUrlMatcherReturns(new UrlMatch(true)); GivenTheHeadersMatcherReturns(false); GivenTheUpstreamHttpMethodIs("Get"); @@ -889,7 +889,7 @@ private void ThenTheUrlMatcherIsNotCalled() .Verify(x => x.Match(_upstreamUrlPath, _upstreamQuery, _routesConfig[0].UpstreamTemplatePattern), Times.Never); } - private void GivenTheUrlMatcherReturns(Response match) + private void GivenTheUrlMatcherReturns(UrlMatch match) { _match = match; _mockUrlMatcher diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs index 5189b42ec..4428afb65 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs @@ -9,7 +9,7 @@ public class RegExUrlMatcherTests : UnitTest private readonly IUrlPathToUrlTemplateMatcher _urlMatcher; private string _path; private string _downstreamPathTemplate; - private Response _result; + private UrlMatch _result; private string _queryString; private bool _containsQueryString; @@ -273,12 +273,12 @@ private void WhenIMatchThePaths() private void ThenTheResultIsTrue() { - _result.Data.Match.ShouldBeTrue(); + _result.Match.ShouldBeTrue(); } private void ThenTheResultIsFalse() { - _result.Data.Match.ShouldBeFalse(); + _result.Match.ShouldBeFalse(); } private void GivenThereIsAQueryInTemplate() From 0c68cd59403fc763009e6857312188a080a80624 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Wed, 15 Jan 2025 18:11:54 +0300 Subject: [PATCH 3/7] Remove Regex obj in favor of StringBuilder --- .../DownstreamUrlCreatorMiddleware.cs | 36 ++++++++++++++++--- .../Routing/RoutingWithQueryStringTests.cs | 4 +++ .../DownstreamUrlCreatorMiddlewareTests.cs | 4 +++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 428e764d3..2d2a90179 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -26,7 +26,7 @@ public DownstreamUrlCreatorMiddleware( RequestDelegate next, IOcelotLoggerFactory loggerFactory, IDownstreamPathPlaceholderReplacer replacer) - : base(loggerFactory.CreateLogger()) + : base(loggerFactory.CreateLogger()) { _next = next; _replacer = replacer; @@ -83,8 +83,7 @@ public async Task Invoke(HttpContext httpContext) } else { - RemoveQueryStringParametersThatHaveBeenUsedInTemplate(downstreamRequest, placeholders); - + RemoveQueryStringParametersThatHaveBeenUsedInTemplate2(downstreamRequest, placeholders); downstreamRequest.AbsolutePath = dsPath; } } @@ -121,9 +120,13 @@ private static string MergeQueryStringsWithoutDuplicateValues(string queryString private static string MapQueryParameter(KeyValuePair pair) => $"{pair.Key}={pair.Value}"; private static readonly ConcurrentDictionary _regex = new(); - private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamRequest downstreamRequest, List templatePlaceholderNameAndValues) + /// + /// Feature 467: + /// Added support for query string parameters in upstream path template. + /// + private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamRequest downstreamRequest, List templatePlaceholders) { - foreach (var nAndV in templatePlaceholderNameAndValues) + foreach (var nAndV in templatePlaceholders) { var name = nAndV.Name.Trim(OpeningBrace, ClosingBrace); var value = Regex.Escape(nAndV.Value); // to ensure a placeholder value containing special Regex characters from URL query parameters is safely used in a Regex constructor, it's necessary to escape the value @@ -145,6 +148,29 @@ private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(Downst } } + private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate2(DownstreamRequest downstreamRequest, List templatePlaceholders) + { + var builder = new StringBuilder(); + foreach (var nAndV in templatePlaceholders) + { + var name = nAndV.Name.Trim(OpeningBrace, ClosingBrace); + var parameter = $"{name}={nAndV.Value}"; + if (!downstreamRequest.Query.Contains(parameter)) + { + continue; + } + + int questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); + builder.Clear() + .Append(downstreamRequest.Query) + .Replace(parameter, string.Empty) + .Remove(--questionMarkOrAmpersand, 1); + downstreamRequest.Query = builder.Length > 0 + ? builder.Remove(0, 1).Insert(0, QuestionMark).ToString() + : string.Empty; + } + } + private static string GetPath(string downstreamPath) { int length = downstreamPath.IndexOf(QuestionMark, StringComparison.Ordinal); diff --git a/test/Ocelot.AcceptanceTests/Routing/RoutingWithQueryStringTests.cs b/test/Ocelot.AcceptanceTests/Routing/RoutingWithQueryStringTests.cs index ed0f8d424..ea89d52d8 100644 --- a/test/Ocelot.AcceptanceTests/Routing/RoutingWithQueryStringTests.cs +++ b/test/Ocelot.AcceptanceTests/Routing/RoutingWithQueryStringTests.cs @@ -161,6 +161,7 @@ public void Should_return_response_200_with_odata_query_string() } [Fact] + [Trait("Feat", "467")] public void Should_return_response_200_with_query_string_upstream_template() { var subscriptionId = Guid.NewGuid().ToString(); @@ -181,6 +182,7 @@ public void Should_return_response_200_with_query_string_upstream_template() } [Fact] + [Trait("Feat", "467")] public void Should_return_response_404_with_query_string_upstream_template_no_query_string() { var subscriptionId = Guid.NewGuid().ToString(); @@ -200,6 +202,7 @@ public void Should_return_response_404_with_query_string_upstream_template_no_qu } [Fact] + [Trait("Feat", "467")] public void Should_return_response_404_with_query_string_upstream_template_different_query_string() { var subscriptionId = Guid.NewGuid().ToString(); @@ -219,6 +222,7 @@ public void Should_return_response_404_with_query_string_upstream_template_diffe } [Fact] + [Trait("Feat", "467")] public void Should_return_response_200_with_query_string_upstream_template_multiple_params() { var subscriptionId = Guid.NewGuid().ToString(); diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs index 12e345f13..81da9bc91 100644 --- a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs @@ -72,6 +72,7 @@ public async Task Should_replace_scheme_and_path() } [Fact] + [Trait("Feat", "467")] public async Task Should_replace_query_string() { // Arrange @@ -106,6 +107,7 @@ public async Task Should_replace_query_string() } [Fact] + [Trait("Feat", "467")] public async Task Should_replace_query_string_but_leave_non_placeholder_queries() { // Arrange @@ -140,6 +142,7 @@ public async Task Should_replace_query_string_but_leave_non_placeholder_queries( } [Fact] + [Trait("Bug", "1288")] public async Task Should_replace_query_string_but_leave_non_placeholder_queries_2() { // Arrange @@ -174,6 +177,7 @@ public async Task Should_replace_query_string_but_leave_non_placeholder_queries_ } [Fact] + [Trait("Feat", "467")] public async Task Should_replace_query_string_exact_match() { // Arrange From 48595237a17ecd00db8eafde830aba2d8282b10d Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 16 Jan 2025 14:35:47 +0300 Subject: [PATCH 4/7] Apply cache in rapid creation of UpstreamPathTemplate objects. With 1min TTL by default --- .../OcelotCacheManagerCache.cs | 5 ++ src/Ocelot/Cache/DefaultMemoryCache.cs | 24 +++++---- src/Ocelot/Cache/IOcelotCache.cs | 2 + .../Creator/UpstreamTemplatePatternCreator.cs | 50 ++++++++++++++----- src/Ocelot/DependencyInjection/Features.cs | 1 + src/Ocelot/Values/UpstreamPathTemplate.cs | 15 +++--- .../ConsulConfigurationInConsulTests.cs | 1 + .../UpstreamTemplatePatternCreatorTests.cs | 6 ++- .../UrlMatcher/RegExUrlMatcherTests.cs | 8 ++- 9 files changed, 78 insertions(+), 34 deletions(-) diff --git a/src/Ocelot.Cache.CacheManager/OcelotCacheManagerCache.cs b/src/Ocelot.Cache.CacheManager/OcelotCacheManagerCache.cs index f1345e27e..7ce4b3d62 100644 --- a/src/Ocelot.Cache.CacheManager/OcelotCacheManagerCache.cs +++ b/src/Ocelot.Cache.CacheManager/OcelotCacheManagerCache.cs @@ -37,4 +37,9 @@ public void ClearRegion(string region) { _cacheManager.ClearRegion(region); } + + public bool TryGetValue(string key, string region, out T value) + { + return _cacheManager.TryGetOrAdd(key, region, (a, b) => default, out value); + } } diff --git a/src/Ocelot/Cache/DefaultMemoryCache.cs b/src/Ocelot/Cache/DefaultMemoryCache.cs index 7ecaaad8f..272e87d77 100644 --- a/src/Ocelot/Cache/DefaultMemoryCache.cs +++ b/src/Ocelot/Cache/DefaultMemoryCache.cs @@ -5,12 +5,12 @@ namespace Ocelot.Cache; public class DefaultMemoryCache : IOcelotCache { private readonly IMemoryCache _memoryCache; - private readonly Dictionary> _regions; + private readonly ConcurrentDictionary> _regions; public DefaultMemoryCache(IMemoryCache memoryCache) { _memoryCache = memoryCache; - _regions = new Dictionary>(); + _regions = new(); } public void Add(string key, T value, TimeSpan ttl, string region) @@ -21,29 +21,29 @@ public void Add(string key, T value, TimeSpan ttl, string region) } _memoryCache.Set(key, value, ttl); - SetRegion(region, key); } public T Get(string key, string region) { - if (_memoryCache.TryGetValue(key, out T value)) + if (TryGetValue(key, region, out T value)) { return value; } - return default(T); + return default; } public void ClearRegion(string region) { - if (_regions.ContainsKey(region)) + if (_regions.TryGetValue(region, out var keys)) { - var keys = _regions[region]; foreach (var key in keys) { _memoryCache.Remove(key); } + + keys.Clear(); } } @@ -59,9 +59,8 @@ public void AddAndDelete(string key, T value, TimeSpan ttl, string region) private void SetRegion(string region, string key) { - if (_regions.ContainsKey(region)) + if (_regions.TryGetValue(region, out var current)) { - var current = _regions[region]; if (!current.Contains(key)) { current.Add(key); @@ -69,7 +68,12 @@ private void SetRegion(string region, string key) } else { - _regions.Add(region, new List { key }); + _regions.TryAdd(region, new() { key }); } } + + public bool TryGetValue(string key, string region, out T value) + { + return _memoryCache.TryGetValue(key, out value); + } } diff --git a/src/Ocelot/Cache/IOcelotCache.cs b/src/Ocelot/Cache/IOcelotCache.cs index 7185d681f..63126756c 100644 --- a/src/Ocelot/Cache/IOcelotCache.cs +++ b/src/Ocelot/Cache/IOcelotCache.cs @@ -9,4 +9,6 @@ public interface IOcelotCache void ClearRegion(string region); void AddAndDelete(string key, T value, TimeSpan ttl, string region); + + bool TryGetValue(string key, string region, out T value); } diff --git a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs index abf9c957b..b414a79c6 100644 --- a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs +++ b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs @@ -1,4 +1,6 @@ +using Ocelot.Cache; using Ocelot.Configuration.File; +using Ocelot.Infrastructure; using Ocelot.Values; namespace Ocelot.Configuration.Creator; @@ -11,11 +13,16 @@ public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator private const string RegExIgnoreCase = "(?i)"; private const string RegExForwardSlashOnly = "^/$"; private const string RegExForwardSlashAndOnePlaceHolder = "^/.*"; + private readonly IOcelotCache _cache; + + public UpstreamTemplatePatternCreator(IOcelotCache cache) + { + _cache = cache; + } public UpstreamPathTemplate Create(IRoute route) { var upstreamTemplate = route.UpstreamPathTemplate; - var placeholders = new List(); for (var i = 0; i < upstreamTemplate.Length; i++) @@ -27,10 +34,10 @@ public UpstreamPathTemplate Create(IRoute route) var placeHolderName = upstreamTemplate.Substring(i, difference); placeholders.Add(placeHolderName); - //hack to handle /{url} case + // Hack to handle /{url} case if (ForwardSlashAndOnePlaceHolder(upstreamTemplate, placeholders, postitionOfPlaceHolderClosingBracket)) { - return new UpstreamPathTemplate(RegExForwardSlashAndOnePlaceHolder, 0, false, route.UpstreamPathTemplate); + return CreateTemplate(RegExForwardSlashAndOnePlaceHolder, 0, false, route.UpstreamPathTemplate); } } } @@ -59,7 +66,7 @@ public UpstreamPathTemplate Create(IRoute route) if (upstreamTemplate == "/") { - return new UpstreamPathTemplate(RegExForwardSlashOnly, route.Priority, containsQueryString, route.UpstreamPathTemplate); + return CreateTemplate(RegExForwardSlashOnly, route.Priority, containsQueryString, route.UpstreamPathTemplate); } var index = upstreamTemplate.LastIndexOf('/'); // index of last forward slash @@ -77,21 +84,40 @@ public UpstreamPathTemplate Create(IRoute route) ? $"^{upstreamTemplate}{RegExMatchEndString}" : $"^{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}"; - return new UpstreamPathTemplate(template, route.Priority, containsQueryString, route.UpstreamPathTemplate); + return CreateTemplate(template, route.Priority, containsQueryString, route.UpstreamPathTemplate); } - private static bool ForwardSlashAndOnePlaceHolder(string upstreamTemplate, List placeholders, int postitionOfPlaceHolderClosingBracket) + /// Time-to-live for caching to initialize the property. + /// A constant structure, default absolute value is 1 minute. + public static TimeSpan RegexCachingTTL { get; } = TimeSpan.FromMinutes(1.0D); + + protected Regex GetRegex(string key) { - if (upstreamTemplate.Substring(0, 2) == "/{" && placeholders.Count == 1 && upstreamTemplate.Length == postitionOfPlaceHolderClosingBracket + 1) + if (string.IsNullOrEmpty(key)) { - return true; + return null; } - return false; + if (!_cache.TryGetValue(key, nameof(UpstreamPathTemplate), out var rgx)) + { + rgx = RegexGlobal.New(key, RegexOptions.Singleline); + _cache.Add(key, rgx, RegexCachingTTL, nameof(UpstreamPathTemplate)); + } + + return rgx; } + protected UpstreamPathTemplate CreateTemplate(string template, int priority, bool containsQueryString, string originalValue) + => new(template, priority, containsQueryString, originalValue) + { + Pattern = GetRegex(template), + }; + + private static bool ForwardSlashAndOnePlaceHolder(string upstreamTemplate, List placeholders, int postitionOfPlaceHolderClosingBracket) + => upstreamTemplate.Substring(0, 2) == "/{" && + placeholders.Count == 1 && + upstreamTemplate.Length == postitionOfPlaceHolderClosingBracket + 1; + private static bool IsPlaceHolder(string upstreamTemplate, int i) - { - return upstreamTemplate[i] == '{'; - } + => upstreamTemplate[i] == '{'; } diff --git a/src/Ocelot/DependencyInjection/Features.cs b/src/Ocelot/DependencyInjection/Features.cs index 1ea558932..93c225c71 100644 --- a/src/Ocelot/DependencyInjection/Features.cs +++ b/src/Ocelot/DependencyInjection/Features.cs @@ -30,6 +30,7 @@ public static IServiceCollection AddRateLimiting(this IServiceCollection service /// The services collection to add the feature to. /// The same object. public static IServiceCollection AddOcelotCache(this IServiceCollection services) => services + .AddSingleton, DefaultMemoryCache>() .AddSingleton, DefaultMemoryCache>() .AddSingleton, DefaultMemoryCache>() .AddSingleton() diff --git a/src/Ocelot/Values/UpstreamPathTemplate.cs b/src/Ocelot/Values/UpstreamPathTemplate.cs index e518d42e6..43713f805 100644 --- a/src/Ocelot/Values/UpstreamPathTemplate.cs +++ b/src/Ocelot/Values/UpstreamPathTemplate.cs @@ -12,7 +12,6 @@ public partial class UpstreamPathTemplate private static readonly Regex _regexNoTemplate = RegexGlobal.New("$^", RegexOptions.Singleline); private static Regex RegexNoTemplate() => _regexNoTemplate; #endif - private static readonly ConcurrentDictionary _regex = new(); public UpstreamPathTemplate(string template, int priority, bool containsQueryString, string originalValue) { @@ -20,19 +19,17 @@ public UpstreamPathTemplate(string template, int priority, bool containsQueryStr Priority = priority; ContainsQueryString = containsQueryString; OriginalValue = originalValue; - Pattern = template == null ? RegexNoTemplate() : - _regex.AddOrUpdate(template, - RegexGlobal.New(template, RegexOptions.Singleline), - (key, oldValue) => oldValue); } public string Template { get; } - public int Priority { get; } - public bool ContainsQueryString { get; } - public string OriginalValue { get; } - public Regex Pattern { get; } + private Regex _pattern; + public Regex Pattern + { + get => _pattern; + set => _pattern = Template == null || value == null ? RegexNoTemplate() : value; + } } diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs index bfe7d345b..16fae1eb8 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs @@ -482,5 +482,6 @@ private class FakeCache : IOcelotCache public FileConfiguration Get(string key, string region) => throw new NotImplementedException(); public void ClearRegion(string region) => throw new NotImplementedException(); public void AddAndDelete(string key, FileConfiguration value, TimeSpan ttl, string region) => throw new NotImplementedException(); + public bool TryGetValue(string key, string region, out FileConfiguration value) => throw new NotImplementedException(); } } diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs index 576c36ebf..4f6c5829d 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs @@ -1,19 +1,23 @@ +using Ocelot.Cache; using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Values; +using System.Text.RegularExpressions; namespace Ocelot.UnitTests.Configuration; public class UpstreamTemplatePatternCreatorTests : UnitTest { private FileRoute _fileRoute; + private readonly Mock> _cache; private readonly UpstreamTemplatePatternCreator _creator; private UpstreamPathTemplate _result; private const string MatchEverything = UpstreamTemplatePatternCreator.RegExMatchZeroOrMoreOfEverything; public UpstreamTemplatePatternCreatorTests() { - _creator = new UpstreamTemplatePatternCreator(); + _cache = new(); + _creator = new UpstreamTemplatePatternCreator(_cache.Object); } [Fact] diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs index 4428afb65..c1cb155a3 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/RegExUrlMatcherTests.cs @@ -1,6 +1,6 @@ using Ocelot.DownstreamRouteFinder.UrlMatcher; -using Ocelot.Responses; using Ocelot.Values; +using System.Text.RegularExpressions; namespace Ocelot.UnitTests.DownstreamRouteFinder.UrlMatcher; @@ -268,7 +268,11 @@ private void GivenIHaveAnUpstreamUrlTemplatePattern(string downstreamUrlTemplate private void WhenIMatchThePaths() { - _result = _urlMatcher.Match(_path, _queryString, new UpstreamPathTemplate(_downstreamPathTemplate, 0, _containsQueryString, _downstreamPathTemplate)); + var upt = new UpstreamPathTemplate(_downstreamPathTemplate, 0, _containsQueryString, _downstreamPathTemplate) + { + Pattern = new Regex(_downstreamPathTemplate), + }; + _result = _urlMatcher.Match(_path, _queryString, upt); } private void ThenTheResultIsTrue() From 34e31f18aebc173b435fc4e3b6e546d2fd021f27 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 16 Jan 2025 17:50:56 +0300 Subject: [PATCH 5/7] #2246 Final version of load test --- .../Creator/UpstreamTemplatePatternCreator.cs | 2 +- .../DownstreamUrlCreatorMiddleware.cs | 27 +------ test/Ocelot.AcceptanceTests/Core/LoadTests.cs | 71 ++++++++++--------- test/Ocelot.AcceptanceTests/Steps.cs | 10 +++ 4 files changed, 50 insertions(+), 60 deletions(-) diff --git a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs index b414a79c6..04df41bd4 100644 --- a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs +++ b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs @@ -89,7 +89,7 @@ public UpstreamPathTemplate Create(IRoute route) /// Time-to-live for caching to initialize the property. /// A constant structure, default absolute value is 1 minute. - public static TimeSpan RegexCachingTTL { get; } = TimeSpan.FromMinutes(1.0D); + public static TimeSpan RegexCachingTTL { get; set; } = TimeSpan.FromMinutes(1.0D); protected Regex GetRegex(string key) { diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 2d2a90179..be58add3c 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -1,7 +1,6 @@ using Microsoft.AspNetCore.Http; using Ocelot.Configuration; using Ocelot.DownstreamRouteFinder.UrlMatcher; -using Ocelot.Infrastructure; using Ocelot.Logging; using Ocelot.Middleware; using Ocelot.Request.Middleware; @@ -83,7 +82,7 @@ public async Task Invoke(HttpContext httpContext) } else { - RemoveQueryStringParametersThatHaveBeenUsedInTemplate2(downstreamRequest, placeholders); + RemoveQueryStringParametersThatHaveBeenUsedInTemplate(downstreamRequest, placeholders); downstreamRequest.AbsolutePath = dsPath; } } @@ -125,30 +124,6 @@ private static string MergeQueryStringsWithoutDuplicateValues(string queryString /// Added support for query string parameters in upstream path template. /// private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamRequest downstreamRequest, List templatePlaceholders) - { - foreach (var nAndV in templatePlaceholders) - { - var name = nAndV.Name.Trim(OpeningBrace, ClosingBrace); - var value = Regex.Escape(nAndV.Value); // to ensure a placeholder value containing special Regex characters from URL query parameters is safely used in a Regex constructor, it's necessary to escape the value - var pattern = $@"\b{name}={value}\b"; - var rgx = _regex.AddOrUpdate(pattern, - RegexGlobal.New(pattern), - (key, oldValue) => oldValue); - if (rgx.IsMatch(downstreamRequest.Query)) - { - var questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); - downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty); - downstreamRequest.Query = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1); - - if (!string.IsNullOrEmpty(downstreamRequest.Query)) - { - downstreamRequest.Query = QuestionMark + downstreamRequest.Query[1..]; - } - } - } - } - - private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate2(DownstreamRequest downstreamRequest, List templatePlaceholders) { var builder = new StringBuilder(); foreach (var nAndV in templatePlaceholders) diff --git a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs index 5f1d42079..4c5bae283 100644 --- a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs +++ b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs @@ -5,6 +5,10 @@ namespace Ocelot.AcceptanceTests.Core; +/// +/// TODO Move to separate Performance Testing (load testing) project. +/// It requires the class; therefore, both Steps-classes must be moved to the common Testing project. +/// public sealed class LoadTests : ConcurrentSteps, IDisposable { private readonly ServiceHandler _serviceHandler; @@ -25,9 +29,8 @@ public override void Dispose() [Fact] [Trait("Feat", "1348")] [Trait("Bug", "2246")] - public async Task ShouldLoadRegexCachingHeavily_NoOutOfMemoryExceptions_NoMemoryLeaks() + public async Task ShouldLoadRegexCachingHeavily_NoMemoryLeaks() { - var limit = Environment.GetEnvironmentVariable("DOTNET_GCHeapHardLimit"); var port = PortFinder.GetRandomPort(); var route = GivenRoute(port, "/my-gateway/order/{orderNumber}", "/order/{orderNumber}"); var configuration = GivenConfiguration(route); @@ -35,56 +38,58 @@ public async Task ShouldLoadRegexCachingHeavily_NoOutOfMemoryExceptions_NoMemory GivenOcelotIsRunning(); GivenThereIsAServiceRunningOn(port, "/order/", "Hello from Donny"); - var currentProcess = Process.GetCurrentProcess(); - long memoryUsage = currentProcess.WorkingSet64; - float memoryUsageMB = (float)memoryUsage / (1024 * 1024); - // Step 1: Measure memory consumption for constant upstream URL GC.Collect(); var a = GC.GetGCMemoryInfo(); - long step1TotalMemory = GC.GetTotalMemory(true); - long step1TotalBytes = GC.GetTotalAllocatedBytes(); - long step1ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); - - //.When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 50)) - //.When(x => RunParallelRequests(100_000, (i) => "/my-gateway/order/" + i)) - await WhenIDoActionMultipleTimes(10_000, (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/1")); // const url + var totalMemory = ToMegabytes(GC.GetTotalMemory(true)); + var currentProcess = Process.GetCurrentProcess(); + var memoryUsage = ToMegabytes(currentProcess.WorkingSet64); + // Perform 50% of job for stable indicators + await WhenIDoActionMultipleTimes(5_000, (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/1")); // const url GC.Collect(); - step1TotalMemory = GC.GetTotalMemory(true); - step1TotalBytes = GC.GetTotalAllocatedBytes(); - step1ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); - long memoryUsage1 = currentProcess.WorkingSet64; - float memoryUsageMB1 = (float)memoryUsage / (1024 * 1024); + var totalMemory0 = ToMegabytes(GC.GetTotalMemory(true)); + var process0 = Process.GetCurrentProcess(); + var memoryUsage0 = ToMegabytes(process0.WorkingSet64); + + await WhenIDoActionMultipleTimes(5_000, (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/1")); // const url await WhenIGetUrlOnTheApiGateway("/my-gateway/order/1"); ThenTheStatusCodeShouldBe(HttpStatusCode.OK); ThenTheResponseBodyShouldBe("Hello from Donny"); - // Step 2: Measure memory consumption for varying upstream URL GC.Collect(); - long step2TotalMemory = GC.GetTotalMemory(true); - long step2TotalBytes = GC.GetTotalAllocatedBytes(); - long step2ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); + var totalMemory1 = ToMegabytes(GC.GetTotalMemory(true)); + var process1 = Process.GetCurrentProcess(); + var memoryUsage1 = ToMegabytes(process1.WorkingSet64); + // Step 2: Measure memory consumption for varying upstream URL + // await WhenIDoActionForTime(TimeSpan.FromSeconds(30), (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/" + i)); // varying url await WhenIDoActionMultipleTimes(10_000, (i) => WhenIGetUrlOnTheApiGateway("/my-gateway/order/" + i)); // varying url GC.Collect(); - step2TotalMemory = GC.GetTotalMemory(true); - step2TotalBytes = GC.GetTotalAllocatedBytes(); - step2ThreadBytes = GC.GetAllocatedBytesForCurrentThread(); - long memoryUsage2 = currentProcess.WorkingSet64; - float memoryUsageMB2 = (float)memoryUsage / (1024 * 1024); + var totalMemory2 = ToMegabytes(GC.GetTotalMemory(true)); + var process2 = Process.GetCurrentProcess(); + var memoryUsage2 = ToMegabytes(process2.WorkingSet64); + + // Assert + AssertDelta(totalMemory0, totalMemory1, totalMemory2); + AssertDelta(memoryUsage0, memoryUsage1, memoryUsage2); } - private async Task WhenIGetUrlOnTheApiGatewaySequentially(int i) + private static float ToMegabytes(long total) => (float)total / (1024 * 1024); + + private static void AssertDelta(float value0, float value1, float value2) { - //int count = i + 1; - await WhenIGetUrlOnTheApiGateway("/my-gateway/order/" + i); + if (value1 <= value0) + return; + + var delta = value1 - value0; + if (value2 <= value1) + return; - //ThenTheStatusCodeShouldBe(HttpStatusCode.OK); - //ThenServiceShouldHaveBeenCalledTimes(0, count); - //ThenTheResponseBodyShouldBe($"{count}:{Bug2119ServiceNames[0]}", $"i is {i}"); + var delta2 = value2 - value1; + Assert.True(delta2 <= delta); // delta is not growing } private FileRoute GivenRoute(int port, string upstream, string downstream) => new() diff --git a/test/Ocelot.AcceptanceTests/Steps.cs b/test/Ocelot.AcceptanceTests/Steps.cs index 9c27b9aea..bd8d967bb 100644 --- a/test/Ocelot.AcceptanceTests/Steps.cs +++ b/test/Ocelot.AcceptanceTests/Steps.cs @@ -24,6 +24,7 @@ using Ocelot.Tracing.OpenTracing; using Serilog; using Serilog.Core; +using System.Diagnostics; using System.IO.Compression; using System.Net.Http.Headers; using System.Text; @@ -732,6 +733,15 @@ public static async Task WhenIDoActionMultipleTimes(int times, Func a for (int i = 0; i < times; i++) await action.Invoke(i); } + public static async Task WhenIDoActionForTime(TimeSpan time, Func action) + { + var watcher = Stopwatch.StartNew(); + for (int i = 0; watcher.Elapsed < time; i++) + { + await action.Invoke(i); + } + watcher.Stop(); + } public async Task WhenIGetUrlOnTheApiGateway(string url, string requestId) { From e380ce4e8bf6583aa976d25c4e44e503a3f1e433 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 16 Jan 2025 18:11:03 +0300 Subject: [PATCH 6/7] #2246 Make the test sequential --- test/Ocelot.AcceptanceTests/Core/LoadTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs index 4c5bae283..725655b3f 100644 --- a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs +++ b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs @@ -9,6 +9,7 @@ namespace Ocelot.AcceptanceTests.Core; /// TODO Move to separate Performance Testing (load testing) project. /// It requires the class; therefore, both Steps-classes must be moved to the common Testing project. /// +[Collection(nameof(SequentialTests))] public sealed class LoadTests : ConcurrentSteps, IDisposable { private readonly ServiceHandler _serviceHandler; From 73c2124d424c895d502a61c4372f0227906bc333 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 16 Jan 2025 19:43:40 +0300 Subject: [PATCH 7/7] Skip load test, add unit tests to improve coverage --- .../DownstreamUrlCreatorMiddleware.cs | 1 - test/Ocelot.AcceptanceTests/Core/LoadTests.cs | 2 +- .../UpstreamTemplatePatternCreatorTests.cs | 31 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index be58add3c..27a586f49 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -117,7 +117,6 @@ private static string MergeQueryStringsWithoutDuplicateValues(string queryString } private static string MapQueryParameter(KeyValuePair pair) => $"{pair.Key}={pair.Value}"; - private static readonly ConcurrentDictionary _regex = new(); /// /// Feature 467: diff --git a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs index 725655b3f..ad63f3005 100644 --- a/test/Ocelot.AcceptanceTests/Core/LoadTests.cs +++ b/test/Ocelot.AcceptanceTests/Core/LoadTests.cs @@ -27,7 +27,7 @@ public override void Dispose() base.Dispose(); } - [Fact] + [Fact(Skip = "It should be moved to a separate project. It should be run during release only as an extra check for quality gates.")] [Trait("Feat", "1348")] [Trait("Bug", "2246")] public async Task ShouldLoadRegexCachingHeavily_NoMemoryLeaks() diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs index 4f6c5829d..431bbb447 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs @@ -2,6 +2,7 @@ using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Values; +using System.Reflection; using System.Text.RegularExpressions; namespace Ocelot.UnitTests.Configuration; @@ -239,6 +240,36 @@ public void should_create_template_pattern_that_matches_query_string_with_multip .BDDfy(); } + [Fact] + [Trait("Feat", "1348")] + [Trait("Bug", "2246")] + public void GetRegex_NoKey_ReturnsNull() + { + // Act + var actual = GetRegex.Invoke(_creator, new object[] { string.Empty }); + + // Assert + actual.ShouldBeNull(); + } + + [Fact] + [Trait("Feat", "1348")] + [Trait("Bug", "2246")] + public void CreateTemplate_PatternProperty_NullChecks() + { + // Act + string nullTemplate = null; + var actual = CreateTemplate.Invoke(_creator, new object[] { nullTemplate, 0, false, null }) as UpstreamPathTemplate; + + // Assert + actual.ShouldNotBeNull(); + actual.Pattern.ShouldNotBeNull().ToString().ShouldBe("$^"); + } + + private static Type Me { get; } = typeof(UpstreamTemplatePatternCreator); + private static MethodInfo GetRegex { get; } = Me.GetMethod(nameof(GetRegex), BindingFlags.NonPublic | BindingFlags.Instance); + private static MethodInfo CreateTemplate { get; } = Me.GetMethod(nameof(CreateTemplate), BindingFlags.NonPublic | BindingFlags.Instance); + private void GivenTheFollowingFileRoute(FileRoute fileRoute) { _fileRoute = fileRoute;