From 7245c66e2b49d1ae36ec2a4aacf20558fd95d80c Mon Sep 17 00:00:00 2001 From: Russ Cam Date: Mon, 30 Sep 2019 15:56:54 +1000 Subject: [PATCH] Fix initial randomization of nodes This commit fixes the initial randomization of nodes in a StaticConnectionPool. Instead of calling the protected ctor, the initialization logic has been pulled out into a separate method that is now called from the ctor that accepts a randomize parameter. Fixes #4055 --- .../ConnectionPool/StaticConnectionPool.cs | 36 +++++++++++++------ .../BuildingBlocks/ConnectionPooling.doc.cs | 24 +++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs b/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs index 0b2198c0eba..f260ff65445 100644 --- a/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs +++ b/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs @@ -15,24 +15,38 @@ public StaticConnectionPool(IEnumerable uris, bool randomize = true, IDateT : this(uris.Select(uri => new Node(uri)), randomize, dateTimeProvider) { } public StaticConnectionPool(IEnumerable nodes, bool randomize = true, IDateTimeProvider dateTimeProvider = null) - : this(nodes, null, dateTimeProvider) => Randomize = randomize; + { + nodes.ThrowIfEmpty(nameof(nodes)); + Randomize = randomize; + Initialize(nodes, dateTimeProvider); + } //this constructor is protected because nodeScorer only makes sense on subclasses that support reseeding //otherwise just manually sort `nodes` before instantiating. protected StaticConnectionPool(IEnumerable nodes, Func nodeScorer, IDateTimeProvider dateTimeProvider = null) { nodes.ThrowIfEmpty(nameof(nodes)); - DateTimeProvider = dateTimeProvider ?? Net.DateTimeProvider.Default; + _nodeScorer = nodeScorer; + Initialize(nodes, dateTimeProvider); + } - var nn = nodes.ToList(); - var uris = nn.Select(n => n.Uri).ToList(); - if (uris.Select(u => u.Scheme).Distinct().Count() > 1) - throw new ArgumentException("Trying to instantiate a connection pool with mixed URI Schemes"); + private void Initialize(IEnumerable nodes, IDateTimeProvider dateTimeProvider) + { + DateTimeProvider = dateTimeProvider ?? Net.DateTimeProvider.Default; - UsingSsl = uris.Any(uri => uri.Scheme == "https"); + string scheme = null; + foreach (var node in nodes) + { + if (scheme == null) + { + scheme = node.Uri.Scheme; + UsingSsl = scheme == "https"; + } + else if (scheme != node.Uri.Scheme) + throw new ArgumentException("Trying to instantiate a connection pool with mixed URI Schemes"); + } - _nodeScorer = nodeScorer; - InternalNodes = SortNodes(nn) + InternalNodes = SortNodes(nodes) .DistinctBy(n => n.Uri) .ToList(); LastUpdate = DateTimeProvider.Now(); @@ -57,7 +71,7 @@ protected StaticConnectionPool(IEnumerable nodes, Func nodeSc public virtual bool SupportsReseeding => false; /// - public bool UsingSsl { get; } + public bool UsingSsl { get; private set; } protected List AliveNodes { @@ -70,7 +84,7 @@ protected List AliveNodes } } - protected IDateTimeProvider DateTimeProvider { get; } + protected IDateTimeProvider DateTimeProvider { get; private set; } protected List InternalNodes { get; set; } protected Random Random { get; } = new Random(); diff --git a/src/Tests/Tests/ClientConcepts/ConnectionPooling/BuildingBlocks/ConnectionPooling.doc.cs b/src/Tests/Tests/ClientConcepts/ConnectionPooling/BuildingBlocks/ConnectionPooling.doc.cs index ed6a9c654f7..b91f7ec12d0 100644 --- a/src/Tests/Tests/ClientConcepts/ConnectionPooling/BuildingBlocks/ConnectionPooling.doc.cs +++ b/src/Tests/Tests/ClientConcepts/ConnectionPooling/BuildingBlocks/ConnectionPooling.doc.cs @@ -214,6 +214,30 @@ [U] public void Static() } } + // hide + [U] public void RandomizedInitialNodes() + { + StaticConnectionPool CreatStaticConnectionPool() + { + var uris = new[] + { + new Uri("https://10.0.0.1:9200/"), + new Uri("https://10.0.0.2:9200/"), + new Uri("https://10.0.0.3:9200/") + }; + + var staticConnectionPool = new StaticConnectionPool(uris); + return staticConnectionPool; + } + + // assertion works on the probability of seeing a Uri other than https://10.0.0.1:9200/ + // as the first value over 50 runs, when randomized. + Enumerable.Repeat(CreatStaticConnectionPool().CreateView().First().Uri.ToString(), 50) + .All(uri => uri == "https://10.0.0.1:9200/") + .Should() + .BeFalse(); + } + /**[[sniffing-connection-pool]] * ==== SniffingConnectionPool *