diff --git a/src/Elasticsearch.Net/ConnectionPool/SingleNodeConnectionPool.cs b/src/Elasticsearch.Net/ConnectionPool/SingleNodeConnectionPool.cs index c79552263bf..5070d5015ed 100644 --- a/src/Elasticsearch.Net/ConnectionPool/SingleNodeConnectionPool.cs +++ b/src/Elasticsearch.Net/ConnectionPool/SingleNodeConnectionPool.cs @@ -27,6 +27,9 @@ public SingleNodeConnectionPool(Uri uri, IDateTimeProvider dateTimeProvider = nu /// public IReadOnlyCollection Nodes { get; } + /// + public ProductCheckStatus ProductCheckStatus { get; set; } = ProductCheckStatus.NotChecked; + /// public bool SniffedOnStartup { @@ -34,9 +37,6 @@ public bool SniffedOnStartup set { } } - /// - public ProductCheckStatus ProductCheckStatus { get; set; } - /// public bool SupportsPinging => false; diff --git a/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs b/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs index d6ded72722d..9b1c423a0f2 100644 --- a/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs +++ b/src/Elasticsearch.Net/ConnectionPool/StaticConnectionPool.cs @@ -19,7 +19,7 @@ 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, randomize, randomizeSeed: null, dateTimeProvider) { } + : this(nodes, randomize, null, dateTimeProvider) { } protected StaticConnectionPool(IEnumerable nodes, bool randomize, int? randomizeSeed = null, IDateTimeProvider dateTimeProvider = null) { @@ -39,30 +39,6 @@ protected StaticConnectionPool(IEnumerable nodes, Func nodeSc Initialize(nodes, dateTimeProvider); } - private void Initialize(IEnumerable nodes, IDateTimeProvider dateTimeProvider) - { - var nodesProvided = nodes?.ToList() ?? throw new ArgumentNullException(nameof(nodes)); - nodesProvided.ThrowIfEmpty(nameof(nodes)); - DateTimeProvider = dateTimeProvider ?? Net.DateTimeProvider.Default; - - string scheme = null; - foreach (var node in nodesProvided) - { - 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"); - } - - InternalNodes = SortNodes(nodesProvided) - .DistinctBy(n => n.Uri) - .ToList(); - LastUpdate = DateTimeProvider.Now(); - } - /// public DateTime LastUpdate { get; protected set; } @@ -73,10 +49,10 @@ private void Initialize(IEnumerable nodes, IDateTimeProvider dateTimeProvi public virtual IReadOnlyCollection Nodes => InternalNodes; /// - public bool SniffedOnStartup { get; set; } + public ProductCheckStatus ProductCheckStatus { get; set; } = ProductCheckStatus.NotChecked; /// - public ProductCheckStatus ProductCheckStatus { get; set; } + public bool SniffedOnStartup { get; set; } /// public virtual bool SupportsPinging => true; @@ -133,6 +109,28 @@ public virtual void Reseed(IEnumerable nodes) { } //ignored void IDisposable.Dispose() => DisposeManagedResources(); + private void Initialize(IEnumerable nodes, IDateTimeProvider dateTimeProvider) + { + var nodesProvided = nodes?.ToList() ?? throw new ArgumentNullException(nameof(nodes)); + nodesProvided.ThrowIfEmpty(nameof(nodes)); + DateTimeProvider = dateTimeProvider ?? Net.DateTimeProvider.Default; + + string scheme = null; + foreach (var node in nodesProvided) + 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"); + + InternalNodes = SortNodes(nodesProvided) + .DistinctBy(n => n.Uri) + .ToList(); + LastUpdate = DateTimeProvider.Now(); + } + protected virtual Node RetryInternalNodes(int globalCursor, Action audit = null) { audit?.Invoke(AuditEvent.AllNodesDead, null); diff --git a/src/Elasticsearch.Net/Transport/Pipeline/RequestPipeline.cs b/src/Elasticsearch.Net/Transport/Pipeline/RequestPipeline.cs index 6ad35894507..f1aad69a891 100644 --- a/src/Elasticsearch.Net/Transport/Pipeline/RequestPipeline.cs +++ b/src/Elasticsearch.Net/Transport/Pipeline/RequestPipeline.cs @@ -682,13 +682,19 @@ private bool ApplyProductCheckRules(RootResponse response) return true; } - if (response.HttpStatusCode.HasValue && (response.HttpStatusCode.Value == 401 || response.HttpStatusCode.Value == 403)) + // We should always have a status code! + var statusCode = response.HttpStatusCode ?? 0; + + // The call to the root path requires monitor permissions. If the current use lacks those, we cannot perform product validation. + if (statusCode is 401 or 403) { - // The call to the root path requires monitor permissions. If the current use lacks those, we cannot perform product validation. _connectionPool.ProductCheckStatus = ProductCheckStatus.UndeterminedProduct; return true; } + // Any response besides a 200, 401 or 403 is considered a failure and the check is considered incomplete remaining in the NotChecked state. + // This means that the check will run on subsequent requests until we have a valid response to evaluate. + // By returning false, the failure to perform the product check will be included in the audit log. if (!response.Success) return false; // Start by assuming the product is valid @@ -715,6 +721,8 @@ private bool ApplyProductCheckRules(RootResponse response) _connectionPool.ProductCheckStatus = ProductCheckStatus.InvalidProduct; } + return true; + // Elasticsearch should be version 6.0.0 or greater // Note: For best compatibility, the client should not be used with versions prior to 7.0.0, but we do not enforce that here static bool VersionTooLow(Version version) @@ -741,8 +749,6 @@ static bool Version714InvalidHeader(Version version, string productName) { return version >= Version714 && !ExpectedProductName.Equals(productName, StringComparison.Ordinal); } - - return true; } private bool PingDisabled(Node node) => diff --git a/tests/Tests/ClientConcepts/ConnectionPooling/ProductChecking/ProductCheckAtStartup.doc.cs b/tests/Tests/ClientConcepts/ConnectionPooling/ProductChecking/ProductCheckAtStartup.doc.cs index 62dfed42283..940376c5960 100644 --- a/tests/Tests/ClientConcepts/ConnectionPooling/ProductChecking/ProductCheckAtStartup.doc.cs +++ b/tests/Tests/ClientConcepts/ConnectionPooling/ProductChecking/ProductCheckAtStartup.doc.cs @@ -2,6 +2,7 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information +using System; using System.Threading.Tasks; using Elastic.Elasticsearch.Xunit.XunitPlumbing; using Elasticsearch.Net.VirtualizedCluster; @@ -50,7 +51,7 @@ public async Task ProductCheckPerformedOnSecondCallWhenFirstCheckFails() /** Here's an example with a single node cluster which fails for some reason during the first product check attempt. */ var audit = new Auditor(() => VirtualClusterWith .Nodes(1, productCheckAlwaysSucceeds: false) - .ProductCheck(r => r.Fails(TimesHelper.Once)) + .ProductCheck(r => r.Fails(TimesHelper.Once, 429)) .ProductCheck(r => r.SucceedAlways()) .ClientCalls(r => r.SucceedAlways()) .StaticConnectionPool() @@ -60,12 +61,12 @@ public async Task ProductCheckPerformedOnSecondCallWhenFirstCheckFails() audit = await audit.TraceCalls(skipProductCheck: false, new ClientCall() { { ProductCheckOnStartup }, - { ProductCheckFailure, 9200 }, // <1> as this is the first call, the product check is executed, but fails + { ProductCheckFailure, 9200 }, // <1> as this is the first call, the product check is executed, but times out { HealthyResponse, 9200 } // <2> the actual request is still sent and succeeds }, new ClientCall() { { ProductCheckOnStartup }, - { ProductCheckSuccess, 9200 }, // <3> as the previous product check failed, it runs again on the second call + { ProductCheckSuccess, 9200 }, // <3> as the previous product check failed, it runs again on the second call and this time it succeeds { HealthyResponse, 9200 } }, new ClientCall() { @@ -77,10 +78,10 @@ public async Task ProductCheckPerformedOnSecondCallWhenFirstCheckFails() [U] public async Task ProductCheckAttemptsAllNodes() { - /** Here's an example with a three node cluster which fails for some reason during the first and second product check attempts. */ + /** Here's an example with a three node cluster which fails (due to too many requests) during the first and second product check attempts. */ var audit = new Auditor(() => VirtualClusterWith .Nodes(3, productCheckAlwaysSucceeds: false) - .ProductCheck(r => r.FailAlways()) + .ProductCheck(r => r.FailAlways(429)) .ProductCheck(r => r.OnPort(9202).SucceedAlways()) .ClientCalls(r => r.SucceedAlways()) .StaticConnectionPool()