From 4e87f5b4473f6b02e5e06e88b7fde58cf3a01335 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Tue, 19 Nov 2024 15:49:12 +0200 Subject: [PATCH 01/23] IGNITE-23703 .NET: Add client pool --- .../dotnet/Apache.Ignite/IgniteClientPool.cs | 115 ++++++++++++++++++ .../IgniteClientPoolConfiguration.cs | 27 ++++ 2 files changed, 142 insertions(+) create mode 100644 modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs create mode 100644 modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs new file mode 100644 index 00000000000..d83602793b8 --- /dev/null +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +namespace Apache.Ignite; + +using System; +using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; +using System.Threading; +using System.Threading.Tasks; +using Internal.Common; + +/// +/// Ignite client pool. Thread safe. +/// +/// This pool creates up to Ignite clients and returns them in a round-robin fashion. +/// Ignite clients are thread safe, so there is no rent/return semantics. +/// +/// +public sealed class IgniteClientPool : IDisposable +{ + private readonly IIgniteClient?[] _clients; + + private readonly SemaphoreSlim _clientsLock = new(1); + + private int _disposed; + + private int _clientCount; + + private int _clientIndex; + + /// + /// Initializes a new instance of the class. + /// + /// Configuration. + public IgniteClientPool(IgniteClientPoolConfiguration configuration) + { + IgniteArgumentCheck.NotNull(configuration); + IgniteArgumentCheck.Ensure(configuration.PoolSize > 0, nameof(configuration.PoolSize), "PoolSize > 0"); + + Configuration = configuration; + _clients = new IIgniteClient[configuration.PoolSize]; + } + + /// + /// Gets the configuration. + /// + public IgniteClientPoolConfiguration Configuration { get; } + + /// + /// Gets an Ignite client from the pool. Creates a new one if necessary. + /// Performs round-robin balancing across pooled instances. + /// + /// Ignite client. + [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] + public async Task GetClientAsync() + { + var index = Interlocked.Increment(ref _clientIndex) % _clients.Length; + + var client = _clients[index]; + if (client != null) + { + return client; + } + + await _clientsLock.WaitAsync().ConfigureAwait(false); + + try + { + client = _clients[index]; + if (client != null) + { + return client; + } + + client = await CreateClientAsync().ConfigureAwait(false); + _clients[index] = client; + } + finally + { + _clientsLock.Release(); + } + } + + /// + public void Dispose() + { + if (Interlocked.CompareExchange(ref _disposed, 1, 0) == 1) + { + return; + } + + while (_clients.TryDequeue(out var client)) + { + client.Dispose(); + } + } + + private async Task CreateClientAsync() => + await IgniteClient.StartAsync(Configuration.ClientConfiguration).ConfigureAwait(false); +} diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs new file mode 100644 index 00000000000..14b2237ef1a --- /dev/null +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs @@ -0,0 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +namespace Apache.Ignite; + +/// +/// Ignite client pool configuration. +/// +/// Client configuration. +/// Pool size. +public sealed record IgniteClientPoolConfiguration( + IgniteClientConfiguration ClientConfiguration, + int PoolSize); From 2fa84023fafb74d1ca6576c829666959d800b6eb Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Tue, 19 Nov 2024 15:57:35 +0200 Subject: [PATCH 02/23] Pool implemented --- .../dotnet/Apache.Ignite/IgniteClientPool.cs | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index d83602793b8..934da3ff09e 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -18,7 +18,6 @@ namespace Apache.Ignite; using System; -using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; @@ -39,8 +38,6 @@ public sealed class IgniteClientPool : IDisposable private int _disposed; - private int _clientCount; - private int _clientIndex; /// @@ -61,6 +58,11 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) /// public IgniteClientPoolConfiguration Configuration { get; } + /// + /// Gets a value indicating whether the pool is disposed. + /// + public bool IsDisposed => Interlocked.CompareExchange(ref _disposed, 0, 0) == 1; + /// /// Gets an Ignite client from the pool. Creates a new one if necessary. /// Performs round-robin balancing across pooled instances. @@ -69,9 +71,11 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] public async Task GetClientAsync() { - var index = Interlocked.Increment(ref _clientIndex) % _clients.Length; + ObjectDisposedException.ThrowIf(IsDisposed, this); - var client = _clients[index]; + int index = Interlocked.Increment(ref _clientIndex) % _clients.Length; + + IIgniteClient? client = _clients[index]; if (client != null) { return client; @@ -89,6 +93,8 @@ public async Task GetClientAsync() client = await CreateClientAsync().ConfigureAwait(false); _clients[index] = client; + + return client; } finally { @@ -104,10 +110,13 @@ public void Dispose() return; } - while (_clients.TryDequeue(out var client)) + foreach (var client in _clients) { - client.Dispose(); + // Dispose is not supposed to throw, so we expect all clients to dispose correctly. + client?.Dispose(); } + + _clientsLock.Dispose(); } private async Task CreateClientAsync() => From 692adf3c0e8478110a990d1fa6011a5838b8927f Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Tue, 19 Nov 2024 16:03:04 +0200 Subject: [PATCH 03/23] wip tests --- .../IgniteClientPoolTests.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs new file mode 100644 index 00000000000..4910e6b54a8 --- /dev/null +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +namespace Apache.Ignite.Tests; + +using System; +using NUnit.Framework; + +/// +/// Tests for . +/// +public class IgniteClientPoolTests +{ + [Test] + public void TestConstructorValidatesArgs() + { + Assert.Throws(() => new IgniteClientPool(null!)); + } +} From c2d377c251bedc6f3c394061b95b9f339a58b957 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:01:04 +0200 Subject: [PATCH 04/23] wip --- modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index 934da3ff09e..c4f37344fe2 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -71,6 +71,7 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] public async Task GetClientAsync() { + // TODO: Race condition when disposing. ObjectDisposedException.ThrowIf(IsDisposed, this); int index = Interlocked.Increment(ref _clientIndex) % _clients.Length; From c25e7dde6b9604d412e2c7b900e9d2db301fb91f Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:05:28 +0200 Subject: [PATCH 05/23] TestUseAfterDispose --- .../Apache.Ignite.Tests/IgniteClientPoolTests.cs | 10 ++++++++++ .../Apache.Ignite/IgniteClientPoolConfiguration.cs | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index 4910e6b54a8..e886aeefec0 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -18,6 +18,7 @@ namespace Apache.Ignite.Tests; using System; +using System.Threading.Tasks; using NUnit.Framework; /// @@ -30,4 +31,13 @@ public void TestConstructorValidatesArgs() { Assert.Throws(() => new IgniteClientPool(null!)); } + + [Test] + public void TestUseAfterDispose() + { + var pool = new IgniteClientPool(new(new(), 1)); + pool.Dispose(); + + Assert.ThrowsAsync(async () => await pool.GetClientAsync()); + } } diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs index 14b2237ef1a..14c1549890e 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs @@ -22,6 +22,6 @@ namespace Apache.Ignite; /// /// Client configuration. /// Pool size. -public sealed record IgniteClientPoolConfiguration( +public sealed record IgniteClientPoolConfiguration( // TODO: immutability inconsistent with IgniteClientConfiguration IgniteClientConfiguration ClientConfiguration, int PoolSize); From 8e1b256a7caa05d9c452d6681b6dbd8f1e2e8b1d Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:08:31 +0200 Subject: [PATCH 06/23] Fix race condition on dispose --- .../dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs | 1 - modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index e886aeefec0..5bb1fb87074 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -18,7 +18,6 @@ namespace Apache.Ignite.Tests; using System; -using System.Threading.Tasks; using NUnit.Framework; /// diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index c4f37344fe2..b18215bf85c 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -71,7 +71,6 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] public async Task GetClientAsync() { - // TODO: Race condition when disposing. ObjectDisposedException.ThrowIf(IsDisposed, this); int index = Interlocked.Increment(ref _clientIndex) % _clients.Length; @@ -111,6 +110,8 @@ public void Dispose() return; } + _clientsLock.Wait(); + foreach (var client in _clients) { // Dispose is not supposed to throw, so we expect all clients to dispose correctly. From 8e321997afb8d7f71245be5b9758d153dacd83df Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:31:19 +0200 Subject: [PATCH 07/23] Add IsDisposed check --- .../IgniteClientPoolTests.cs | 19 +++++++++++++++++++ .../dotnet/Apache.Ignite/IgniteClientPool.cs | 19 ++++++++++++------- .../Internal/ClientFailoverSocket.cs | 5 +++++ .../Internal/IgniteClientInternal.cs | 5 +++++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index 5bb1fb87074..2547b48e6a9 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -18,6 +18,7 @@ namespace Apache.Ignite.Tests; using System; +using System.Threading.Tasks; using NUnit.Framework; /// @@ -25,6 +26,24 @@ namespace Apache.Ignite.Tests; /// public class IgniteClientPoolTests { + private FakeServer _server; + + [OneTimeSetUp] + public void StartServer() => _server = new FakeServer(); + + [OneTimeTearDown] + public void StopServer() => _server.Dispose(); + + [Test] + public async Task TestGetClient() + { + var pool = new IgniteClientPool(new(new(_server.Endpoint), 1)); + IIgniteClient client = await pool.GetClientAsync(); + + Assert.IsNotNull(client); + await client.Tables.GetTablesAsync(); + } + [Test] public void TestConstructorValidatesArgs() { diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index b18215bf85c..ac650a521d1 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -21,6 +21,7 @@ namespace Apache.Ignite; using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; +using Internal; using Internal.Common; /// @@ -32,7 +33,7 @@ namespace Apache.Ignite; /// public sealed class IgniteClientPool : IDisposable { - private readonly IIgniteClient?[] _clients; + private readonly IgniteClientInternal?[] _clients; private readonly SemaphoreSlim _clientsLock = new(1); @@ -50,7 +51,7 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) IgniteArgumentCheck.Ensure(configuration.PoolSize > 0, nameof(configuration.PoolSize), "PoolSize > 0"); Configuration = configuration; - _clients = new IIgniteClient[configuration.PoolSize]; + _clients = new IgniteClientInternal[configuration.PoolSize]; } /// @@ -75,8 +76,8 @@ public async Task GetClientAsync() int index = Interlocked.Increment(ref _clientIndex) % _clients.Length; - IIgniteClient? client = _clients[index]; - if (client != null) + IgniteClientInternal? client = _clients[index]; + if (client is { IsDisposed: false }) { return client; } @@ -86,7 +87,7 @@ public async Task GetClientAsync() try { client = _clients[index]; - if (client != null) + if (client is { IsDisposed: false }) { return client; } @@ -121,6 +122,10 @@ public void Dispose() _clientsLock.Dispose(); } - private async Task CreateClientAsync() => - await IgniteClient.StartAsync(Configuration.ClientConfiguration).ConfigureAwait(false); + private async Task CreateClientAsync() + { + var client = await IgniteClient.StartAsync(Configuration.ClientConfiguration).ConfigureAwait(false); + + return (IgniteClientInternal)client; + } } diff --git a/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs b/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs index b95172cf966..5678025dba9 100644 --- a/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs +++ b/modules/platforms/dotnet/Apache.Ignite/Internal/ClientFailoverSocket.cs @@ -118,6 +118,11 @@ private ClientFailoverSocket(IgniteClientConfiguration configuration, ILogger lo /// public Guid ClientId { get; } = Guid.NewGuid(); + /// + /// Gets a value indicating whether the socket is disposed. + /// + public bool IsDisposed => _disposed; + /// /// Connects the socket. /// diff --git a/modules/platforms/dotnet/Apache.Ignite/Internal/IgniteClientInternal.cs b/modules/platforms/dotnet/Apache.Ignite/Internal/IgniteClientInternal.cs index 9fd3a8f6f7c..e04ca9cb1f8 100644 --- a/modules/platforms/dotnet/Apache.Ignite/Internal/IgniteClientInternal.cs +++ b/modules/platforms/dotnet/Apache.Ignite/Internal/IgniteClientInternal.cs @@ -68,6 +68,11 @@ public IgniteClientInternal(ClientFailoverSocket socket) /// public ISql Sql { get; } + /// + /// Gets a value indicating whether the client is disposed. + /// + public bool IsDisposed => Socket.IsDisposed; + /// /// Gets the underlying socket. /// From 6c9c3258d444d417e896be51d38dbf1103eba888 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:36:42 +0200 Subject: [PATCH 08/23] wip TestPoolReconnectsDisposedClient --- .../IgniteClientPoolTests.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index 2547b48e6a9..00a8f413c34 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -37,16 +37,35 @@ public class IgniteClientPoolTests [Test] public async Task TestGetClient() { - var pool = new IgniteClientPool(new(new(_server.Endpoint), 1)); + IgniteClientPool pool = CreatePool(); IIgniteClient client = await pool.GetClientAsync(); + IIgniteClient client2 = await pool.GetClientAsync(); Assert.IsNotNull(client); + Assert.AreSame(client, client2); + await client.Tables.GetTablesAsync(); } + [Test] + public async Task TestPoolReconnectsDisposedClient() + { + IgniteClientPool pool = CreatePool(); + IIgniteClient client = await pool.GetClientAsync(); + + await client.Tables.GetTablesAsync(); + client.Dispose(); + + IIgniteClient client2 = await pool.GetClientAsync(); + await client2.Tables.GetTablesAsync(); + + Assert.AreNotSame(client, client2); + } + [Test] public void TestConstructorValidatesArgs() { + // ReSharper disable once ObjectCreationAsStatement Assert.Throws(() => new IgniteClientPool(null!)); } @@ -58,4 +77,7 @@ public void TestUseAfterDispose() Assert.ThrowsAsync(async () => await pool.GetClientAsync()); } + + private IgniteClientPool CreatePool() => + new(new(new(_server.Endpoint), 1)); } From 7beeaceaa5b9c47e383653e4d8278dc010bedbf0 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:37:28 +0200 Subject: [PATCH 09/23] wip TestPoolReconnectsDisposedClient --- .../dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index 00a8f413c34..6217251e270 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -37,7 +37,7 @@ public class IgniteClientPoolTests [Test] public async Task TestGetClient() { - IgniteClientPool pool = CreatePool(); + using IgniteClientPool pool = CreatePool(); IIgniteClient client = await pool.GetClientAsync(); IIgniteClient client2 = await pool.GetClientAsync(); @@ -50,7 +50,7 @@ public async Task TestGetClient() [Test] public async Task TestPoolReconnectsDisposedClient() { - IgniteClientPool pool = CreatePool(); + using IgniteClientPool pool = CreatePool(); IIgniteClient client = await pool.GetClientAsync(); await client.Tables.GetTablesAsync(); @@ -72,7 +72,7 @@ public void TestConstructorValidatesArgs() [Test] public void TestUseAfterDispose() { - var pool = new IgniteClientPool(new(new(), 1)); + IgniteClientPool pool = CreatePool(); pool.Dispose(); Assert.ThrowsAsync(async () => await pool.GetClientAsync()); From ef419197922d680c953ee578129d01d99e546cd6 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:37:53 +0200 Subject: [PATCH 10/23] wip tests --- .../dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index 6217251e270..a4d3c6560d3 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -28,10 +28,10 @@ public class IgniteClientPoolTests { private FakeServer _server; - [OneTimeSetUp] + [SetUp] public void StartServer() => _server = new FakeServer(); - [OneTimeTearDown] + [TearDown] public void StopServer() => _server.Dispose(); [Test] From 6192bc80dc7ffcc36f171720df1c6fe28b93db3d Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:39:32 +0200 Subject: [PATCH 11/23] wip TestRoundRobin --- .../IgniteClientPoolTests.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index a4d3c6560d3..40dbba8108b 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -47,6 +47,27 @@ public async Task TestGetClient() await client.Tables.GetTablesAsync(); } + [Test] + public async Task TestRoundRobin() + { + using IgniteClientPool pool = CreatePool(3); + + var client1 = await pool.GetClientAsync(); + var client2 = await pool.GetClientAsync(); + var client3 = await pool.GetClientAsync(); + + Assert.AreNotSame(client1, client2); + Assert.AreNotSame(client2, client3); + + Assert.AreSame(client1, await pool.GetClientAsync()); + Assert.AreSame(client2, await pool.GetClientAsync()); + Assert.AreSame(client3, await pool.GetClientAsync()); + + Assert.AreSame(client1, await pool.GetClientAsync()); + Assert.AreSame(client2, await pool.GetClientAsync()); + Assert.AreSame(client3, await pool.GetClientAsync()); + } + [Test] public async Task TestPoolReconnectsDisposedClient() { @@ -78,6 +99,6 @@ public void TestUseAfterDispose() Assert.ThrowsAsync(async () => await pool.GetClientAsync()); } - private IgniteClientPool CreatePool() => - new(new(new(_server.Endpoint), 1)); + private IgniteClientPool CreatePool(int size = 1) => + new(new(new(_server.Endpoint), size)); } From 4083e21f24ee09381e7593af84a6c4c585e17c04 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 13:51:41 +0200 Subject: [PATCH 12/23] Allow multiple connections in FakeServer / IgniteServerBase --- .../Apache.Ignite.Tests/IgniteServerBase.cs | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServerBase.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServerBase.cs index fc82f0043e2..c57101e404b 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServerBase.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServerBase.cs @@ -18,6 +18,7 @@ namespace Apache.Ignite.Tests; using System; +using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Net; using System.Net.Sockets; @@ -35,7 +36,7 @@ public abstract class IgniteServerBase : IDisposable private readonly object _disposeSyncRoot = new(); - private volatile Socket? _handler; + private readonly ConcurrentDictionary _handlers = new(); private bool _disposed; @@ -66,7 +67,13 @@ public bool DropNewConnections protected Socket Listener => _listener; - public void DropExistingConnection() => _handler?.Dispose(); + public void DropExistingConnection() + { + foreach (var handler in _handlers.Keys) + { + handler.Dispose(); + } + } public void Dispose() { @@ -124,7 +131,14 @@ protected virtual void Dispose(bool disposing) } _cts.Cancel(); - _handler?.Dispose(); + + foreach (var handler in _handlers.Keys) + { + handler.Dispose(); + } + + _handlers.Clear(); + _listener.Dispose(); _cts.Dispose(); @@ -159,21 +173,28 @@ private void ListenLoopInternal() { while (!_cts.IsCancellationRequested) { - using Socket handler = _listener.Accept(); + Socket handler = _listener.Accept(); if (DropNewConnections) { handler.Disconnect(true); - _handler = null; + handler.Dispose(); continue; } - _handler = handler; - handler.NoDelay = true; + _handlers[handler] = null; + + Task.Run(() => + { + using (handler) + { + handler.NoDelay = true; - Handle(handler, _cts.Token); - handler.Disconnect(true); - _handler = null; + Handle(handler, _cts.Token); + handler.Disconnect(true); + _handlers.TryRemove(handler, out _); + } + }); } } From 893bafe6f55211702be3aa8e42d67d4008dafe13 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 14:12:37 +0200 Subject: [PATCH 13/23] Add TestToString --- .../Apache.Ignite.Tests/IgniteClientPoolTests.cs | 13 ++++++++++++- .../dotnet/Apache.Ignite/IgniteClientPool.cs | 8 ++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index 40dbba8108b..ecc233fcf72 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -50,7 +50,7 @@ public async Task TestGetClient() [Test] public async Task TestRoundRobin() { - using IgniteClientPool pool = CreatePool(3); + using IgniteClientPool pool = CreatePool(size: 3); var client1 = await pool.GetClientAsync(); var client2 = await pool.GetClientAsync(); @@ -99,6 +99,17 @@ public void TestUseAfterDispose() Assert.ThrowsAsync(async () => await pool.GetClientAsync()); } + [Test] + public async Task TestToString() + { + var pool = CreatePool(5); + + await pool.GetClientAsync(); + await pool.GetClientAsync(); + + Assert.AreEqual("IgniteClientPool { Connected = 2, Size = 5 }", pool.ToString()); + } + private IgniteClientPool CreatePool(int size = 1) => new(new(new(_server.Endpoint), size)); } diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index ac650a521d1..d32133bc162 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -19,6 +19,7 @@ namespace Apache.Ignite; using System; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Internal; @@ -122,6 +123,13 @@ public void Dispose() _clientsLock.Dispose(); } + /// + public override string ToString() => + new IgniteToStringBuilder(typeof(IgniteClientPool)) + .Append(_clients.Count(static c => c is { IsDisposed: false }), "Connected") + .Append(Configuration.PoolSize, "Size") + .Build(); + private async Task CreateClientAsync() { var client = await IgniteClient.StartAsync(Configuration.ClientConfiguration).ConfigureAwait(false); From 070fc6c750257851a1b93305a7d93fcd772192bc Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 17:37:57 +0200 Subject: [PATCH 14/23] Make IgniteClientPoolConfiguration mutable --- .../dotnet/Apache.Ignite/IgniteClientPool.cs | 11 +++++++---- .../IgniteClientPoolConfiguration.cs | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index d32133bc162..7082e8e60b4 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -28,7 +28,7 @@ namespace Apache.Ignite; /// /// Ignite client pool. Thread safe. /// -/// This pool creates up to Ignite clients and returns them in a round-robin fashion. +/// This pool creates up to Ignite clients and returns them in a round-robin fashion. /// Ignite clients are thread safe, so there is no rent/return semantics. /// /// @@ -49,10 +49,11 @@ public sealed class IgniteClientPool : IDisposable public IgniteClientPool(IgniteClientPoolConfiguration configuration) { IgniteArgumentCheck.NotNull(configuration); - IgniteArgumentCheck.Ensure(configuration.PoolSize > 0, nameof(configuration.PoolSize), "PoolSize > 0"); + IgniteArgumentCheck.NotNull(configuration.ClientConfiguration); + IgniteArgumentCheck.Ensure(configuration.Size > 0, nameof(configuration.Size), "Pool size must be positive."); Configuration = configuration; - _clients = new IgniteClientInternal[configuration.PoolSize]; + _clients = new IgniteClientInternal[configuration.Size]; } /// @@ -68,6 +69,8 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) /// /// Gets an Ignite client from the pool. Creates a new one if necessary. /// Performs round-robin balancing across pooled instances. + /// + /// NOTE: Do not dispose the client instance returned by this method, it is managed by the pool. /// /// Ignite client. [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] @@ -127,7 +130,7 @@ public void Dispose() public override string ToString() => new IgniteToStringBuilder(typeof(IgniteClientPool)) .Append(_clients.Count(static c => c is { IsDisposed: false }), "Connected") - .Append(Configuration.PoolSize, "Size") + .Append(Configuration.Size, "Size") .Build(); private async Task CreateClientAsync() diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs index 14c1549890e..644b0c73b0a 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs @@ -20,8 +20,15 @@ namespace Apache.Ignite; /// /// Ignite client pool configuration. /// -/// Client configuration. -/// Pool size. -public sealed record IgniteClientPoolConfiguration( // TODO: immutability inconsistent with IgniteClientConfiguration - IgniteClientConfiguration ClientConfiguration, - int PoolSize); +public sealed record IgniteClientPoolConfiguration +{ + /// + /// Gets or sets the pool size (maximum number of clients). + /// + public int Size { get; set; } + + /// + /// Gets or sets the configuration for pooled clients. + /// + public required IgniteClientConfiguration ClientConfiguration { get; set; } +} From a0f5931311ddd185b12ecf77ba7261e15418cc3f Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 17:40:11 +0200 Subject: [PATCH 15/23] Fix tests --- .../dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index ecc233fcf72..fc861b13bdd 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -111,5 +111,10 @@ public async Task TestToString() } private IgniteClientPool CreatePool(int size = 1) => - new(new(new(_server.Endpoint), size)); + new IgniteClientPool( + new IgniteClientPoolConfiguration + { + Size = size, + ClientConfiguration = new IgniteClientConfiguration(_server.Endpoint) + }); } From adf9d9a253884dd24a275d818e656016891c4c96 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 17:50:45 +0200 Subject: [PATCH 16/23] Improve TestUseAfterDispose --- .../Apache.Ignite.Tests/IgniteClientPoolTests.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs index fc861b13bdd..2525807a85b 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs @@ -91,12 +91,21 @@ public void TestConstructorValidatesArgs() } [Test] - public void TestUseAfterDispose() + public async Task TestUseAfterDispose() { - IgniteClientPool pool = CreatePool(); + IgniteClientPool pool = CreatePool(size: 2); + + var client1 = await pool.GetClientAsync(); + var client2 = await pool.GetClientAsync(); + + Assert.AreNotSame(client1, client2); + pool.Dispose(); + // Pool and clients are disposed, all operations should throw. Assert.ThrowsAsync(async () => await pool.GetClientAsync()); + Assert.ThrowsAsync(async () => await client1.Tables.GetTablesAsync()); + Assert.ThrowsAsync(async () => await client2.Tables.GetTablesAsync()); } [Test] From 12feabb0d7d3becd290dda97cb58733d45231dc5 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 17:55:46 +0200 Subject: [PATCH 17/23] Use ValueTask --- modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index 7082e8e60b4..784a725c805 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -74,7 +74,7 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) /// /// Ignite client. [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] - public async Task GetClientAsync() + public async ValueTask GetClientAsync() { ObjectDisposedException.ThrowIf(IsDisposed, this); From 84d50b95f30fff362c8de81c2661c5febd8b22bd Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Wed, 20 Nov 2024 18:03:57 +0200 Subject: [PATCH 18/23] Add xmldoc examples --- .../dotnet/Apache.Ignite/IgniteClientPool.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs index 784a725c805..eb857ba55d0 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs @@ -31,6 +31,26 @@ namespace Apache.Ignite; /// This pool creates up to Ignite clients and returns them in a round-robin fashion. /// Ignite clients are thread safe, so there is no rent/return semantics. /// +/// +/// Register as a singleton in DI container: +/// +/// builder.Services.AddSingleton(_ => new IgniteClientPool( +/// new IgniteClientPoolConfiguration +/// { +/// Size = 3, +/// ClientConfiguration = new("localhost"), +/// })); +/// +/// Invoke from a controller: +/// +/// public async Task<IActionResult> Index([FromServices] IgniteClientPool ignite) +/// { +/// var client = await ignite.GetClientAsync(); +/// var tables = await client.Tables.GetTablesAsync(); +/// return Ok(tables); +/// } +/// +/// /// public sealed class IgniteClientPool : IDisposable { From cb6b8264e683ffd3c190268d0ed20d60fae31cd2 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Fri, 22 Nov 2024 09:53:31 +0200 Subject: [PATCH 19/23] Rename to IgniteClientGroup, return IIgnite --- ...PoolTests.cs => IgniteClientGroupTests.cs} | 60 +++++++++---------- ...niteClientPool.cs => IgniteClientGroup.cs} | 16 ++--- ...n.cs => IgniteClientGroupConfiguration.cs} | 8 +-- 3 files changed, 42 insertions(+), 42 deletions(-) rename modules/platforms/dotnet/Apache.Ignite.Tests/{IgniteClientPoolTests.cs => IgniteClientGroupTests.cs} (64%) rename modules/platforms/dotnet/Apache.Ignite/{IgniteClientPool.cs => IgniteClientGroup.cs} (90%) rename modules/platforms/dotnet/Apache.Ignite/{IgniteClientPoolConfiguration.cs => IgniteClientGroupConfiguration.cs} (80%) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs similarity index 64% rename from modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs rename to modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs index 2525807a85b..8c3d9294381 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientPoolTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs @@ -22,9 +22,9 @@ namespace Apache.Ignite.Tests; using NUnit.Framework; /// -/// Tests for . +/// Tests for . /// -public class IgniteClientPoolTests +public class IgniteClientGroupTests { private FakeServer _server; @@ -37,9 +37,9 @@ public class IgniteClientPoolTests [Test] public async Task TestGetClient() { - using IgniteClientPool pool = CreatePool(); - IIgniteClient client = await pool.GetClientAsync(); - IIgniteClient client2 = await pool.GetClientAsync(); + using IgniteClientGroup group = CreatePool(); + IIgnite client = await group.GetIgniteAsync(); + IIgnite client2 = await group.GetIgniteAsync(); Assert.IsNotNull(client); Assert.AreSame(client, client2); @@ -50,34 +50,34 @@ public async Task TestGetClient() [Test] public async Task TestRoundRobin() { - using IgniteClientPool pool = CreatePool(size: 3); + using IgniteClientGroup group = CreatePool(size: 3); - var client1 = await pool.GetClientAsync(); - var client2 = await pool.GetClientAsync(); - var client3 = await pool.GetClientAsync(); + var client1 = await group.GetIgniteAsync(); + var client2 = await group.GetIgniteAsync(); + var client3 = await group.GetIgniteAsync(); Assert.AreNotSame(client1, client2); Assert.AreNotSame(client2, client3); - Assert.AreSame(client1, await pool.GetClientAsync()); - Assert.AreSame(client2, await pool.GetClientAsync()); - Assert.AreSame(client3, await pool.GetClientAsync()); + Assert.AreSame(client1, await group.GetIgniteAsync()); + Assert.AreSame(client2, await group.GetIgniteAsync()); + Assert.AreSame(client3, await group.GetIgniteAsync()); - Assert.AreSame(client1, await pool.GetClientAsync()); - Assert.AreSame(client2, await pool.GetClientAsync()); - Assert.AreSame(client3, await pool.GetClientAsync()); + Assert.AreSame(client1, await group.GetIgniteAsync()); + Assert.AreSame(client2, await group.GetIgniteAsync()); + Assert.AreSame(client3, await group.GetIgniteAsync()); } [Test] public async Task TestPoolReconnectsDisposedClient() { - using IgniteClientPool pool = CreatePool(); - IIgniteClient client = await pool.GetClientAsync(); + using IgniteClientGroup group = CreatePool(); + IIgnite client = await group.GetIgniteAsync(); await client.Tables.GetTablesAsync(); - client.Dispose(); + ((IDisposable)client).Dispose(); - IIgniteClient client2 = await pool.GetClientAsync(); + IIgnite client2 = await group.GetIgniteAsync(); await client2.Tables.GetTablesAsync(); Assert.AreNotSame(client, client2); @@ -87,23 +87,23 @@ public async Task TestPoolReconnectsDisposedClient() public void TestConstructorValidatesArgs() { // ReSharper disable once ObjectCreationAsStatement - Assert.Throws(() => new IgniteClientPool(null!)); + Assert.Throws(() => new IgniteClientGroup(null!)); } [Test] public async Task TestUseAfterDispose() { - IgniteClientPool pool = CreatePool(size: 2); + IgniteClientGroup group = CreatePool(size: 2); - var client1 = await pool.GetClientAsync(); - var client2 = await pool.GetClientAsync(); + var client1 = await group.GetIgniteAsync(); + var client2 = await group.GetIgniteAsync(); Assert.AreNotSame(client1, client2); - pool.Dispose(); + group.Dispose(); // Pool and clients are disposed, all operations should throw. - Assert.ThrowsAsync(async () => await pool.GetClientAsync()); + Assert.ThrowsAsync(async () => await group.GetIgniteAsync()); Assert.ThrowsAsync(async () => await client1.Tables.GetTablesAsync()); Assert.ThrowsAsync(async () => await client2.Tables.GetTablesAsync()); } @@ -113,15 +113,15 @@ public async Task TestToString() { var pool = CreatePool(5); - await pool.GetClientAsync(); - await pool.GetClientAsync(); + await pool.GetIgniteAsync(); + await pool.GetIgniteAsync(); Assert.AreEqual("IgniteClientPool { Connected = 2, Size = 5 }", pool.ToString()); } - private IgniteClientPool CreatePool(int size = 1) => - new IgniteClientPool( - new IgniteClientPoolConfiguration + private IgniteClientGroup CreatePool(int size = 1) => + new IgniteClientGroup( + new IgniteClientGroupConfiguration { Size = size, ClientConfiguration = new IgniteClientConfiguration(_server.Endpoint) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs similarity index 90% rename from modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs rename to modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs index eb857ba55d0..e27a25e59d0 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPool.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs @@ -26,9 +26,9 @@ namespace Apache.Ignite; using Internal.Common; /// -/// Ignite client pool. Thread safe. +/// Ignite client group. Thread safe. /// -/// This pool creates up to Ignite clients and returns them in a round-robin fashion. +/// Creates and maintains up to Ignite clients and returns them in a round-robin fashion. /// Ignite clients are thread safe, so there is no rent/return semantics. /// /// @@ -52,7 +52,7 @@ namespace Apache.Ignite; /// /// /// -public sealed class IgniteClientPool : IDisposable +public sealed class IgniteClientGroup : IDisposable { private readonly IgniteClientInternal?[] _clients; @@ -63,10 +63,10 @@ public sealed class IgniteClientPool : IDisposable private int _clientIndex; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// Configuration. - public IgniteClientPool(IgniteClientPoolConfiguration configuration) + public IgniteClientGroup(IgniteClientGroupConfiguration configuration) { IgniteArgumentCheck.NotNull(configuration); IgniteArgumentCheck.NotNull(configuration.ClientConfiguration); @@ -79,7 +79,7 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) /// /// Gets the configuration. /// - public IgniteClientPoolConfiguration Configuration { get; } + public IgniteClientGroupConfiguration Configuration { get; } /// /// Gets a value indicating whether the pool is disposed. @@ -94,7 +94,7 @@ public IgniteClientPool(IgniteClientPoolConfiguration configuration) /// /// Ignite client. [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] - public async ValueTask GetClientAsync() + public async ValueTask GetIgniteAsync() { ObjectDisposedException.ThrowIf(IsDisposed, this); @@ -148,7 +148,7 @@ public void Dispose() /// public override string ToString() => - new IgniteToStringBuilder(typeof(IgniteClientPool)) + new IgniteToStringBuilder(typeof(IgniteClientGroup)) .Append(_clients.Count(static c => c is { IsDisposed: false }), "Connected") .Append(Configuration.Size, "Size") .Build(); diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroupConfiguration.cs similarity index 80% rename from modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs rename to modules/platforms/dotnet/Apache.Ignite/IgniteClientGroupConfiguration.cs index 644b0c73b0a..90b569df8ad 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientPoolConfiguration.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroupConfiguration.cs @@ -18,17 +18,17 @@ namespace Apache.Ignite; /// -/// Ignite client pool configuration. +/// Ignite client group configuration. See for more details. /// -public sealed record IgniteClientPoolConfiguration +public sealed record IgniteClientGroupConfiguration { /// - /// Gets or sets the pool size (maximum number of clients). + /// Gets or sets the group size (maximum number of clients). /// public int Size { get; set; } /// - /// Gets or sets the configuration for pooled clients. + /// Gets or sets the client configuration. /// public required IgniteClientConfiguration ClientConfiguration { get; set; } } From 9e6420cbceafe08f1b130d8c84619f1265ed5e76 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Fri, 22 Nov 2024 09:55:25 +0200 Subject: [PATCH 20/23] Fix tests --- .../IgniteClientGroupTests.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs index 8c3d9294381..decef3bebac 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs @@ -37,7 +37,7 @@ public class IgniteClientGroupTests [Test] public async Task TestGetClient() { - using IgniteClientGroup group = CreatePool(); + using IgniteClientGroup group = CreateGroup(); IIgnite client = await group.GetIgniteAsync(); IIgnite client2 = await group.GetIgniteAsync(); @@ -50,7 +50,7 @@ public async Task TestGetClient() [Test] public async Task TestRoundRobin() { - using IgniteClientGroup group = CreatePool(size: 3); + using IgniteClientGroup group = CreateGroup(size: 3); var client1 = await group.GetIgniteAsync(); var client2 = await group.GetIgniteAsync(); @@ -69,9 +69,9 @@ public async Task TestRoundRobin() } [Test] - public async Task TestPoolReconnectsDisposedClient() + public async Task TestGroupReconnectsDisposedClient() { - using IgniteClientGroup group = CreatePool(); + using IgniteClientGroup group = CreateGroup(); IIgnite client = await group.GetIgniteAsync(); await client.Tables.GetTablesAsync(); @@ -93,7 +93,7 @@ public void TestConstructorValidatesArgs() [Test] public async Task TestUseAfterDispose() { - IgniteClientGroup group = CreatePool(size: 2); + IgniteClientGroup group = CreateGroup(size: 2); var client1 = await group.GetIgniteAsync(); var client2 = await group.GetIgniteAsync(); @@ -102,7 +102,7 @@ public async Task TestUseAfterDispose() group.Dispose(); - // Pool and clients are disposed, all operations should throw. + // Group and clients are disposed, all operations should throw. Assert.ThrowsAsync(async () => await group.GetIgniteAsync()); Assert.ThrowsAsync(async () => await client1.Tables.GetTablesAsync()); Assert.ThrowsAsync(async () => await client2.Tables.GetTablesAsync()); @@ -111,15 +111,15 @@ public async Task TestUseAfterDispose() [Test] public async Task TestToString() { - var pool = CreatePool(5); + var group = CreateGroup(5); - await pool.GetIgniteAsync(); - await pool.GetIgniteAsync(); + await group.GetIgniteAsync(); + await group.GetIgniteAsync(); - Assert.AreEqual("IgniteClientPool { Connected = 2, Size = 5 }", pool.ToString()); + Assert.AreEqual("IgniteClientGroup { Connected = 2, Size = 5 }", group.ToString()); } - private IgniteClientGroup CreatePool(int size = 1) => + private IgniteClientGroup CreateGroup(int size = 1) => new IgniteClientGroup( new IgniteClientGroupConfiguration { From 0bf15b30c889548b161948d5290e374b97d1ead3 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Fri, 22 Nov 2024 09:57:20 +0200 Subject: [PATCH 21/23] Fix xmldoc --- .../dotnet/Apache.Ignite/IgniteClientGroup.cs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs index e27a25e59d0..cf2bdcfa579 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs @@ -34,8 +34,8 @@ namespace Apache.Ignite; /// /// Register as a singleton in DI container: /// -/// builder.Services.AddSingleton(_ => new IgniteClientPool( -/// new IgniteClientPoolConfiguration +/// builder.Services.AddSingleton(_ => new IgniteClientGroup( +/// new IgniteClientGroupConfiguration /// { /// Size = 3, /// ClientConfiguration = new("localhost"), @@ -43,10 +43,10 @@ namespace Apache.Ignite; /// /// Invoke from a controller: /// -/// public async Task<IActionResult> Index([FromServices] IgniteClientPool ignite) +/// public async Task<IActionResult> Index([FromServices] IgniteClientGroup igniteGroup) /// { -/// var client = await ignite.GetClientAsync(); -/// var tables = await client.Tables.GetTablesAsync(); +/// IIgnite ignite = await igniteGroup.GetIgniteAsync(); +/// var tables = await ignite.Tables.GetTablesAsync(); /// return Ok(tables); /// } /// @@ -70,7 +70,7 @@ public IgniteClientGroup(IgniteClientGroupConfiguration configuration) { IgniteArgumentCheck.NotNull(configuration); IgniteArgumentCheck.NotNull(configuration.ClientConfiguration); - IgniteArgumentCheck.Ensure(configuration.Size > 0, nameof(configuration.Size), "Pool size must be positive."); + IgniteArgumentCheck.Ensure(configuration.Size > 0, nameof(configuration.Size), "Group size must be positive."); Configuration = configuration; _clients = new IgniteClientInternal[configuration.Size]; @@ -82,18 +82,16 @@ public IgniteClientGroup(IgniteClientGroupConfiguration configuration) public IgniteClientGroupConfiguration Configuration { get; } /// - /// Gets a value indicating whether the pool is disposed. + /// Gets a value indicating whether the group is disposed. /// public bool IsDisposed => Interlocked.CompareExchange(ref _disposed, 0, 0) == 1; /// - /// Gets an Ignite client from the pool. Creates a new one if necessary. - /// Performs round-robin balancing across pooled instances. - /// - /// NOTE: Do not dispose the client instance returned by this method, it is managed by the pool. + /// Gets an Ignite client from the group. Creates a new one if necessary. + /// Performs round-robin balancing across grouped instances. /// /// Ignite client. - [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Pooled.")] + [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Managed by the group.")] public async ValueTask GetIgniteAsync() { ObjectDisposedException.ThrowIf(IsDisposed, this); From f613e60a9565d89b18bc2be3e6bbb7ebf766ebb3 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Fri, 22 Nov 2024 10:40:33 +0200 Subject: [PATCH 22/23] Add defensive copy for IgniteClientGroupConfiguration --- .../dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs | 2 ++ .../platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs | 9 +++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs index decef3bebac..de33e683274 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs @@ -103,6 +103,8 @@ public async Task TestUseAfterDispose() group.Dispose(); // Group and clients are disposed, all operations should throw. + Assert.IsTrue(group.IsDisposed); + Assert.ThrowsAsync(async () => await group.GetIgniteAsync()); Assert.ThrowsAsync(async () => await client1.Tables.GetTablesAsync()); Assert.ThrowsAsync(async () => await client2.Tables.GetTablesAsync()); diff --git a/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs index cf2bdcfa579..978352d3d61 100644 --- a/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs +++ b/modules/platforms/dotnet/Apache.Ignite/IgniteClientGroup.cs @@ -54,6 +54,8 @@ namespace Apache.Ignite; /// public sealed class IgniteClientGroup : IDisposable { + private readonly IgniteClientGroupConfiguration _configuration; + private readonly IgniteClientInternal?[] _clients; private readonly SemaphoreSlim _clientsLock = new(1); @@ -72,14 +74,14 @@ public IgniteClientGroup(IgniteClientGroupConfiguration configuration) IgniteArgumentCheck.NotNull(configuration.ClientConfiguration); IgniteArgumentCheck.Ensure(configuration.Size > 0, nameof(configuration.Size), "Group size must be positive."); - Configuration = configuration; + _configuration = Copy(configuration); _clients = new IgniteClientInternal[configuration.Size]; } /// /// Gets the configuration. /// - public IgniteClientGroupConfiguration Configuration { get; } + public IgniteClientGroupConfiguration Configuration => Copy(_configuration); /// /// Gets a value indicating whether the group is disposed. @@ -151,6 +153,9 @@ public override string ToString() => .Append(Configuration.Size, "Size") .Build(); + private static IgniteClientGroupConfiguration Copy(IgniteClientGroupConfiguration cfg) => + cfg with { ClientConfiguration = cfg.ClientConfiguration with { } }; + private async Task CreateClientAsync() { var client = await IgniteClient.StartAsync(Configuration.ClientConfiguration).ConfigureAwait(false); From 795bbe75af4341ccb21eda17f65fff6a9f8bcc18 Mon Sep 17 00:00:00 2001 From: Pavel Tupitsyn Date: Fri, 22 Nov 2024 10:50:26 +0200 Subject: [PATCH 23/23] Add TestConfigurationCantBeChanged --- .../Apache.Ignite.Tests/IgniteClientGroupTests.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs index de33e683274..fbe37172fa0 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/IgniteClientGroupTests.cs @@ -121,6 +121,18 @@ public async Task TestToString() Assert.AreEqual("IgniteClientGroup { Connected = 2, Size = 5 }", group.ToString()); } + [Test] + public void TestConfigurationCantBeChanged() + { + IgniteClientGroup group = CreateGroup(3); + + var configuration = group.Configuration; + configuration.Size = 100; + + Assert.AreEqual(3, group.Configuration.Size); + Assert.AreNotSame(configuration, group.Configuration); + } + private IgniteClientGroup CreateGroup(int size = 1) => new IgniteClientGroup( new IgniteClientGroupConfiguration