From 51fb59a6559ceb2844a306b4cbca121b0fedc3a2 Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Sun, 20 Sep 2026 10:25:49 -0700 Subject: [PATCH 01/10] fix(zookeeper): retry connection-loss reads safely --- .../Orleans.Clustering.ZooKeeper.csproj | 1 + .../ZooKeeperBasedMembershipTable.cs | 95 ++- .../ZooKeeperGatewayListProvider.cs | 22 +- .../ZooKeeperReadRetryPolicy.cs | 75 ++ .../ZooKeeperSession.cs | 37 + .../Orleans.Clustering.ZooKeeper.Tests.csproj | 2 + .../ZooKeeperBasedMembershipTableUnitTests.cs | 351 ++++++++- .../ZooKeeperNativeDiagnostics.cs | 98 +++ .../ZooKeeperReadResilienceTests.cs | 125 ++++ .../ZooKeeperReadRetryTests.cs | 701 ++++++++++++++++++ 10 files changed, 1481 insertions(+), 26 deletions(-) create mode 100644 src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs create mode 100644 src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs create mode 100644 test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperNativeDiagnostics.cs create mode 100644 test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs create mode 100644 test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs diff --git a/src/Orleans.Clustering.ZooKeeper/Orleans.Clustering.ZooKeeper.csproj b/src/Orleans.Clustering.ZooKeeper/Orleans.Clustering.ZooKeeper.csproj index 806eefb0e86..4d5467dc510 100644 --- a/src/Orleans.Clustering.ZooKeeper/Orleans.Clustering.ZooKeeper.csproj +++ b/src/Orleans.Clustering.ZooKeeper/Orleans.Clustering.ZooKeeper.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs index f81f18834fd..e26aa3fad8e 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs @@ -12,6 +12,7 @@ using Microsoft.Extensions.Options; using Orleans.Configuration; using Orleans.Runtime.Host; +using Polly; namespace Orleans.Runtime.Membership { @@ -43,6 +44,8 @@ public partial class ZooKeeperBasedMembershipTable : IMembershipTable private const int ZOOKEEPER_SESSION_TIMEOUT = 10_000; private readonly ZooKeeperWatcher watcher; + private readonly Func _createSession; + private readonly ResiliencePipeline _readRetryPipeline; /// /// The deployment connection string. for eg. "192.168.1.1,192.168.1.2/ClusterId" @@ -73,6 +76,16 @@ public ZooKeeperBasedMembershipTable( ILogger logger, IOptions membershipTableOptions, IOptions clusterOptions) + : this(logger, membershipTableOptions, clusterOptions, null, null) + { + } + + internal ZooKeeperBasedMembershipTable( + ILogger logger, + IOptions membershipTableOptions, + IOptions clusterOptions, + Func? createSession, + ResiliencePipeline? readRetryPipeline) { ArgumentNullException.ThrowIfNull(logger); ArgumentNullException.ThrowIfNull(membershipTableOptions); @@ -84,6 +97,8 @@ public ZooKeeperBasedMembershipTable( this.clusterPath = "/" + clusterOptions.Value.ClusterId; rootConnectionString = options.ConnectionString; deploymentConnectionString = options.ConnectionString + this.clusterPath; + _createSession = createSession ?? (readOnly => CreateSession(deploymentConnectionString, watcher, readOnly)); + _readRetryPipeline = readRetryPipeline ?? ZooKeeperReadRetryPolicy.CreatePipeline(logger, TimeProvider.System); } /// @@ -128,10 +143,13 @@ await UsingZookeeper(rootConnectionString, async zk => public Task ReadRow(SiloAddress siloAddress) => ReadRowAsync(siloAddress, CancellationToken.None); /// + /// + /// Connection-loss failures in native reads are retried up to four times on the operation's session. + /// The table and child versions fence each complete snapshot pass. + /// public Task ReadRowAsync(SiloAddress siloAddress, CancellationToken cancellationToken = default) { - return UsingZookeeper(zk => ReadCoreAsync(zk, siloAddress, cancellationToken), - this.deploymentConnectionString, this.watcher, cancellationToken, canBeReadOnly: true); + return ReadAsync(() => _createSession(true), _readRetryPipeline, siloAddress, cancellationToken); } /// @@ -145,15 +163,36 @@ public Task ReadRowAsync(SiloAddress siloAddress, Cancellat public Task ReadAll() => ReadAllAsync(CancellationToken.None); /// + /// + /// Rows are read concurrently on an operation-owned connection, + /// with each membership record read before its heartbeat. + /// Table and child-version checks fence the complete snapshot. + /// Connection-loss failures in native reads are retried up to four times on the same session. + /// Caller cancellation stops further requests while admitted requests and client close complete. + /// public Task ReadAllAsync(CancellationToken cancellationToken = default) { - return ReadAllAsync(this.deploymentConnectionString, this.watcher, cancellationToken); + return ReadAsync(() => _createSession(true), _readRetryPipeline, null, cancellationToken); } - internal static Task ReadAllAsync(string deploymentConnectionString, ZooKeeperWatcher watcher, CancellationToken cancellationToken) + internal static Task ReadAsync( + Func createSession, + ResiliencePipeline pipeline, + SiloAddress? siloAddress, + CancellationToken cancellationToken) { - return UsingZookeeper(zk => ReadCoreAsync(zk, null, cancellationToken), - deploymentConnectionString, watcher, cancellationToken, canBeReadOnly: true); + return ZooKeeperSession.ExecuteAsync(createSession, + native => ReadCoreAsync(ZooKeeperReadRetryPolicy.Wrap(native, pipeline, cancellationToken), siloAddress, cancellationToken), + cancellationToken); + } + + internal static ZooKeeperSession CreateSession(string connectionString, ZooKeeperWatcher watcher, bool readOnly) + { + var client = new ZooKeeper(connectionString, ZOOKEEPER_SESSION_TIMEOUT, watcher, readOnly); + return new ZooKeeperSession( + new NativeOperations(path => client.getDataAsync(path), path => client.getChildrenAsync(path), + client.sync, operations => client.multiAsync(operations), client.setDataAsync), + client.closeAsync); } internal static async Task ReadCoreAsync( @@ -179,36 +218,36 @@ internal static async Task ReadCoreAsync( addresses = [siloAddress]; } + var rows = new List>(); + KeeperException.NoNodeException? missingRow = null; + cancellationToken.ThrowIfCancellationRequested(); var pendingRows = Task.WhenAll(addresses.Select(address => GetRow(zk, address, siloAddress is not null, cancellationToken))); - Tuple?[] rows; try { - rows = await pendingRows; + rows.AddRange((await pendingRows).OfType>()); } - catch (KeeperException.NoNodeException) + catch (KeeperException.NoNodeException exception) { - // Observe every parallel read: a removed row must not hide another request's failure. - var failure = pendingRows.Exception!.InnerExceptions.FirstOrDefault(exception => exception is not KeeperException.NoNodeException); + // Join every admitted read so a missing row cannot hide another native failure. + var failure = pendingRows.Exception!.InnerExceptions.FirstOrDefault(error => error is not KeeperException.NoNodeException); if (failure is not null) { System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(failure).Throw(); } - cancellationToken.ThrowIfCancellationRequested(); - var current = await zk.GetData("/"); - if (SameVersion(before, current.Stat)) - { - throw; - } - - continue; + missingRow = exception; } cancellationToken.ThrowIfCancellationRequested(); var after = await zk.GetData("/"); if (SameVersion(before, after.Stat)) { - return new MembershipTableData(rows.OfType>().ToList(), ConvertToTableVersion(after.Stat)); + if (missingRow is not null) + { + System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(missingRow).Throw(); + } + + return new MembershipTableData(rows, ConvertToTableVersion(after.Stat)); } } } @@ -238,14 +277,18 @@ private static bool SameVersion(Stat before, Stat after) => public Task InsertRow(MembershipEntry entry, TableVersion tableVersion) => InsertRowAsync(entry, tableVersion, CancellationToken.None); /// + /// + /// The conditional transaction is submitted once. Connection loss propagates when its + /// commit outcome is unknown, preserving the caller's ability to resolve that outcome. + /// public Task InsertRowAsync(MembershipEntry entry, TableVersion tableVersion, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(entry); ArgumentNullException.ThrowIfNull(tableVersion); cancellationToken.ThrowIfCancellationRequested(); - return UsingZookeeper(zk => InsertRowCoreAsync(zk, entry, tableVersion, cancellationToken), - this.deploymentConnectionString, this.watcher, cancellationToken); + return ZooKeeperSession.ExecuteAsync(() => _createSession(false), + zk => InsertRowCoreAsync(zk, entry, tableVersion, cancellationToken), cancellationToken); } internal static async Task InsertRowCoreAsync( @@ -300,6 +343,10 @@ await zk.Multi( public Task UpdateRow(MembershipEntry entry, string etag, TableVersion tableVersion) => UpdateRowAsync(entry, etag, tableVersion, CancellationToken.None); /// + /// + /// The conditional transaction is submitted once. Connection loss propagates when its + /// commit outcome is unknown, preserving the caller's ability to resolve that outcome. + /// public Task UpdateRowAsync(MembershipEntry entry, string etag, TableVersion tableVersion, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(entry); @@ -307,8 +354,8 @@ public Task UpdateRowAsync(MembershipEntry entry, string etag, TableVersio ArgumentNullException.ThrowIfNull(tableVersion); cancellationToken.ThrowIfCancellationRequested(); - return UsingZookeeper(zk => UpdateRowCoreAsync(zk, entry, etag, tableVersion, cancellationToken), - this.deploymentConnectionString, this.watcher, cancellationToken); + return ZooKeeperSession.ExecuteAsync(() => _createSession(false), + zk => UpdateRowCoreAsync(zk, entry, etag, tableVersion, cancellationToken), cancellationToken); } internal static async Task UpdateRowCoreAsync( diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperGatewayListProvider.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperGatewayListProvider.cs index aa9cebecda2..c35bd695c68 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperGatewayListProvider.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperGatewayListProvider.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Orleans.Configuration; +using Polly; namespace Orleans.Runtime.Membership { @@ -27,6 +28,8 @@ public class ZooKeeperGatewayListProvider : IGatewayListProvider /// private readonly string _deploymentConnectionString; private readonly TimeSpan _maxStaleness; + private readonly Func _createSession; + private readonly ResiliencePipeline _readRetryPipeline; /// /// Initializes a new instance of the class. @@ -44,6 +47,17 @@ public ZooKeeperGatewayListProvider( IOptions options, IOptions gatewayOptions, IOptions clusterOptions) + : this(logger, options, gatewayOptions, clusterOptions, null, null) + { + } + + internal ZooKeeperGatewayListProvider( + ILogger logger, + IOptions options, + IOptions gatewayOptions, + IOptions clusterOptions, + Func? createSession, + ResiliencePipeline? readRetryPipeline) { ArgumentNullException.ThrowIfNull(logger); ArgumentNullException.ThrowIfNull(options); @@ -54,6 +68,8 @@ public ZooKeeperGatewayListProvider( _deploymentPath = "/" + clusterOptions.Value.ClusterId; _deploymentConnectionString = options.Value.ConnectionString + _deploymentPath; _maxStaleness = gatewayOptions.Value.GatewayListRefreshPeriod; + _createSession = createSession ?? (() => ZooKeeperBasedMembershipTable.CreateSession(_deploymentConnectionString, _watcher, true)); + _readRetryPipeline = readRetryPipeline ?? ZooKeeperReadRetryPolicy.CreatePipeline(logger, TimeProvider.System); } /// @@ -65,9 +81,13 @@ public ZooKeeperGatewayListProvider( /// Returns the list of gateways (silos) that can be used by a client to connect to Orleans cluster. /// The Uri is in the form of: "gwy.tcp://IP:port/Generation". See Utils.ToGatewayUri and Utils.ToSiloAddress for more details about Uri format. /// + /// + /// Gateway discovery uses a version-fenced membership snapshot. Native reads retry connection-loss + /// failures up to four times on the operation's session before propagating the final failure. + /// public async Task> GetGateways() { - var membershipTableData = await ZooKeeperBasedMembershipTable.ReadAllAsync(this._deploymentConnectionString, this._watcher, CancellationToken.None); + var membershipTableData = await ZooKeeperBasedMembershipTable.ReadAsync(_createSession, _readRetryPipeline, null, CancellationToken.None); return membershipTableData.Members.Select(e => e.Item1). Where(m => m.Status == SiloStatus.Active && m.ProxyPort != 0). Select(m => diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs new file mode 100644 index 00000000000..da5ce58583b --- /dev/null +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs @@ -0,0 +1,75 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using org.apache.zookeeper; +using Polly; +using Polly.Retry; + +namespace Orleans.Runtime.Membership; + +internal static partial class ZooKeeperReadRetryPolicy +{ + internal const int MaxRetryAttempts = 4; + internal static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(250); + + internal static ResiliencePipeline CreatePipeline(ILogger logger, TimeProvider timeProvider) => + new ResiliencePipelineBuilder { TimeProvider = timeProvider } + .AddRetry(new RetryStrategyOptions + { + MaxRetryAttempts = MaxRetryAttempts, + BackoffType = DelayBackoffType.Exponential, + Delay = RetryDelay, + ShouldHandle = new PredicateBuilder().Handle(), + OnRetry = args => + { + LogWarningRetryRead(logger, args.Outcome.Exception!, args.Context.OperationKey!, + args.AttemptNumber + 1, MaxRetryAttempts, args.RetryDelay.TotalMilliseconds); + return default; + } + }) + .Build(); + + internal static ZooKeeperBasedMembershipTable.NativeOperations Wrap( + ZooKeeperBasedMembershipTable.NativeOperations native, + ResiliencePipeline pipeline, + CancellationToken cancellationToken) => + new( + path => ExecuteAsync("GetData", () => native.GetData(path), pipeline, cancellationToken), + path => ExecuteAsync("GetChildren", () => native.GetChildren(path), pipeline, cancellationToken), + path => ExecuteAsync("Sync", async () => + { + await native.Sync(path); + return true; + }, pipeline, cancellationToken), + native.Multi, + native.SetData); + + private static async Task ExecuteAsync( + string operationName, + Func> operation, + ResiliencePipeline pipeline, + CancellationToken cancellationToken) + { + var context = ResilienceContextPool.Shared.Get(operationName, cancellationToken); + try + { + return await pipeline.ExecuteAsync(async context => + { + context.CancellationToken.ThrowIfCancellationRequested(); + // Await actual native completion: cancellation ends admission, not an in-flight request. + return await operation(); + }, context); + } + finally + { + ResilienceContextPool.Shared.Return(context); + } + } + + [LoggerMessage( + Level = LogLevel.Warning, + Message = "ZooKeeper {Operation} lost its connection. Retrying in {DelayMilliseconds}ms ({Retry}/{MaxRetries}).")] + private static partial void LogWarningRetryRead( + ILogger logger, Exception exception, string operation, int retry, int maxRetries, double delayMilliseconds); +} diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs new file mode 100644 index 00000000000..73b52673286 --- /dev/null +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs @@ -0,0 +1,37 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Orleans.Runtime.Membership; + +internal sealed class ZooKeeperSession( + ZooKeeperBasedMembershipTable.NativeOperations operations, + Func close) +{ + internal Task Completion { get; private set; } = Task.CompletedTask; + + internal static Task ExecuteAsync( + Func createSession, + Func> operation, + CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + var session = createSession(); + var completion = session.RunAsync(operation); + session.Completion = completion; + return ZooKeeperBasedMembershipTable.AwaitOperationAsync(completion, cancellationToken); + } + + private async Task RunAsync(Func> operation) + { + try + { + return await operation(operations); + } + finally + { + // The callback joins native requests before this tokenless close. + await close(); + } + } +} diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/Orleans.Clustering.ZooKeeper.Tests.csproj b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/Orleans.Clustering.ZooKeeper.Tests.csproj index 74ae4ddd3cd..a41b183a5b8 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/Orleans.Clustering.ZooKeeper.Tests.csproj +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/Orleans.Clustering.ZooKeeper.Tests.csproj @@ -6,7 +6,9 @@ + + diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs index b33fc088b03..11e277cd7a5 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs @@ -1,7 +1,10 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Net; +using System.Net.Sockets; using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -22,6 +25,29 @@ namespace UnitTests.MembershipTests [TestArea("Membership")] public sealed class ZooKeeperBasedMembershipTableUnitTests { + [Fact] + public async Task NativeSocketDiagnostics_ExistingSourceReportsOriginalCompletionError() + { + using var destination = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + destination.Bind(new IPEndPoint(IPAddress.Loopback, 0)); + var messages = new ConcurrentQueue(); + using var diagnostics = new NativeSocketDiagnostics(messages.Enqueue); + Assert.Contains(messages, message => message.EndsWith("Error listener enabled", StringComparison.Ordinal)); + using var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + using var operation = new SocketAsyncEventArgs { RemoteEndPoint = destination.LocalEndPoint }; + var completion = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + operation.Completed += (_, result) => completion.TrySetResult(result.SocketError); + + if (!client.ConnectAsync(operation)) + { + completion.TrySetResult(operation.SocketError); + } + + Assert.Equal(SocketError.ConnectionRefused, await completion.Task.WaitAsync(TestContext.Current.CancellationToken)); + Assert.Contains(messages, message => + message.Contains($"Socket#{client.GetHashCode()}; UpdateStatusAfterSocketError; errorCode:ConnectionRefused", StringComparison.Ordinal)); + } + [Fact] public void Constructor_NullLogger_ThrowsArgumentNullException() { @@ -381,7 +407,330 @@ public async Task Read_ConcurrentHeartbeat_PreservesCanonicalFence(bool pointRea } [Fact] - public async Task Read_ParallelMissingRowAndAuthorizationFailure_PropagatesAuthorizationFailure() + public async Task ReadAll_PipelinesRowsWithSequentialMemberAndHeartbeatReads() + { + var (fake, first) = await CreateNativeTable(); + var second = CreateTimedEntry(12346); + Assert.True(await Insert(fake, second, 1)); + fake.Calls.Clear(); + var rowStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseRow = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + fake.BeforeRead = async path => + { + if (path == ZooKeeperNativeFake.RowPath(first.SiloAddress)) + { + rowStarted.TrySetResult(); + await releaseRow.Task.WaitAsync(TestContext.Current.CancellationToken); + } + }; + + var read = Read(fake); + try + { + await rowStarted.Task.WaitAsync(TestContext.Current.CancellationToken); + Assert.Equal( + new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), + "read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), + "read " + ZooKeeperNativeFake.HeartbeatPath(second.SiloAddress) }, + fake.Calls); + Assert.False(read.IsCompleted); + } + finally + { + releaseRow.TrySetResult(); + await read; + } + + var result = await read; + Assert.Equal(2, result.Version.Version); + var members = result.Members.ToDictionary(row => row.Item1.SiloAddress); + Assert.Equal(2, members.Count); + foreach (var entry in new[] { first, second }) + { + var member = members[entry.SiloAddress]; + Assert.Equal("0", member.Item2); + Assert.Equal(ZooKeeperBasedMembershipTable.Serialize(entry), ZooKeeperBasedMembershipTable.Serialize(member.Item1)); + } + Assert.Equal( + new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), + "read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), + "read " + ZooKeeperNativeFake.HeartbeatPath(second.SiloAddress), + "read " + ZooKeeperNativeFake.HeartbeatPath(first.SiloAddress), "read /" }, + fake.Calls); + } + + [Theory] + [InlineData(9, false)] + [InlineData(9, true)] + [InlineData(128, false)] + [InlineData(128, true)] + public async Task ReadAll_StartsEveryRowBeforeAwaitingNativeCompletion(int rowCount, bool cancel) + { + var fake = new ZooKeeperNativeFake(); + var entries = Enumerable.Range(0, rowCount).Select(index => CreateTimedEntry(12345 + index)).ToArray(); + for (var index = 0; index < entries.Length; index++) + { + Assert.True(await Insert(fake, entries[index], index)); + } + + fake.Calls.Clear(); + var releaseMembers = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var cancellation = new CancellationTokenSource(); + fake.BeforeRead = async path => + { + if (path != "/" && !path.EndsWith("/IAmAlive", StringComparison.Ordinal)) + { + await releaseMembers.Task.WaitAsync(TestContext.Current.CancellationToken); + } + }; + + var native = fake.Operations; + Task ReadData(string path) + { + // Protect the fake's synchronous call log while native completions remain independently gated. + lock (fake.Calls) + { + return native.GetData(path); + } + } + var operations = new ZooKeeperBasedMembershipTable.NativeOperations( + ReadData, native.GetChildren, native.Sync, native.Multi, native.SetData); + var read = ZooKeeperBasedMembershipTable.ReadCoreAsync(operations, null, cancellation.Token); + var completion = Record.ExceptionAsync(() => read); + try + { + Assert.Equal( + new[] { "sync /", "children /" }.Concat( + entries.Select(entry => "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))), + fake.Calls); + if (cancel) + { + cancellation.Cancel(); + } + + Assert.False(read.IsCompleted); + } + finally + { + releaseMembers.TrySetResult(); + await completion; + } + + if (cancel) + { + Assert.Equal(cancellation.Token, + Assert.IsAssignableFrom(await completion).CancellationToken); + Assert.Equal(2 + rowCount, fake.Calls.Count); + } + else + { + Assert.Null(await completion); + var result = await read; + Assert.Equal(entries.Length, result.Version.Version); + var members = result.Members.ToDictionary(row => row.Item1.SiloAddress); + Assert.Equal(entries.Length, members.Count); + foreach (var entry in entries) + { + var member = members[entry.SiloAddress]; + Assert.Equal("0", member.Item2); + Assert.Equal(ZooKeeperBasedMembershipTable.Serialize(entry), ZooKeeperBasedMembershipTable.Serialize(member.Item1)); + } + Assert.Equal(3 + entries.Length * 2, fake.Calls.Count); + } + } + + [Theory] + [InlineData("missing")] + [InlineData("authorization")] + [InlineData("cancellation")] + public async Task Read_MissingRow_AwaitsOtherAdmittedRowsAndPreservesFailures(string otherOutcome) + { + var (fake, first) = await CreateNativeTable(); + var second = CreateTimedEntry(12346); + Assert.True(await Insert(fake, second, 1)); + fake.Calls.Clear(); + var missingRow = new KeeperException.NoNodeException(ZooKeeperNativeFake.RowPath(first.SiloAddress)); + var releaseOther = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var authorizationFailure = new KeeperException.NoAuthException(); + using var cancellation = new CancellationTokenSource(); + fake.BeforeRead = async path => + { + if (path == ZooKeeperNativeFake.RowPath(first.SiloAddress)) + { + throw missingRow; + } + + if (path == ZooKeeperNativeFake.RowPath(second.SiloAddress)) + { + await releaseOther.Task.WaitAsync(TestContext.Current.CancellationToken); + if (otherOutcome == "authorization") + { + throw authorizationFailure; + } + + if (otherOutcome == "cancellation") + { + cancellation.Cancel(); + cancellation.Token.ThrowIfCancellationRequested(); + } + + throw new KeeperException.NoNodeException(path); + } + }; + + var read = ZooKeeperBasedMembershipTable.ReadCoreAsync(fake.Operations, null, cancellation.Token); + var completion = Record.ExceptionAsync(() => read); + try + { + Assert.Contains("read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), fake.Calls); + Assert.False(read.IsCompleted); + } + finally + { + releaseOther.TrySetResult(); + await completion; + } + + var failure = await completion; + if (otherOutcome == "authorization") + { + Assert.Same(authorizationFailure, failure); + } + else if (otherOutcome == "cancellation") + { + Assert.Equal(cancellation.Token, Assert.IsAssignableFrom(failure).CancellationToken); + } + else + { + Assert.Same(missingRow, failure); + } + } + + [Fact] + public async Task Read_CanceledMember_PropagatesWithoutStartingHeartbeat() + { + var fake = new ZooKeeperNativeFake(); + fake.Nodes["/"] = new([], 17); + var address = CreateSiloAddress(); + using var nativeCancellation = new CancellationTokenSource(); + nativeCancellation.Cancel(); + fake.BeforeRead = path => path == ZooKeeperNativeFake.RowPath(address) + ? Task.FromCanceled(nativeCancellation.Token) + : Task.CompletedTask; + + var failure = await Assert.ThrowsAnyAsync(() => Read(fake, address)); + + Assert.Equal(nativeCancellation.Token, failure.CancellationToken); + Assert.Equal( + new[] { "sync /", "read /", "read " + ZooKeeperNativeFake.RowPath(address) }, + fake.Calls); + } + + [Fact] + public async Task Read_CancellationBeforeHeartbeat_AwaitsTheStartedMemberRequest() + { + var (fake, entry) = await CreateNativeTable(); + using var cancellation = new CancellationTokenSource(); + var releaseMember = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + fake.BeforeRead = async path => + { + if (path == ZooKeeperNativeFake.RowPath(entry.SiloAddress)) + { + cancellation.Cancel(); + await releaseMember.Task.WaitAsync(TestContext.Current.CancellationToken); + } + }; + + var read = ZooKeeperBasedMembershipTable.ReadCoreAsync(fake.Operations, entry.SiloAddress, cancellation.Token); + var completion = Record.ExceptionAsync(() => read); + try + { + Assert.False(read.IsCompleted); + Assert.Equal( + new[] { "sync /", "read /", "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress) }, + fake.Calls); + } + finally + { + releaseMember.TrySetResult(); + await completion; + } + + Assert.Equal(cancellation.Token, + Assert.IsAssignableFrom(await completion).CancellationToken); + } + + [Theory] + [InlineData(2)] + [InlineData(128)] + public async Task ReadAll_UpdateDuringLaterRow_RetriesTheWholeSnapshot(int rowCount) + { + var (fake, first) = await CreateNativeTable(); + var others = Enumerable.Range(1, rowCount - 1).Select(index => CreateTimedEntry(12345 + index)).ToArray(); + for (var index = 0; index < others.Length; index++) + { + Assert.True(await Insert(fake, others[index], index + 1)); + } + + var last = others[^1]; + fake.Calls.Clear(); + fake.AfterRead = async path => + { + if (path == ZooKeeperNativeFake.RowPath(last.SiloAddress)) + { + fake.AfterRead = null; + first.Status = SiloStatus.Dead; + Assert.True(await ZooKeeperBasedMembershipTable.UpdateRowCoreAsync( + fake.Operations, first, "0", + new TableVersion(rowCount + 1, rowCount.ToString(CultureInfo.InvariantCulture)), + TestContext.Current.CancellationToken)); + } + }; + + var result = await Read(fake); + + Assert.Equal(rowCount + 1, result.Version.Version); + Assert.Equal((rowCount + 1).ToString(CultureInfo.InvariantCulture), result.Version.VersionEtag); + Assert.Equal(SiloStatus.Dead, result.TryGet(first.SiloAddress)!.Item1.Status); + Assert.All(others, entry => Assert.Equal(SiloStatus.Active, result.TryGet(entry.SiloAddress)!.Item1.Status)); + Assert.Equal(rowCount, result.Members.Count); + Assert.Equal(1, fake.Calls.Count(call => call == "sync /")); + Assert.Equal(2, fake.Calls.Count(call => call == "children /")); + Assert.Equal(2, fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress))); + Assert.All(others, entry => Assert.Equal(2, + fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)))); + } + + [Fact] + public async Task ReadAll_CancellationStopsBeforeRequestingTheNextRow() + { + var (fake, first) = await CreateNativeTable(); + var second = CreateTimedEntry(12346); + Assert.True(await Insert(fake, second, 1)); + fake.Calls.Clear(); + using var cancellation = new CancellationTokenSource(); + fake.AfterRead = path => + { + if (path == ZooKeeperNativeFake.HeartbeatPath(first.SiloAddress)) + { + cancellation.Cancel(); + } + + return Task.CompletedTask; + }; + + var exception = await Assert.ThrowsAnyAsync(() => + ZooKeeperBasedMembershipTable.ReadCoreAsync(fake.Operations, null, cancellation.Token)); + + Assert.Equal(cancellation.Token, exception.CancellationToken); + Assert.Equal( + new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), + "read " + ZooKeeperNativeFake.HeartbeatPath(first.SiloAddress) }, + fake.Calls); + } + + [Fact] + public async Task Read_MissingRowAndAuthorizationFailure_PropagatesAuthorizationFailure() { var (fake, entry) = await CreateNativeTable(); var second = CreateTimedEntry(12346); diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperNativeDiagnostics.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperNativeDiagnostics.cs new file mode 100644 index 00000000000..692944b9b80 --- /dev/null +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperNativeDiagnostics.cs @@ -0,0 +1,98 @@ +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Diagnostics.Tracing; +using org.apache.utils; +using org.apache.zookeeper; + +namespace UnitTests.MembershipTests; + +internal sealed class NativeSocketDiagnostics(Action write) : EventListener +{ + private const int MaxLoggedEvents = 256; + private readonly Action _write = write; + private int _eventCount; + + protected override void OnEventSourceCreated(EventSource eventSource) + { + if (eventSource.Name == "Private.InternalDiagnostics.System.Net.Sockets") + { + // Capture native completion errors before the SDK assigns its own SocketError. + EnableEvents(eventSource, EventLevel.Error, (EventKeywords)1); + _write($"{DateTime.UtcNow:O} [NativeSockets#{GetHashCode()}] Error listener enabled"); + } + } + + protected override void OnEventWritten(EventWrittenEventArgs eventData) + { + if (eventData.EventName is "ErrorMessage" or "EventSourceMessage") + { + var count = Interlocked.Increment(ref _eventCount); + if (count <= MaxLoggedEvents) + { + _write($"{DateTime.UtcNow:O} [NativeSockets#{GetHashCode()}] {eventData.EventName}: {string.Join("; ", eventData.Payload!)}"); + } + else if (count == MaxLoggedEvents + 1) + { + _write($"{DateTime.UtcNow:O} [NativeSockets#{GetHashCode()}] Event limit reached; subsequent event details are omitted"); + } + } + } + + public override void Dispose() + { + base.Dispose(); + _write($"{DateTime.UtcNow:O} [NativeSockets#{GetHashCode()}] Listener disposed; observed-events={Volatile.Read(ref _eventCount)}; detail-limit={MaxLoggedEvents}"); + } +} + +internal sealed class ZooKeeperNativeDiagnostics : ILogConsumer, IDisposable +{ + private const int MaxSdkMessages = 256; + private readonly TraceLevel _previousLevel = ZooKeeper.LogLevel; + private readonly bool _previousTrace = ZooKeeper.LogToTrace; + private readonly ILogConsumer? _previousConsumer = ZooKeeper.CustomLogConsumer; + private readonly ConcurrentQueue _messages = new(); + private readonly NativeSocketDiagnostics _sockets; + private int _sdkMessageCount; + + internal ZooKeeperNativeDiagnostics() + { + _sockets = new NativeSocketDiagnostics(_messages.Enqueue); + ZooKeeper.LogLevel = TraceLevel.Info; + ZooKeeper.LogToTrace = false; + ZooKeeper.CustomLogConsumer = this; + } + + public void Log(TraceLevel severity, string className, string message, Exception exception) + { + if (exception is not null || severity <= TraceLevel.Warning) + { + var record = $"{DateTime.UtcNow:O} [{className}] {severity}: {message}{Environment.NewLine}{exception}"; + Console.Error.WriteLine(record); + var count = Interlocked.Increment(ref _sdkMessageCount); + if (count <= MaxSdkMessages) + { + _messages.Enqueue(record); + } + else if (count == MaxSdkMessages + 1) + { + _messages.Enqueue($"{DateTime.UtcNow:O} SDK message limit reached; subsequent message details are omitted"); + } + } + } + + public void Dispose() + { + ZooKeeper.CustomLogConsumer = _previousConsumer; + ZooKeeper.LogToTrace = _previousTrace; + ZooKeeper.LogLevel = _previousLevel; + _sockets.Dispose(); + _messages.Enqueue($"SDK messages observed={Volatile.Read(ref _sdkMessageCount)}; detail-limit={MaxSdkMessages}"); + } + + internal async Task WriteAsync(string path) + { + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + await File.WriteAllLinesAsync(path, _messages); + } +} diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs new file mode 100644 index 00000000000..5e0a548a6d4 --- /dev/null +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs @@ -0,0 +1,125 @@ +using System.Collections.Concurrent; +using System.Diagnostics; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Orleans.Clustering.TestKit; +using Orleans.Configuration; +using Orleans.Runtime.Membership; +using Orleans.TestingHost.Utils; +using org.apache.zookeeper; +using TestExtensions; +using Tester.ZooKeeperUtils; +using Xunit; + +namespace UnitTests.MembershipTests; + +[Collection(TestEnvironmentFixture.DefaultCollection)] +[TestCategory("Membership"), TestCategory("ZooKeeper")] +[TestSuite("Functional"), TestProvider("ZooKeeper"), TestArea("Membership")] +public sealed class ZooKeeperReadResilienceTests : IAsyncLifetime +{ + private readonly ZooKeeperNativeDiagnostics _diagnostics = new(); + private readonly ConcurrentBag _sessions = []; + private readonly string _socketLog = Path.Combine(AppContext.BaseDirectory, "TestResults", $"zookeeper-sockets-{Guid.NewGuid():N}.log"); + private readonly ILoggerFactory _loggerFactory = TestingUtils.CreateDefaultLoggerFactory( + $"zookeeper-reads-{Guid.NewGuid():N}.log", new LoggerFilterOptions()); + private string _connectionString = null!; + + public async ValueTask InitializeAsync() + { + Assert.True(await ZookeeperTestUtils.EnsureZooKeeperAsync(TestContext.Current.CancellationToken), + "ZooKeeper resilience tests require the configured ZooKeeper service."); + _connectionString = TestDefaultConfiguration.ZooKeeperConnectionString!; + } + + public async ValueTask DisposeAsync() + { + try + { + // A canceled caller wait can finish before its native requests and close. + await Task.WhenAll(_sessions.Select(session => session.Completion)); + } + finally + { + _diagnostics.Dispose(); + await _diagnostics.WriteAsync(_socketLog); + _loggerFactory.Dispose(); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task MembershipTable_ZooKeeper_RepeatedSnapshotReadCompatibility(bool pointRead) + { + for (var iteration = 0; iteration < 3; iteration++) + { + var started = Stopwatch.GetTimestamp(); + await CreateFixture().RunAsync(async (fixture, cancellationToken) => + { + var runner = new MembershipTableTestRunner(fixture, seed: 17, concurrencyRowCount: 128); + if (pointRead) + { + await runner.ConcurrentReadRow_ReturnsOnlyAtomicCommittedViews(cancellationToken); + } + else + { + await runner.ConcurrentReadAll_ReturnsOnlyAtomicCommittedViews(cancellationToken); + } + + var readStarted = Stopwatch.GetTimestamp(); + var snapshot = await fixture.First.ReadAllAsync(cancellationToken); + TestContext.Current.TestOutputHelper?.WriteLine( + $"Stable snapshot rows={snapshot.Members.Count}; elapsed={Stopwatch.GetElapsedTime(readStarted)}"); + }, TestContext.Current.CancellationToken); + TestContext.Current.TestOutputHelper?.WriteLine( + $"Snapshot compatibility pass {iteration + 1}/3; pointRead={pointRead}; elapsed={Stopwatch.GetElapsedTime(started)}"); + } + } + + [Fact] + public Task MembershipTable_ZooKeeper_DeletionProbe_PreservesOriginalScope() => + CreateFixture().RunAsync((fixture, cancellationToken) => + new MembershipTableTestRunner(fixture, seed: 17) + .DeleteMembershipTableEntries_DeletesOwnClusterAndPreservesOtherCluster(cancellationToken), + TestContext.Current.CancellationToken); + + private MembershipTableTestFixture CreateFixture() => + new(nameof(ZooKeeperReadResilienceTests), (serviceId, clusterId, cancellationToken) => + { + cancellationToken.ThrowIfCancellationRequested(); + var logger = _loggerFactory.CreateLogger(); + var sessions = new ConcurrentBag(); + var table = new ZooKeeperBasedMembershipTable( + logger, + Options.Create(new ZooKeeperClusteringSiloOptions { ConnectionString = _connectionString }), + Options.Create(new ClusterOptions { ServiceId = serviceId, ClusterId = clusterId }), + readOnly => + { + var session = ZooKeeperBasedMembershipTable.CreateSession( + _connectionString + "/" + clusterId, new ZooKeeperWatcher(logger), readOnly); + sessions.Add(session); + _sessions.Add(session); + return session; + }, + null); + return ValueTask.FromResult(new MembershipTableTestHandle(table, + () => new ValueTask(Task.WhenAll(sessions.Select(session => session.Completion))))); + }, IsConformanceClusterDeletedAsync); + + private async ValueTask IsConformanceClusterDeletedAsync(string clusterId, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + return await ZooKeeper.Using(_connectionString, 10_000, new ConformanceWatcher(), async client => + { + await client.sync("/"); + cancellationToken.ThrowIfCancellationRequested(); + return await client.existsAsync("/" + clusterId, false) is null; + }); + } + + private sealed class ConformanceWatcher : Watcher + { + public override Task process(WatchedEvent @event) => Task.CompletedTask; + } +} diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs new file mode 100644 index 00000000000..861171cb3bf --- /dev/null +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -0,0 +1,701 @@ +using System.Collections.Concurrent; +using System.Globalization; +using System.Net; +using System.Threading.Channels; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.Time.Testing; +using Orleans.Configuration; +using Orleans.Runtime; +using Orleans.Runtime.Membership; +using org.apache.zookeeper; +using Polly; +using TestExtensions; +using Xunit; + +namespace UnitTests.MembershipTests; + +[TestCategory("Membership"), TestCategory("ZooKeeper")] +[TestSuite("BVT"), TestProvider("ZooKeeper"), TestArea("Membership")] +public sealed class ZooKeeperReadRetryTests +{ + [Theory] + [InlineData("sync", false)] + [InlineData("children", false)] + [InlineData("before", true)] + [InlineData("member", false)] + [InlineData("heartbeat", false)] + [InlineData("after", false)] + public async Task Read_ConnectionLossThenSuccess_RetriesOnlyFailedNativeRequest(string boundary, bool point) + { + var harness = await Harness.CreateAsync(); + var key = boundary switch + { + "sync" => "Sync /", + "children" => "GetChildren /", + "before" or "after" => "GetData /", + "member" => "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress), + "heartbeat" => "GetData " + ZooKeeperNativeFake.HeartbeatPath(harness.Entries[0].SiloAddress), + _ => throw new ArgumentOutOfRangeException(nameof(boundary)) + }; + var failure = new KeeperException.ConnectionLossException(); + var failed = false; + harness.BeforeRequest = request => + { + if (request == key && !failed) + { + failed = true; + throw failure; + } + return Task.CompletedTask; + }; + + var read = harness.Read(point, TestContext.Current.CancellationToken); + await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(250)); + var result = await read; + + harness.AssertSnapshot(result, point ? [harness.Entries[0]] : harness.Entries, 2); + var expected = harness.ExpectedReadCalls(point).GroupBy(value => value).ToDictionary(group => group.Key, group => group.Count()); + expected[key]++; + Assert.Equal(expected.OrderBy(pair => pair.Key), harness.CountCalls().OrderBy(pair => pair.Key)); + var warning = Assert.Single(harness.Logger.Warnings); + Assert.Same(failure, warning.Exception); + Assert.Equal(key.Split(' ')[0], warning.Values["Operation"]); + Assert.Equal(1, warning.Values["Retry"]); + Assert.Equal(250d, warning.Values["DelayMilliseconds"]); + harness.AssertOneOwner(readOnly: true); + } + + [Fact] + public async Task ReadRetry_Exhaustion_PreservesFinalExceptionAndBackoff() + { + var harness = await Harness.CreateAsync(); + var failures = Enumerable.Range(0, 5).Select(_ => new KeeperException.ConnectionLossException()).ToArray(); + var times = new List(); + harness.BeforeRequest = _ => + { + times.Add(harness.Clock.GetUtcNow()); + throw failures[times.Count - 1]; + }; + var read = harness.Read(TestContext.Current.CancellationToken); + var completion = Record.ExceptionAsync(() => read); + foreach (var delay in new[] { 250, 500, 1000, 2000 }) + { + var timer = await harness.Clock.NextTimerAsync(); + Assert.Equal(TimeSpan.FromMilliseconds(delay), timer); + var attempts = harness.Calls.Count; + harness.Clock.Advance(timer - TimeSpan.FromMilliseconds(1)); + Assert.Equal(attempts, harness.Calls.Count); + harness.Clock.Advance(TimeSpan.FromMilliseconds(1)); + } + + Assert.Same(failures[^1], await completion); + Assert.Equal(new[] { 0d, 250d, 750d, 1750d, 3750d }, times.Select(time => (time - times[0]).TotalMilliseconds)); + Assert.Equal(Enumerable.Repeat("Sync /", 5), harness.Calls); + Assert.Equal(failures.Take(4), harness.Logger.Warnings.Select(warning => warning.Exception)); + Assert.Equal(new[] { 1, 2, 3, 4 }, harness.Logger.Warnings.Select(warning => (int)warning.Values["Retry"]!)); + Assert.All(harness.Logger.Warnings, warning => Assert.Equal(4, warning.Values["MaxRetries"])); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData("authorization")] + [InlineData("session")] + [InlineData("missing")] + [InlineData("version")] + [InlineData("cancellation")] + [InlineData("ordinary")] + public async Task ReadRetry_IneligibleFailure_PropagatesAfterOneAttempt(string kind) + { + var harness = await Harness.CreateAsync(); + Exception failure = kind switch + { + "authorization" => new KeeperException.NoAuthException(), + "session" => new KeeperException.SessionExpiredException(), + "missing" => new KeeperException.NoNodeException("/"), + "version" => new KeeperException.BadVersionException("/"), + "cancellation" => new OperationCanceledException(), + "ordinary" => new InvalidOperationException("native failure"), + _ => throw new ArgumentOutOfRangeException(nameof(kind)) + }; + harness.BeforeRequest = _ => Task.FromException(failure); + + Assert.Same(failure, await Record.ExceptionAsync(() => harness.Read(TestContext.Current.CancellationToken))); + Assert.Equal("Sync /", Assert.Single(harness.Calls)); + Assert.Empty(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ReadOwner_PreCanceled_DoesNotCreateSession(bool point) + { + var harness = await Harness.CreateAsync(); + using var cancellation = new CancellationTokenSource(); + cancellation.Cancel(); + + var failure = await Assert.ThrowsAnyAsync(() => harness.Read(point, cancellation.Token)); + + Assert.Equal(cancellation.Token, failure.CancellationToken); + Assert.Empty(harness.Sessions); + Assert.Empty(harness.Calls); + Assert.Equal(0, harness.CloseCount); + } + + [Fact] + public async Task ReadRetry_CancellationDuringDelay_StopsAdmission() + { + var harness = await Harness.CreateAsync(); + using var cancellation = new CancellationTokenSource(); + harness.BeforeRequest = _ => Task.FromException(new KeeperException.ConnectionLossException()); + var read = harness.Read(cancellationToken: cancellation.Token); + Assert.Equal(TimeSpan.FromMilliseconds(250), await harness.Clock.NextTimerAsync()); + + cancellation.Cancel(); + + var failure = await Assert.ThrowsAnyAsync(() => read); + Assert.Equal(cancellation.Token, failure.CancellationToken); + await Assert.ThrowsAnyAsync(() => Assert.Single(harness.Sessions).Completion); + harness.Clock.Advance(TimeSpan.FromDays(1)); + Assert.Equal("Sync /", Assert.Single(harness.Calls)); + Assert.Single(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: true); + } + + [Fact] + public async Task ReadOwner_CanceledCaller_JoinsNativeTasksBeforeClose() + { + var harness = await Harness.CreateAsync(); + using var cancellation = new CancellationTokenSource(); + var first = Gate(); + var second = Gate(); + var firstCompleted = Gate(); + var closeStarted = Gate(); + var releaseClose = Gate(); + var firstKey = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); + var secondKey = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[1].SiloAddress); + harness.BeforeRequest = request => request == firstKey ? first.Task : request == secondKey ? second.Task : Task.CompletedTask; + harness.AfterRequest = request => + { + if (request == firstKey) + firstCompleted.SetResult(); + }; + harness.Close = () => + { + closeStarted.SetResult(); + return releaseClose.Task; + }; + + var read = harness.Read(cancellationToken: cancellation.Token); + var owner = Assert.Single(harness.Sessions); + try + { + Assert.Equal(new[] { "Sync /", "GetChildren /", firstKey, secondKey }, harness.Calls); + cancellation.Cancel(); + var failure = await Assert.ThrowsAnyAsync(() => read); + Assert.Equal(cancellation.Token, failure.CancellationToken); + Assert.False(owner.Completion.IsCompleted); + Assert.False(closeStarted.Task.IsCompleted); + first.SetResult(); + await firstCompleted.Task.WaitAsync(TestContext.Current.CancellationToken); + Assert.False(owner.Completion.IsCompleted); + Assert.False(closeStarted.Task.IsCompleted); + second.SetResult(); + await closeStarted.Task.WaitAsync(TestContext.Current.CancellationToken); + Assert.False(owner.Completion.IsCompleted); + } + finally + { + first.TrySetResult(); + second.TrySetResult(); + releaseClose.TrySetResult(); + await Record.ExceptionAsync(() => owner.Completion); + } + + var ownedFailure = await Assert.ThrowsAnyAsync(() => owner.Completion); + Assert.Equal(cancellation.Token, ownedFailure.CancellationToken); + Assert.Equal(new[] { "Sync /", "GetChildren /", firstKey, secondKey }, harness.Calls); + harness.AssertOneOwner(readOnly: true); + } + + [Fact] + public async Task ReadOwner_CloseCompletesBeforeResult() + { + var harness = await Harness.CreateAsync(); + var close = Gate(); + harness.Close = () => close.Task; + var read = harness.Read(TestContext.Current.CancellationToken); + try + { + Assert.Equal(harness.ExpectedReadCalls(), harness.Calls); + Assert.Equal(1, harness.CloseCount); + Assert.False(read.IsCompleted); + Assert.False(Assert.Single(harness.Sessions).Completion.IsCompleted); + } + finally + { + close.TrySetResult(); + } + harness.AssertSnapshot(await read, harness.Entries, 2); + harness.AssertOneOwner(readOnly: true); + } + + [Fact] + public async Task ReadOwner_CloseFailureAfterSuccess_DoesNotReplay() + { + var harness = await Harness.CreateAsync(); + var failure = new KeeperException.ConnectionLossException(); + harness.Close = () => Task.FromException(failure); + + Assert.Same(failure, await Record.ExceptionAsync(() => harness.Read(TestContext.Current.CancellationToken))); + + Assert.Equal(harness.ExpectedReadCalls(), harness.Calls); + Assert.Empty(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Read_ConnectionLossDuringConcurrentMutation_RefencesWholeSnapshot(bool point, bool cleanup) + { + var harness = await Harness.CreateAsync(); + var entry = harness.Entries[0]; + var heartbeat = "GetData " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress); + var failed = false; + harness.BeforeRequest = request => + { + if (request == heartbeat && !failed) + { + failed = true; + throw new KeeperException.ConnectionLossException(); + } + return Task.CompletedTask; + }; + var read = harness.Read(point, TestContext.Current.CancellationToken); + var delay = await harness.Clock.NextTimerAsync(); + if (cleanup) + { + harness.Fake.Nodes.Remove(ZooKeeperNativeFake.RowPath(entry.SiloAddress)); + harness.Fake.Nodes.Remove(ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress)); + var root = harness.Fake.Nodes["/"]; + harness.Fake.Nodes["/"] = root with { ChildrenVersion = root.ChildrenVersion + 1 }; + } + else + { + entry.Status = SiloStatus.Dead; + Assert.True(await ZooKeeperBasedMembershipTable.UpdateRowCoreAsync( + harness.Fake.Operations, entry, "0", new TableVersion(3, "2"), TestContext.Current.CancellationToken)); + } + harness.Clock.Advance(delay); + + var result = await read; + var entries = cleanup ? harness.Entries.Skip(1).ToArray() : harness.Entries; + harness.AssertSnapshot(result, point ? entries.Where(value => value.SiloAddress.Equals(entry.SiloAddress)) : entries, + cleanup ? 2 : 3); + Assert.Equal(1, harness.Calls.Count(call => call == "Sync /")); + Assert.Equal(point ? 4 : 2, harness.Calls.Count(call => call == (point ? "GetData /" : "GetChildren /"))); + Assert.Equal(cleanup && !point ? 1 : 2, + harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(9)] + [InlineData(128)] + public async Task Read_RetryingOneRow_PreservesSuccessfulSiblings(int rowCount) + { + var harness = await Harness.CreateAsync(rowCount); + var key = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); + var failed = false; + harness.BeforeRequest = request => + { + if (request == key && !failed) + { + failed = true; + throw new KeeperException.ConnectionLossException(); + } + return Task.CompletedTask; + }; + var read = harness.Read(TestContext.Current.CancellationToken); + Assert.False(read.IsCompleted); + Assert.All(harness.Entries.Skip(1), entry => + Assert.Contains("GetData " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress), harness.Calls)); + await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(250)); + + harness.AssertSnapshot(await read, harness.Entries, rowCount); + var expected = harness.ExpectedReadCalls().ToDictionary(call => call, _ => 1); + expected[key]++; + Assert.Equal(expected.OrderBy(pair => pair.Key), harness.CountCalls().OrderBy(pair => pair.Key)); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(9, false)] + [InlineData(9, true)] + [InlineData(128, false)] + [InlineData(128, true)] + public async Task ReadOwner_StartsEveryRowBeforeAwaitingNativeCompletion(int rowCount, bool cancel) + { + var harness = await Harness.CreateAsync(rowCount); + using var cancellation = new CancellationTokenSource(); + var release = Gate(); + harness.BeforeRequest = request => request.StartsWith("GetData ", StringComparison.Ordinal) && request != "GetData /" + && !request.EndsWith("/IAmAlive", StringComparison.Ordinal) ? release.Task : Task.CompletedTask; + var read = harness.Read(cancellationToken: cancellation.Token); + var owner = Assert.Single(harness.Sessions); + try + { + Assert.Equal(new[] { "Sync /", "GetChildren /" }.Concat( + harness.Entries.Select(entry => "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))), harness.Calls); + if (cancel) + { + cancellation.Cancel(); + await Assert.ThrowsAnyAsync(() => read); + } + Assert.False(owner.Completion.IsCompleted); + Assert.Equal(0, harness.CloseCount); + } + finally + { + release.TrySetResult(); + await Record.ExceptionAsync(() => owner.Completion); + } + if (cancel) + { + var failure = await Assert.ThrowsAnyAsync(() => owner.Completion); + Assert.Equal(cancellation.Token, failure.CancellationToken); + Assert.Equal(2 + rowCount, harness.Calls.Count); + } + else + { + harness.AssertSnapshot(await read, harness.Entries, rowCount); + Assert.Equal(3 + rowCount * 2, harness.Calls.Count); + } + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Read_MissingRow_PreservesOtherNativeFailureAfterJoining(bool connectionLoss) + { + var harness = await Harness.CreateAsync(); + var release = Gate(); + Exception failure = connectionLoss ? new KeeperException.ConnectionLossException() : new KeeperException.NoAuthException(); + var first = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); + var second = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[1].SiloAddress); + harness.BeforeRequest = async request => + { + if (request == first) + throw new KeeperException.NoNodeException(ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress)); + if (request == second) + { + await release.Task; + throw failure; + } + }; + var read = harness.Read(TestContext.Current.CancellationToken); + var completion = Record.ExceptionAsync(() => read); + try + { + Assert.Contains(second, harness.Calls); + Assert.False(read.IsCompleted); + Assert.Equal(0, harness.CloseCount); + } + finally + { + release.TrySetResult(); + } + if (connectionLoss) + { + foreach (var delay in new[] { 250, 500, 1000, 2000 }) + await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(delay)); + } + Assert.Same(failure, await completion); + Assert.DoesNotContain("GetData /", harness.Calls); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task GetGateways_ConnectionLoss_UsesRetriedSnapshot(bool exhaust) + { + var harness = await Harness.CreateAsync(3); + harness.Entries[1].ProxyPort = 0; + harness.Entries[2].Status = SiloStatus.Dead; + foreach (var entry in harness.Entries) + { + var path = ZooKeeperNativeFake.RowPath(entry.SiloAddress); + harness.Fake.Nodes[path] = harness.Fake.Nodes[path] with { Data = ZooKeeperBasedMembershipTable.Serialize(entry) }; + } + var failure = new KeeperException.ConnectionLossException(); + var failures = 0; + harness.BeforeRequest = request => + { + if (request == "Sync /" && (exhaust || failures++ == 0)) + throw failure; + return Task.CompletedTask; + }; + var provider = new ZooKeeperGatewayListProvider( + NullLogger.Instance, + Options.Create(new ZooKeeperGatewayListProviderOptions { ConnectionString = "unused.invalid" }), + Options.Create(new GatewayOptions()), + Options.Create(new ClusterOptions { ClusterId = "test" }), + () => harness.CreateSession(true), + harness.Pipeline); + var read = provider.GetGateways(); + var completion = Record.ExceptionAsync(() => read); + foreach (var delay in exhaust ? new[] { 250, 500, 1000, 2000 } : [250]) + await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(delay)); + if (exhaust) + { + Assert.Same(failure, await completion); + } + else + { + Assert.Null(await completion); + var entry = harness.Entries[0]; + Assert.Equal(SiloAddress.New(entry.SiloAddress.Endpoint.Address, entry.ProxyPort, entry.SiloAddress.Generation).ToGatewayUri(), + Assert.Single(await read)); + } + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task ConditionalWrite_CommitOrCloseLoss_PropagatesWithoutReplay(bool update, bool closeFailure) + { + var harness = await Harness.CreateAsync(1); + var entry = update ? harness.Entries[0] : Harness.Entry(1); + entry.Status = SiloStatus.Dead; + var failure = new KeeperException.ConnectionLossException(); + if (closeFailure) + harness.Close = () => Task.FromException(failure); + else + harness.AfterMulti = () => Task.FromException(failure); + + var actual = await Record.ExceptionAsync(() => update + ? harness.Table.UpdateRowAsync(entry, "0", new TableVersion(2, "1"), TestContext.Current.CancellationToken) + : harness.Table.InsertRowAsync(entry, new TableVersion(2, "1"), TestContext.Current.CancellationToken)); + + Assert.Same(failure, actual); + Assert.Equal("Multi", Assert.Single(harness.Calls)); + Assert.Single(harness.Fake.Transactions); + Assert.Equal(2, harness.Fake.Nodes["/"].Version); + var row = harness.Fake.Nodes[ZooKeeperNativeFake.RowPath(entry.SiloAddress)]; + Assert.Equal(update ? 1 : 0, row.Version); + Assert.Equal(ZooKeeperBasedMembershipTable.Serialize(entry), row.Data); + Assert.Equal(update ? 3 : 5, harness.Fake.Nodes.Count); + Assert.Empty(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: false); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConditionalWrite_KnownConflict_PreservesFalseResult(bool update) + { + var harness = await Harness.CreateAsync(1); + var before = harness.Fake.Nodes.ToDictionary(pair => pair.Key, pair => pair.Value); + var result = update + ? await harness.Table.UpdateRowAsync(harness.Entries[0], "99", new TableVersion(2, "1"), TestContext.Current.CancellationToken) + : await harness.Table.InsertRowAsync(harness.Entries[0], new TableVersion(2, "1"), TestContext.Current.CancellationToken); + + Assert.False(result); + Assert.Equal(before, harness.Fake.Nodes); + Assert.Equal("Multi", Assert.Single(harness.Calls)); + Assert.Empty(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: false); + } + + [Fact] + public async Task ReadDecorator_PreservesMutationDelegates() + { + var harness = await Harness.CreateAsync(); + var native = harness.Fake.Operations; + var wrapped = ZooKeeperReadRetryPolicy.Wrap(native, harness.Pipeline, CancellationToken.None); + + Assert.Same(native.Multi, wrapped.Multi); + Assert.Same(native.SetData, wrapped.SetData); + } + + private static TaskCompletionSource Gate() => new(TaskCreationOptions.RunContinuationsAsynchronously); + + private sealed class Harness + { + internal ZooKeeperNativeFake Fake { get; } = new(); + internal RetryClock Clock { get; } = new(); + internal RecordingLogger Logger { get; } = new(); + internal ConcurrentQueue Calls { get; } = new(); + internal List Sessions { get; } = []; + internal List ReadOnly { get; } = []; + internal Func? BeforeRequest { get; set; } + internal Action? AfterRequest { get; set; } + internal Func Close { get; set; } = () => Task.CompletedTask; + internal Func AfterMulti { get; set; } = () => Task.CompletedTask; + internal int CloseCount; + internal MembershipEntry[] Entries { get; private set; } = []; + internal ResiliencePipeline Pipeline { get; } + internal ZooKeeperBasedMembershipTable Table { get; } + + private Harness() + { + Pipeline = ZooKeeperReadRetryPolicy.CreatePipeline(Logger, Clock); + Table = new ZooKeeperBasedMembershipTable(NullLogger.Instance, + Options.Create(new ZooKeeperClusteringSiloOptions { ConnectionString = "unused.invalid" }), + Options.Create(new ClusterOptions { ClusterId = "test" }), CreateSession, Pipeline); + } + + internal static async Task CreateAsync(int rowCount = 2) + { + var result = new Harness { Entries = Enumerable.Range(0, rowCount).Select(Entry).ToArray() }; + for (var index = 0; index < rowCount; index++) + { + Assert.True(await ZooKeeperBasedMembershipTable.InsertRowCoreAsync(result.Fake.Operations, + result.Entries[index], new TableVersion(index + 1, index.ToString(CultureInfo.InvariantCulture)), + TestContext.Current.CancellationToken)); + } + result.Fake.Calls.Clear(); + result.Fake.Transactions.Clear(); + return result; + } + + internal static MembershipEntry Entry(int index) => new() + { + SiloAddress = SiloAddress.New(new IPEndPoint(IPAddress.Loopback, 11111), 12345 + index), + HostName = "host-a", + SiloName = "silo-a", + Status = SiloStatus.Active, + ProxyPort = 30000 + index, + StartTime = DateTime.UnixEpoch, + IAmAliveTime = DateTime.UnixEpoch.AddDays(1) + }; + + internal ZooKeeperSession CreateSession(bool readOnly) + { + ReadOnly.Add(readOnly); + var native = Fake.Operations; + var operations = new ZooKeeperBasedMembershipTable.NativeOperations( + path => Request("GetData " + path, () => native.GetData(path)), + path => Request("GetChildren " + path, () => native.GetChildren(path)), + path => Request("Sync " + path, async () => { await native.Sync(path); return true; }), + async ops => + { + Calls.Enqueue("Multi"); + await native.Multi(ops); + await AfterMulti(); + }, + native.SetData); + var session = new ZooKeeperSession(operations, () => + { + Interlocked.Increment(ref CloseCount); + return Close(); + }); + Sessions.Add(session); + return session; + } + + private async Task Request(string request, Func> action) + { + Calls.Enqueue(request); + if (BeforeRequest is { } before) + await before(request); + Task native; + lock (Fake.Calls) + native = action(); + var result = await native; + AfterRequest?.Invoke(request); + return result; + } + + internal Task Read(CancellationToken cancellationToken) => Read(false, cancellationToken); + + internal Task Read(bool point, CancellationToken cancellationToken) => + point ? Table.ReadRowAsync(Entries[0].SiloAddress, cancellationToken) : Table.ReadAllAsync(cancellationToken); + + internal IEnumerable ExpectedReadCalls(bool point = false) + { + yield return "Sync /"; + yield return point ? "GetData /" : "GetChildren /"; + foreach (var entry in point ? Entries.Take(1) : Entries) + { + yield return "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress); + yield return "GetData " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress); + } + yield return "GetData /"; + } + + internal Dictionary CountCalls() => + Calls.GroupBy(value => value).ToDictionary(group => group.Key, group => group.Count()); + + internal void AssertSnapshot(MembershipTableData snapshot, IEnumerable expected, int version) + { + Assert.Equal(version, snapshot.Version.Version); + Assert.Equal(version.ToString(CultureInfo.InvariantCulture), snapshot.Version.VersionEtag); + var entries = expected.ToDictionary(entry => entry.SiloAddress); + var actual = snapshot.Members.ToDictionary(row => row.Item1.SiloAddress); + Assert.Equal(entries.Count, actual.Count); + foreach (var (address, entry) in entries) + { + var row = actual[address]; + Assert.Equal(Fake.Nodes[ZooKeeperNativeFake.RowPath(address)].Version.ToString(CultureInfo.InvariantCulture), row.Item2); + Assert.Equal(ZooKeeperBasedMembershipTable.Serialize(entry), ZooKeeperBasedMembershipTable.Serialize(row.Item1)); + } + } + + internal void AssertOneOwner(bool readOnly) + { + Assert.Single(Sessions); + Assert.Equal(readOnly, Assert.Single(ReadOnly)); + Assert.Equal(1, CloseCount); + Assert.True(Sessions[0].Completion.IsCompleted); + } + } + + private sealed class RetryClock : TimeProvider + { + private readonly FakeTimeProvider _time = new(); + private readonly Channel _timers = Channel.CreateUnbounded(); + public override DateTimeOffset GetUtcNow() => _time.GetUtcNow(); + public override long GetTimestamp() => _time.GetTimestamp(); + public override long TimestampFrequency => _time.TimestampFrequency; + + public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) + { + var timer = _time.CreateTimer(callback, state, dueTime, period); + Assert.True(_timers.Writer.TryWrite(dueTime)); + return timer; + } + + internal ValueTask NextTimerAsync() => _timers.Reader.ReadAsync(TestContext.Current.CancellationToken); + internal void Advance(TimeSpan duration) => _time.Advance(duration); + internal async Task AdvanceNextAsync(TimeSpan expected) + { + Assert.Equal(expected, await NextTimerAsync()); + Advance(expected); + } + } + + private sealed class RecordingLogger : ILogger + { + internal sealed record Warning(Exception? Exception, IReadOnlyDictionary Values); + internal ConcurrentQueue Warnings { get; } = new(); + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + Assert.Equal(LogLevel.Warning, logLevel); + var values = Assert.IsAssignableFrom>>(state); + Warnings.Enqueue(new(exception, values.ToDictionary(pair => pair.Key, pair => pair.Value))); + } + } +} From d1b8f966ecaae7ed06e63b80800acc8325ee9c80 Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Sun, 20 Sep 2026 10:31:19 -0700 Subject: [PATCH 02/10] test(zookeeper): retain diagnostics through native teardown --- .../ZooKeeperReadResilienceTests.cs | 43 +++++++++++++++---- .../ZooKeeperReadRetryTests.cs | 27 ++++++++++++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs index 5e0a548a6d4..4b889705b27 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs @@ -8,7 +8,6 @@ using Orleans.TestingHost.Utils; using org.apache.zookeeper; using TestExtensions; -using Tester.ZooKeeperUtils; using Xunit; namespace UnitTests.MembershipTests; @@ -20,6 +19,8 @@ public sealed class ZooKeeperReadResilienceTests : IAsyncLifetime { private readonly ZooKeeperNativeDiagnostics _diagnostics = new(); private readonly ConcurrentBag _sessions = []; + private readonly List _fixtures = []; + private readonly ConcurrentBag _probes = []; private readonly string _socketLog = Path.Combine(AppContext.BaseDirectory, "TestResults", $"zookeeper-sockets-{Guid.NewGuid():N}.log"); private readonly ILoggerFactory _loggerFactory = TestingUtils.CreateDefaultLoggerFactory( $"zookeeper-reads-{Guid.NewGuid():N}.log", new LoggerFilterOptions()); @@ -27,17 +28,24 @@ public sealed class ZooKeeperReadResilienceTests : IAsyncLifetime public async ValueTask InitializeAsync() { - Assert.True(await ZookeeperTestUtils.EnsureZooKeeperAsync(TestContext.Current.CancellationToken), - "ZooKeeper resilience tests require the configured ZooKeeper service."); + Assert.False(string.IsNullOrWhiteSpace(TestDefaultConfiguration.ZooKeeperConnectionString), + "ZooKeeper resilience tests require a configured connection string."); _connectionString = TestDefaultConfiguration.ZooKeeperConnectionString!; + var probe = ZooKeeper.Using(_connectionString, 2000, new ConformanceWatcher(), + async client => await client.existsAsync("/", false) is not null); + _probes.Add(probe); + Assert.True(await probe.WaitAsync(TestContext.Current.CancellationToken), + "ZooKeeper resilience tests require the configured ZooKeeper service."); } public async ValueTask DisposeAsync() { try { - // A canceled caller wait can finish before its native requests and close. - await Task.WhenAll(_sessions.Select(session => session.Completion)); + // Fixture teardown and canceled caller waits can return before native close. + await Task.WhenAll(_fixtures.Select(fixture => DrainFixtureAsync(fixture.DisposeAsync)) + .Concat(_sessions.Select(session => session.Completion)) + .Concat(_probes)); } finally { @@ -84,8 +92,22 @@ public Task MembershipTable_ZooKeeper_DeletionProbe_PreservesOriginalScope() => .DeleteMembershipTableEntries_DeletesOwnClusterAndPreservesOtherCluster(cancellationToken), TestContext.Current.CancellationToken); - private MembershipTableTestFixture CreateFixture() => - new(nameof(ZooKeeperReadResilienceTests), (serviceId, clusterId, cancellationToken) => + internal static async Task DrainFixtureAsync(Func dispose) + { + try + { + await dispose(); + } + catch (TimeoutException exception) when (exception.Data["ClusteringTestKit.CleanupCompletion"] is Task completion) + { + await completion; + throw; + } + } + + private MembershipTableTestFixture CreateFixture() + { + var fixture = new MembershipTableTestFixture(nameof(ZooKeeperReadResilienceTests), (serviceId, clusterId, cancellationToken) => { cancellationToken.ThrowIfCancellationRequested(); var logger = _loggerFactory.CreateLogger(); @@ -106,16 +128,21 @@ private MembershipTableTestFixture CreateFixture() => return ValueTask.FromResult(new MembershipTableTestHandle(table, () => new ValueTask(Task.WhenAll(sessions.Select(session => session.Completion))))); }, IsConformanceClusterDeletedAsync); + _fixtures.Add(fixture); + return fixture; + } private async ValueTask IsConformanceClusterDeletedAsync(string clusterId, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - return await ZooKeeper.Using(_connectionString, 10_000, new ConformanceWatcher(), async client => + var probe = ZooKeeper.Using(_connectionString, 10_000, new ConformanceWatcher(), async client => { await client.sync("/"); cancellationToken.ThrowIfCancellationRequested(); return await client.existsAsync("/" + clusterId, false) is null; }); + _probes.Add(probe); + return await probe; } private sealed class ConformanceWatcher : Watcher diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index 861171cb3bf..f962bf1b2e0 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -528,6 +528,33 @@ public async Task ReadDecorator_PreservesMutationDelegates() Assert.Same(native.SetData, wrapped.SetData); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task NativeFixture_TeardownTimeout_JoinsActualCompletion(bool fail) + { + var nativeCompletion = Gate(); + var timeout = new TimeoutException("fixture teardown wait expired"); + timeout.Data["ClusteringTestKit.CleanupCompletion"] = nativeCompletion.Task; + var nativeFailure = new KeeperException.ConnectionLossException(); + var drain = ZooKeeperReadResilienceTests.DrainFixtureAsync(() => ValueTask.FromException(timeout)); + + try + { + Assert.False(drain.IsCompleted); + } + finally + { + if (fail) + nativeCompletion.SetException(nativeFailure); + else + nativeCompletion.SetResult(); + } + + Assert.Same(fail ? (Exception)nativeFailure : timeout, await Record.ExceptionAsync(() => drain)); + Assert.True(nativeCompletion.Task.IsCompleted); + } + private static TaskCompletionSource Gate() => new(TaskCreationOptions.RunContinuationsAsynchronously); private sealed class Harness From bb3c3197ea2fdf9ff01b00b378c6f401830a257e Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Sun, 20 Sep 2026 10:40:18 -0700 Subject: [PATCH 03/10] test(zookeeper): observe actual legacy operation completion --- .../ZooKeeperBasedMembershipTable.cs | 14 ++- .../ZooKeeperReadResilienceTests.cs | 12 ++- .../ZooKeeperReadRetryTests.cs | 85 +++++++++++++++++++ 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs index e26aa3fad8e..e84b666620e 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs @@ -46,6 +46,7 @@ public partial class ZooKeeperBasedMembershipTable : IMembershipTable private readonly ZooKeeperWatcher watcher; private readonly Func _createSession; private readonly ResiliencePipeline _readRetryPipeline; + private readonly Action? _observeOperation; /// /// The deployment connection string. for eg. "192.168.1.1,192.168.1.2/ClusterId" @@ -85,7 +86,8 @@ internal ZooKeeperBasedMembershipTable( IOptions membershipTableOptions, IOptions clusterOptions, Func? createSession, - ResiliencePipeline? readRetryPipeline) + ResiliencePipeline? readRetryPipeline, + Action? observeOperation = null) { ArgumentNullException.ThrowIfNull(logger); ArgumentNullException.ThrowIfNull(membershipTableOptions); @@ -99,6 +101,7 @@ internal ZooKeeperBasedMembershipTable( deploymentConnectionString = options.ConnectionString + this.clusterPath; _createSession = createSession ?? (readOnly => CreateSession(deploymentConnectionString, watcher, readOnly)); _readRetryPipeline = readRetryPipeline ?? ZooKeeperReadRetryPolicy.CreatePipeline(logger, TimeProvider.System); + _observeOperation = observeOperation; } /// @@ -478,7 +481,7 @@ internal sealed class NativeOperations( internal Func> SetData { get; } = setData; } - private static async Task UsingZookeeper(Func> zkMethod, string deploymentConnectionString, ZooKeeperWatcher watcher, CancellationToken cancellationToken, bool canBeReadOnly = false) + private async Task UsingZookeeper(Func> zkMethod, string deploymentConnectionString, ZooKeeperWatcher watcher, CancellationToken cancellationToken, bool canBeReadOnly = false) { cancellationToken.ThrowIfCancellationRequested(); var operation = ZooKeeper.Using(deploymentConnectionString, ZOOKEEPER_SESSION_TIMEOUT, watcher, zk => @@ -489,13 +492,15 @@ private static async Task UsingZookeeper(Func> z operations => zk.multiAsync(operations), zk.setDataAsync)); }, canBeReadOnly); - return await AwaitOperationAsync(operation, cancellationToken); + return await AwaitOperationAsync(operation, cancellationToken, _observeOperation); } - internal static async Task AwaitOperationAsync(Task operation, CancellationToken cancellationToken) + internal static async Task AwaitOperationAsync( + Task operation, CancellationToken cancellationToken, Action? observeOperation = null) { // ZooKeeperNetEx is tokenless. Keep the client alive until pending requests and // asynchronous disposal finish, observing failures even if the caller stops waiting. + observeOperation?.Invoke(operation); operation.Ignore(); return await operation.WaitAsync(cancellationToken); } @@ -509,6 +514,7 @@ private async Task UsingZookeeper(string connectString, Func zk return zkMethod(zk); }); + _observeOperation?.Invoke(operation); operation.Ignore(); await operation.WaitAsync(cancellationToken); } diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs index 4b889705b27..0c09570f587 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs @@ -20,6 +20,7 @@ public sealed class ZooKeeperReadResilienceTests : IAsyncLifetime private readonly ZooKeeperNativeDiagnostics _diagnostics = new(); private readonly ConcurrentBag _sessions = []; private readonly List _fixtures = []; + private readonly ConcurrentBag _legacyOperations = []; private readonly ConcurrentBag _probes = []; private readonly string _socketLog = Path.Combine(AppContext.BaseDirectory, "TestResults", $"zookeeper-sockets-{Guid.NewGuid():N}.log"); private readonly ILoggerFactory _loggerFactory = TestingUtils.CreateDefaultLoggerFactory( @@ -45,6 +46,7 @@ public async ValueTask DisposeAsync() // Fixture teardown and canceled caller waits can return before native close. await Task.WhenAll(_fixtures.Select(fixture => DrainFixtureAsync(fixture.DisposeAsync)) .Concat(_sessions.Select(session => session.Completion)) + .Concat(_legacyOperations) .Concat(_probes)); } finally @@ -112,6 +114,7 @@ private MembershipTableTestFixture CreateFixture() cancellationToken.ThrowIfCancellationRequested(); var logger = _loggerFactory.CreateLogger(); var sessions = new ConcurrentBag(); + var legacyOperations = new ConcurrentBag(); var table = new ZooKeeperBasedMembershipTable( logger, Options.Create(new ZooKeeperClusteringSiloOptions { ConnectionString = _connectionString }), @@ -124,9 +127,14 @@ private MembershipTableTestFixture CreateFixture() _sessions.Add(session); return session; }, - null); + null, + operation => + { + legacyOperations.Add(operation); + _legacyOperations.Add(operation); + }); return ValueTask.FromResult(new MembershipTableTestHandle(table, - () => new ValueTask(Task.WhenAll(sessions.Select(session => session.Completion))))); + () => new ValueTask(Task.WhenAll(sessions.Select(session => session.Completion).Concat(legacyOperations))))); }, IsConformanceClusterDeletedAsync); _fixtures.Add(fixture); return fixture; diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index f962bf1b2e0..497d9c46af4 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -557,6 +557,91 @@ public async Task NativeFixture_TeardownTimeout_JoinsActualCompletion(bool fail) private static TaskCompletionSource Gate() => new(TaskCreationOptions.RunContinuationsAsynchronously); + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task NativeFixture_CanceledCleanup_PreservesAdmissionAndOwnsClose(bool fail) + { + var harness = await Harness.CreateAsync(); + foreach (var entry in harness.Entries) + { + entry.Status = SiloStatus.Dead; + var path = ZooKeeperNativeFake.RowPath(entry.SiloAddress); + harness.Fake.Nodes[path] = harness.Fake.Nodes[path] with { Data = ZooKeeperBasedMembershipTable.Serialize(entry) }; + } + var first = Gate(); + var second = Gate(); + var firstCompleted = Gate(); + var closeStarted = Gate(); + var releaseClose = Gate(); + var failure = new KeeperException.NoAuthException(); + var firstPath = ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); + var secondPath = ZooKeeperNativeFake.RowPath(harness.Entries[1].SiloAddress); + harness.Fake.BeforeRead = async path => + { + if (path == firstPath) + await first.Task; + if (path == secondPath) + { + await second.Task; + if (fail) + throw failure; + } + }; + harness.Fake.AfterRead = path => + { + if (path == firstPath) + firstCompleted.SetResult(); + return Task.CompletedTask; + }; + using var cancellation = new CancellationTokenSource(); + async Task NativeOperation() + { + try + { + return await ZooKeeperBasedMembershipTable.CleanupCoreAsync( + harness.Fake.Operations, DateTimeOffset.UnixEpoch.AddDays(3), cancellation.Token); + } + finally + { + closeStarted.SetResult(); + await releaseClose.Task; + } + } + var native = NativeOperation(); + Task? observed = null; + var caller = ZooKeeperBasedMembershipTable.AwaitOperationAsync(native, cancellation.Token, operation => observed = operation); + try + { + Assert.Same(native, observed); + Assert.Equal(new[] { "children /", "read " + firstPath, "read " + secondPath }, harness.Fake.Calls); + cancellation.Cancel(); + var canceled = await Assert.ThrowsAnyAsync(() => caller); + Assert.Equal(cancellation.Token, canceled.CancellationToken); + Assert.False(observed!.IsCompleted); + first.SetResult(); + await firstCompleted.Task.WaitAsync(TestContext.Current.CancellationToken); + Assert.False(closeStarted.Task.IsCompleted); + second.SetResult(); + await closeStarted.Task.WaitAsync(TestContext.Current.CancellationToken); + Assert.False(observed.IsCompleted); + } + finally + { + first.TrySetResult(); + second.TrySetResult(); + releaseClose.TrySetResult(); + await Record.ExceptionAsync(() => native); + } + var actual = await Record.ExceptionAsync(() => observed!); + if (fail) + Assert.Same(failure, actual); + else + Assert.Equal(cancellation.Token, Assert.IsAssignableFrom(actual).CancellationToken); + Assert.Empty(harness.Fake.Transactions); + Assert.Equal(new[] { "children /", "read " + firstPath, "read " + secondPath }, harness.Fake.Calls); + } + private sealed class Harness { internal ZooKeeperNativeFake Fake { get; } = new(); From 2af4afe8cd9a48b34cd97d9a6f20f57db1b5780e Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Sun, 20 Sep 2026 10:43:50 -0700 Subject: [PATCH 04/10] test(zookeeper): archive native read retry diagnostics --- .../ZooKeeperReadResilienceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs index 0c09570f587..ec7a8245238 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs @@ -24,7 +24,7 @@ public sealed class ZooKeeperReadResilienceTests : IAsyncLifetime private readonly ConcurrentBag _probes = []; private readonly string _socketLog = Path.Combine(AppContext.BaseDirectory, "TestResults", $"zookeeper-sockets-{Guid.NewGuid():N}.log"); private readonly ILoggerFactory _loggerFactory = TestingUtils.CreateDefaultLoggerFactory( - $"zookeeper-reads-{Guid.NewGuid():N}.log", new LoggerFilterOptions()); + TestingUtils.CreateTraceFileName("zookeeper-reads", Guid.NewGuid().ToString("N")), new LoggerFilterOptions()); private string _connectionString = null!; public async ValueTask InitializeAsync() From e995372a5497c42886456e1ef9c6ae6e2c0bffdd Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Sun, 20 Sep 2026 11:09:15 -0700 Subject: [PATCH 05/10] fix(zookeeper): preserve owner completion and failures --- .../ZooKeeperSession.cs | 42 ++++++-- .../ZooKeeperReadRetryTests.cs | 101 ++++++++++++++++++ 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs index 73b52673286..7499b3a71fa 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs @@ -4,11 +4,22 @@ namespace Orleans.Runtime.Membership; -internal sealed class ZooKeeperSession( - ZooKeeperBasedMembershipTable.NativeOperations operations, - Func close) +internal sealed class ZooKeeperSession { - internal Task Completion { get; private set; } = Task.CompletedTask; + private readonly ZooKeeperBasedMembershipTable.NativeOperations _operations; + private readonly Func _close; + // Bind synchronously; the owned task supplies the completion semantics. + private readonly TaskCompletionSource _completion = new(); + + internal ZooKeeperSession(ZooKeeperBasedMembershipTable.NativeOperations operations, Func close) + { + _operations = operations; + _close = close; + Completion = _completion.Task.Unwrap(); + Completion.Ignore(); + } + + internal Task Completion { get; } internal static Task ExecuteAsync( Func createSession, @@ -18,20 +29,33 @@ internal static Task ExecuteAsync( cancellationToken.ThrowIfCancellationRequested(); var session = createSession(); var completion = session.RunAsync(operation); - session.Completion = completion; + session._completion.SetResult(completion); return ZooKeeperBasedMembershipTable.AwaitOperationAsync(completion, cancellationToken); } private async Task RunAsync(Func> operation) { + T result; try { - return await operation(operations); + result = await operation(_operations); } - finally + catch (Exception primary) { - // The callback joins native requests before this tokenless close. - await close(); + try + { + await _close(); + } + catch (Exception secondary) + { + throw new AggregateException("The ZooKeeper operation and its close both failed.", primary, secondary); + } + + throw; } + + // The callback joins native requests before this tokenless close. + await _close(); + return result; } } diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index 497d9c46af4..a2606e18398 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -256,6 +256,76 @@ public async Task ReadOwner_CloseFailureAfterSuccess_DoesNotReplay() harness.AssertOneOwner(readOnly: true); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ReadOwner_CompletionIsPendingAtPublicationAndSynchronousPrefix(bool checkAtPublication) + { + var harness = await Harness.CreateAsync(); + var release = Gate(); + Task completionAtPublication = null!; + Task snapshotDrain = null!; + harness.OnSessionCreated = session => + { + completionAtPublication = session.Completion; + snapshotDrain = Task.WhenAll(harness.Sessions.Select(owned => owned.Completion)); + if (checkAtPublication) + { + Assert.False(completionAtPublication.IsCompleted); + Assert.False(snapshotDrain.IsCompleted); + } + }; + harness.BeforeRequest = request => + { + if (request == "Sync /") + { + Assert.Same(completionAtPublication, Assert.Single(harness.Sessions).Completion); + Assert.False(completionAtPublication.IsCompleted); + Assert.False(snapshotDrain.IsCompleted); + return release.Task; + } + return Task.CompletedTask; + }; + var read = harness.Read(TestContext.Current.CancellationToken); + try + { + Assert.False(read.IsCompleted); + Assert.False(snapshotDrain.IsCompleted); + Assert.Equal(0, harness.CloseCount); + Assert.Same(completionAtPublication, Assert.Single(harness.Sessions).Completion); + } + finally + { + release.TrySetResult(); + } + + harness.AssertSnapshot(await read, harness.Entries, 2); + await snapshotDrain.WaitAsync(TestContext.Current.CancellationToken); + Assert.Equal(harness.ExpectedReadCalls(), harness.Calls); + Assert.Same(completionAtPublication, Assert.Single(harness.Sessions).Completion); + harness.AssertOneOwner(readOnly: true); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ReadOwner_CallbackAndCloseFailures_PreserveBothExceptions(bool point) + { + var harness = await Harness.CreateAsync(); + var primary = new KeeperException.NoAuthException(); + var secondary = new KeeperException.ConnectionLossException(); + harness.BeforeRequest = _ => Task.FromException(primary); + harness.Close = () => Task.FromException(secondary); + + var actual = await Assert.ThrowsAsync(() => harness.Read(point, TestContext.Current.CancellationToken)); + + Assert.Equal(new Exception[] { primary, secondary }, actual.InnerExceptions); + Assert.Same(actual, await Record.ExceptionAsync(() => Assert.Single(harness.Sessions).Completion)); + Assert.Equal("Sync /", Assert.Single(harness.Calls)); + Assert.Empty(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: true); + } + [Theory] [InlineData(false, false)] [InlineData(false, true)] @@ -517,6 +587,35 @@ public async Task ConditionalWrite_KnownConflict_PreservesFalseResult(bool updat harness.AssertOneOwner(readOnly: false); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConditionalWrite_CallbackAndCloseFailures_PreserveBothExceptions(bool update) + { + var harness = await Harness.CreateAsync(1); + var entry = update ? harness.Entries[0] : Harness.Entry(1); + entry.Status = SiloStatus.Dead; + var primary = new KeeperException.ConnectionLossException(); + var secondary = new KeeperException.ConnectionLossException(); + harness.AfterMulti = () => Task.FromException(primary); + harness.Close = () => Task.FromException(secondary); + + var actual = await Assert.ThrowsAsync(() => update + ? harness.Table.UpdateRowAsync(entry, "0", new TableVersion(2, "1"), TestContext.Current.CancellationToken) + : harness.Table.InsertRowAsync(entry, new TableVersion(2, "1"), TestContext.Current.CancellationToken)); + + Assert.Equal(new Exception[] { primary, secondary }, actual.InnerExceptions); + Assert.Same(actual, await Record.ExceptionAsync(() => Assert.Single(harness.Sessions).Completion)); + Assert.Equal("Multi", Assert.Single(harness.Calls)); + Assert.Single(harness.Fake.Transactions); + Assert.Equal(2, harness.Fake.Nodes["/"].Version); + var row = harness.Fake.Nodes[ZooKeeperNativeFake.RowPath(entry.SiloAddress)]; + Assert.Equal(update ? 1 : 0, row.Version); + Assert.Equal(ZooKeeperBasedMembershipTable.Serialize(entry), row.Data); + Assert.Empty(harness.Logger.Warnings); + harness.AssertOneOwner(readOnly: false); + } + [Fact] public async Task ReadDecorator_PreservesMutationDelegates() { @@ -650,6 +749,7 @@ private sealed class Harness internal ConcurrentQueue Calls { get; } = new(); internal List Sessions { get; } = []; internal List ReadOnly { get; } = []; + internal Action? OnSessionCreated { get; set; } internal Func? BeforeRequest { get; set; } internal Action? AfterRequest { get; set; } internal Func Close { get; set; } = () => Task.CompletedTask; @@ -713,6 +813,7 @@ internal ZooKeeperSession CreateSession(bool readOnly) return Close(); }); Sessions.Add(session); + OnSessionCreated?.Invoke(session); return session; } From b865783dab0f34cce2dddfb9c0b0309cf7945a93 Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Sun, 20 Sep 2026 11:25:49 -0700 Subject: [PATCH 06/10] test(zookeeper): capture primary failure before teardown --- .../ZooKeeperReadResilienceTests.cs | 57 ++++++++++++++----- .../ZooKeeperReadRetryTests.cs | 37 ++++++++++++ 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs index ec7a8245238..18928c70fa3 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadResilienceTests.cs @@ -67,20 +67,26 @@ public async Task MembershipTable_ZooKeeper_RepeatedSnapshotReadCompatibility(bo var started = Stopwatch.GetTimestamp(); await CreateFixture().RunAsync(async (fixture, cancellationToken) => { - var runner = new MembershipTableTestRunner(fixture, seed: 17, concurrencyRowCount: 128); - if (pointRead) + var phase = "concurrent scenario"; + await CapturePrimaryScenarioFailureAsync(async () => { - await runner.ConcurrentReadRow_ReturnsOnlyAtomicCommittedViews(cancellationToken); - } - else - { - await runner.ConcurrentReadAll_ReturnsOnlyAtomicCommittedViews(cancellationToken); - } + var runner = new MembershipTableTestRunner(fixture, seed: 17, concurrencyRowCount: 128); + if (pointRead) + { + await runner.ConcurrentReadRow_ReturnsOnlyAtomicCommittedViews(cancellationToken); + } + else + { + await runner.ConcurrentReadAll_ReturnsOnlyAtomicCommittedViews(cancellationToken); + } - var readStarted = Stopwatch.GetTimestamp(); - var snapshot = await fixture.First.ReadAllAsync(cancellationToken); - TestContext.Current.TestOutputHelper?.WriteLine( - $"Stable snapshot rows={snapshot.Members.Count}; elapsed={Stopwatch.GetElapsedTime(readStarted)}"); + phase = "stable ReadAll"; + var readStarted = Stopwatch.GetTimestamp(); + var snapshot = await fixture.First.ReadAllAsync(cancellationToken); + TestContext.Current.TestOutputHelper?.WriteLine( + $"Stable snapshot rows={snapshot.Members.Count}; elapsed={Stopwatch.GetElapsedTime(readStarted)}"); + }, failure => RecordPrimaryFailure( + $"Primary snapshot failure; pointRead={pointRead}; repetition={iteration + 1}/3; phase={phase}", failure)); }, TestContext.Current.CancellationToken); TestContext.Current.TestOutputHelper?.WriteLine( $"Snapshot compatibility pass {iteration + 1}/3; pointRead={pointRead}; elapsed={Stopwatch.GetElapsedTime(started)}"); @@ -90,10 +96,33 @@ await CreateFixture().RunAsync(async (fixture, cancellationToken) => [Fact] public Task MembershipTable_ZooKeeper_DeletionProbe_PreservesOriginalScope() => CreateFixture().RunAsync((fixture, cancellationToken) => - new MembershipTableTestRunner(fixture, seed: 17) - .DeleteMembershipTableEntries_DeletesOwnClusterAndPreservesOtherCluster(cancellationToken), + CapturePrimaryScenarioFailureAsync( + () => new MembershipTableTestRunner(fixture, seed: 17) + .DeleteMembershipTableEntries_DeletesOwnClusterAndPreservesOtherCluster(cancellationToken), + failure => RecordPrimaryFailure("Primary deletion-probe failure", failure)), TestContext.Current.CancellationToken); + private void RecordPrimaryFailure(string context, string failure) + { + var record = context + Environment.NewLine + failure; + _loggerFactory.CreateLogger().LogError("{PrimaryFailure}", record); + TestContext.Current.TestOutputHelper?.WriteLine(record); + } + + internal static async Task CapturePrimaryScenarioFailureAsync(Func action, Action record) + { + try + { + await action(); + } + catch (Exception exception) + { + // Keep the caller stack before teardown re-observes retained operation failures. + record(exception.ToString()); + throw; + } + } + internal static async Task DrainFixtureAsync(Func dispose) { try diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index a2606e18398..4b21f63476b 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -656,6 +656,43 @@ public async Task NativeFixture_TeardownTimeout_JoinsActualCompletion(bool fail) private static TaskCompletionSource Gate() => new(TaskCreationOptions.RunContinuationsAsynchronously); + [Fact] + public async Task NativeFixture_PrimaryFailure_IsCapturedBeforeTeardownAndRethrownUnchanged() + { + var harness = await Harness.CreateAsync(); + var primary = new KeeperException.NoAuthException(); + harness.BeforeRequest = _ => Task.FromException(primary); + var events = new List(); + string captured = null!; + async Task PrimaryReadScenario() + { + await harness.Read(TestContext.Current.CancellationToken); + } + async Task RunWithTeardown() + { + try + { + await ZooKeeperReadResilienceTests.CapturePrimaryScenarioFailureAsync(PrimaryReadScenario, text => + { + events.Add("capture"); + Assert.Equal(primary.ToString(), text); + captured = text; + }); + } + finally + { + events.Add("teardown"); + Assert.Same(primary, await Record.ExceptionAsync(() => Assert.Single(harness.Sessions).Completion)); + } + } + + Assert.Same(primary, await Record.ExceptionAsync(RunWithTeardown)); + Assert.Equal(new[] { "capture", "teardown" }, events); + Assert.Contains(nameof(KeeperException.NoAuthException), captured, StringComparison.Ordinal); + Assert.Contains(nameof(PrimaryReadScenario), captured, StringComparison.Ordinal); + harness.AssertOneOwner(readOnly: true); + } + [Theory] [InlineData(false)] [InlineData(true)] From 8dda170160f35efffceed868f71f1acd6a11f64b Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Mon, 21 Sep 2026 11:01:52 -0700 Subject: [PATCH 07/10] fix(zookeeper): wait for reconnect before read retries --- .../ZooKeeperBasedMembershipTable.cs | 131 ++++++++++++- .../ZooKeeperConnectionMonitor.cs | 12 ++ .../ZooKeeperReadRetryPolicy.cs | 29 ++- .../ZooKeeperSession.cs | 18 +- .../ZooKeeperReadRetryTests.cs | 183 +++++++++++++++++- 5 files changed, 351 insertions(+), 22 deletions(-) create mode 100644 src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs index e84b666620e..f972a1831ae 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs @@ -41,7 +41,7 @@ public partial class ZooKeeperBasedMembershipTable : IMembershipTable { private readonly ILogger logger; - private const int ZOOKEEPER_SESSION_TIMEOUT = 10_000; + internal const int ZOOKEEPER_SESSION_TIMEOUT = 10_000; private readonly ZooKeeperWatcher watcher; private readonly Func _createSession; @@ -148,6 +148,7 @@ await UsingZookeeper(rootConnectionString, async zk => /// /// /// Connection-loss failures in native reads are retried up to four times on the operation's session. + /// Each retry waits for that session's next connected event before issuing another request. /// The table and child versions fence each complete snapshot pass. /// public Task ReadRowAsync(SiloAddress siloAddress, CancellationToken cancellationToken = default) @@ -171,6 +172,7 @@ public Task ReadRowAsync(SiloAddress siloAddress, Cancellat /// with each membership record read before its heartbeat. /// Table and child-version checks fence the complete snapshot. /// Connection-loss failures in native reads are retried up to four times on the same session. + /// Each retry waits for that session's next connected event before issuing another request. /// Caller cancellation stops further requests while admitted requests and client close complete. /// public Task ReadAllAsync(CancellationToken cancellationToken = default) @@ -184,18 +186,27 @@ internal static Task ReadAsync( SiloAddress? siloAddress, CancellationToken cancellationToken) { - return ZooKeeperSession.ExecuteAsync(createSession, - native => ReadCoreAsync(ZooKeeperReadRetryPolicy.Wrap(native, pipeline, cancellationToken), siloAddress, cancellationToken), + return ZooKeeperSession.ExecuteSessionAsync(createSession, + session => ReadCoreAsync( + ZooKeeperReadRetryPolicy.Wrap( + session.Operations, + pipeline, + cancellationToken, + session.ConnectionMonitor), + siloAddress, + cancellationToken), cancellationToken); } internal static ZooKeeperSession CreateSession(string connectionString, ZooKeeperWatcher watcher, bool readOnly) { - var client = new ZooKeeper(connectionString, ZOOKEEPER_SESSION_TIMEOUT, watcher, readOnly); + var sessionWatcher = watcher.CreateSessionWatcher(); + var client = new ZooKeeper(connectionString, ZOOKEEPER_SESSION_TIMEOUT, sessionWatcher, readOnly); return new ZooKeeperSession( new NativeOperations(path => client.getDataAsync(path), path => client.getChildrenAsync(path), client.sync, operations => client.multiAsync(operations), client.setDataAsync), - client.closeAsync); + client.closeAsync, + sessionWatcher); } internal static async Task ReadCoreAsync( @@ -668,23 +679,127 @@ await zk.Multi( } /// - /// the state of every ZooKeeper client and its push notifications are published using watchers. - /// in orleans the watcher is only for debugging purposes + /// Publishes ZooKeeper connection transitions for same-session read retry coordination + /// and logs watcher events for diagnostics. /// - internal partial class ZooKeeperWatcher : Watcher + internal partial class ZooKeeperWatcher : Watcher, IZooKeeperConnectionMonitor { private readonly ILogger logger; + private readonly TimeProvider _timeProvider; + private readonly TimeSpan _reconnectTimeout; + private readonly object _connectionLock = new(); + private TaskCompletionSource _connectionChanged = NewConnectionChangedSource(); + private long _connectedGeneration; + private bool _connected; + private bool _terminal; + public ZooKeeperWatcher(ILogger logger) + : this(logger, TimeProvider.System, TimeSpan.FromMilliseconds(ZooKeeperBasedMembershipTable.ZOOKEEPER_SESSION_TIMEOUT)) + { + } + + internal ZooKeeperWatcher(ILogger logger, TimeProvider timeProvider, TimeSpan reconnectTimeout) { this.logger = logger; + _timeProvider = timeProvider; + _reconnectTimeout = reconnectTimeout; } + internal ZooKeeperWatcher CreateSessionWatcher() => new(logger, _timeProvider, _reconnectTimeout); + public override Task process(WatchedEvent @event) { + ProcessConnectionState(@event.getState()); LogDebugWatchedEvent(@event); return Task.CompletedTask; } + internal void ProcessConnectionState(Event.KeeperState state) + { + TaskCompletionSource? changed = null; + lock (_connectionLock) + { + switch (state) + { + case Event.KeeperState.SyncConnected: + case Event.KeeperState.ConnectedReadOnly: + _connected = true; + _connectedGeneration++; + changed = _connectionChanged; + _connectionChanged = NewConnectionChangedSource(); + break; + case Event.KeeperState.AuthFailed: + case Event.KeeperState.Expired: + _connected = false; + _terminal = true; + changed = _connectionChanged; + _connectionChanged = NewConnectionChangedSource(); + break; + case Event.KeeperState.Disconnected: + _connected = false; + changed = _connectionChanged; + _connectionChanged = NewConnectionChangedSource(); + break; + } + } + + changed?.TrySetResult(); + } + + long IZooKeeperConnectionMonitor.CaptureAttemptGeneration() + { + lock (_connectionLock) + { + // Requests admitted while connecting execute on the next connection. + return _connected ? _connectedGeneration : _connectedGeneration + 1; + } + } + + async ValueTask IZooKeeperConnectionMonitor.WaitForConnectionAfterAsync( + long connectedGeneration, + CancellationToken cancellationToken) + { + using var timeoutCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + var timeout = Task.Delay(_reconnectTimeout, _timeProvider, timeoutCancellation.Token); + try + { + while (true) + { + Task changed; + lock (_connectionLock) + { + if (_connectedGeneration > connectedGeneration) + { + return true; + } + + if (_terminal) + { + return false; + } + + changed = _connectionChanged.Task; + } + + var completed = await Task.WhenAny(changed, timeout); + cancellationToken.ThrowIfCancellationRequested(); + if (completed == timeout) + { + return false; + } + + await changed; + } + } + finally + { + timeoutCancellation.Cancel(); + } + } + + private static TaskCompletionSource NewConnectionChangedSource() => + new(TaskCreationOptions.RunContinuationsAsynchronously); + [LoggerMessage( Level = LogLevel.Debug, Message = "{EventString}" diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs new file mode 100644 index 00000000000..d498204ff26 --- /dev/null +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs @@ -0,0 +1,12 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Orleans.Runtime.Membership; + +internal interface IZooKeeperConnectionMonitor +{ + long CaptureAttemptGeneration(); + + ValueTask WaitForConnectionAfterAsync(long connectedGeneration, CancellationToken cancellationToken); +} diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs index da5ce58583b..f6d5233d982 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs @@ -33,15 +33,16 @@ internal static ResiliencePipeline CreatePipeline(ILogger logger, TimeProvider t internal static ZooKeeperBasedMembershipTable.NativeOperations Wrap( ZooKeeperBasedMembershipTable.NativeOperations native, ResiliencePipeline pipeline, - CancellationToken cancellationToken) => + CancellationToken cancellationToken, + IZooKeeperConnectionMonitor? connectionMonitor = null) => new( - path => ExecuteAsync("GetData", () => native.GetData(path), pipeline, cancellationToken), - path => ExecuteAsync("GetChildren", () => native.GetChildren(path), pipeline, cancellationToken), + path => ExecuteAsync("GetData", () => native.GetData(path), pipeline, cancellationToken, connectionMonitor), + path => ExecuteAsync("GetChildren", () => native.GetChildren(path), pipeline, cancellationToken, connectionMonitor), path => ExecuteAsync("Sync", async () => { await native.Sync(path); return true; - }, pipeline, cancellationToken), + }, pipeline, cancellationToken, connectionMonitor), native.Multi, native.SetData); @@ -49,16 +50,32 @@ private static async Task ExecuteAsync( string operationName, Func> operation, ResiliencePipeline pipeline, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + IZooKeeperConnectionMonitor? connectionMonitor) { var context = ResilienceContextPool.Shared.Get(operationName, cancellationToken); + long? reconnectAfter = null; try { return await pipeline.ExecuteAsync(async context => { context.CancellationToken.ThrowIfCancellationRequested(); + if (reconnectAfter is { } connectedGeneration && connectionMonitor is not null) + { + await connectionMonitor.WaitForConnectionAfterAsync(connectedGeneration, context.CancellationToken); + } + + var attemptGeneration = connectionMonitor?.CaptureAttemptGeneration() ?? 0; // Await actual native completion: cancellation ends admission, not an in-flight request. - return await operation(); + try + { + return await operation(); + } + catch (KeeperException.ConnectionLossException) + { + reconnectAfter = attemptGeneration; + throw; + } }, context); } finally diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs index 7499b3a71fa..d99894dc893 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperSession.cs @@ -11,20 +11,32 @@ internal sealed class ZooKeeperSession // Bind synchronously; the owned task supplies the completion semantics. private readonly TaskCompletionSource _completion = new(); - internal ZooKeeperSession(ZooKeeperBasedMembershipTable.NativeOperations operations, Func close) + internal ZooKeeperSession( + ZooKeeperBasedMembershipTable.NativeOperations operations, + Func close, + IZooKeeperConnectionMonitor? connectionMonitor = null) { _operations = operations; _close = close; + ConnectionMonitor = connectionMonitor; Completion = _completion.Task.Unwrap(); Completion.Ignore(); } internal Task Completion { get; } + internal IZooKeeperConnectionMonitor? ConnectionMonitor { get; } + internal ZooKeeperBasedMembershipTable.NativeOperations Operations => _operations; internal static Task ExecuteAsync( Func createSession, Func> operation, CancellationToken cancellationToken) + => ExecuteSessionAsync(createSession, session => operation(session.Operations), cancellationToken); + + internal static Task ExecuteSessionAsync( + Func createSession, + Func> operation, + CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); var session = createSession(); @@ -33,12 +45,12 @@ internal static Task ExecuteAsync( return ZooKeeperBasedMembershipTable.AwaitOperationAsync(completion, cancellationToken); } - private async Task RunAsync(Func> operation) + private async Task RunAsync(Func> operation) { T result; try { - result = await operation(_operations); + result = await operation(this); } catch (Exception primary) { diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index 4b21f63476b..7ed83f5da2c 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -127,6 +127,161 @@ public async Task ReadRetry_IneligibleFailure_PropagatesAfterOneAttempt(string k harness.AssertOneOwner(readOnly: true); } + [Fact] + public async Task ConnectionMonitor_FailureBeforeDisconnected_WaitsForNextConnection() + { + var monitor = CreateConnectionMonitor(); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var connectedGeneration = ((IZooKeeperConnectionMonitor)monitor).CaptureAttemptGeneration(); + + var wait = ((IZooKeeperConnectionMonitor)monitor) + .WaitForConnectionAfterAsync(connectedGeneration, TestContext.Current.CancellationToken).AsTask(); + Assert.False(wait.IsCompleted); + + await Signal(monitor, Watcher.Event.KeeperState.Disconnected); + Assert.False(wait.IsCompleted); + + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + Assert.True(await wait); + } + + [Fact] + public async Task ConnectionMonitor_ReconnectBeforeWaiterRegistration_CompletesImmediately() + { + var monitor = CreateConnectionMonitor(); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var connectedGeneration = ((IZooKeeperConnectionMonitor)monitor).CaptureAttemptGeneration(); + await Signal(monitor, Watcher.Event.KeeperState.Disconnected); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + + var wait = ((IZooKeeperConnectionMonitor)monitor) + .WaitForConnectionAfterAsync(connectedGeneration, TestContext.Current.CancellationToken); + + Assert.True(wait.IsCompletedSuccessfully); + Assert.True(await wait); + } + + [Fact] + public async Task ConnectionMonitor_OneReconnectReleasesAllWaiters_AndCanceledWaiterDoesNotPoisonPeers() + { + var monitor = CreateConnectionMonitor(); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var connectedGeneration = ((IZooKeeperConnectionMonitor)monitor).CaptureAttemptGeneration(); + using var canceled = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var canceledWaiter = ((IZooKeeperConnectionMonitor)monitor) + .WaitForConnectionAfterAsync(connectedGeneration, canceled.Token).AsTask(); + var peers = Enumerable.Range(0, 16).Select(_ => ((IZooKeeperConnectionMonitor)monitor) + .WaitForConnectionAfterAsync(connectedGeneration, TestContext.Current.CancellationToken).AsTask()).ToArray(); + + canceled.Cancel(); + var cancellation = await Assert.ThrowsAnyAsync(() => canceledWaiter); + Assert.Equal(canceled.Token, cancellation.CancellationToken); + Assert.All(peers, peer => Assert.False(peer.IsCompleted)); + + await Signal(monitor, Watcher.Event.KeeperState.Disconnected); + Assert.All(peers, peer => Assert.False(peer.IsCompleted)); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + + Assert.All(await Task.WhenAll(peers), Assert.True); + } + + [Fact] + public async Task ConnectionMonitor_TimeoutReturnsWithoutReplacingNativeFailure() + { + var timeProvider = new FakeTimeProvider(); + var monitor = CreateConnectionMonitor(timeProvider, TimeSpan.FromSeconds(1)); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var connectedGeneration = ((IZooKeeperConnectionMonitor)monitor).CaptureAttemptGeneration(); + var wait = ((IZooKeeperConnectionMonitor)monitor) + .WaitForConnectionAfterAsync(connectedGeneration, TestContext.Current.CancellationToken).AsTask(); + + timeProvider.Advance(TimeSpan.FromMilliseconds(999)); + Assert.False(wait.IsCompleted); + timeProvider.Advance(TimeSpan.FromMilliseconds(1)); + + Assert.False(await wait); + } + + [Fact] + public async Task ConnectionMonitor_InitialConnectionDoesNotSatisfyPostFailureReconnect() + { + var monitor = CreateConnectionMonitor(); + var attemptGeneration = ((IZooKeeperConnectionMonitor)monitor).CaptureAttemptGeneration(); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + + var wait = ((IZooKeeperConnectionMonitor)monitor) + .WaitForConnectionAfterAsync(attemptGeneration, TestContext.Current.CancellationToken).AsTask(); + + Assert.False(wait.IsCompleted); + await Signal(monitor, Watcher.Event.KeeperState.Disconnected); + Assert.False(wait.IsCompleted); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + Assert.True(await wait); + } + + [Fact] + public async Task ReadRetry_ParallelRowsWaitForOneReconnectBeforeReissuing() + { + const int rowCount = 9; + var harness = await Harness.CreateAsync(rowCount); + var monitor = CreateConnectionMonitor(); + harness.ConnectionMonitor = monitor; + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var failed = new ConcurrentDictionary(StringComparer.Ordinal); + harness.BeforeRequest = request => + { + if (request.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) + && !request.EndsWith("/IAmAlive", StringComparison.Ordinal) + && failed.TryAdd(request, 0)) + { + throw new KeeperException.ConnectionLossException(); + } + + return Task.CompletedTask; + }; + + var read = harness.Read(TestContext.Current.CancellationToken); + Assert.Equal(rowCount, harness.Logger.Warnings.Count); + var admittedCalls = harness.Calls.ToArray(); + harness.Clock.Advance(TimeSpan.FromMilliseconds(250)); + Assert.Equal(admittedCalls, harness.Calls); + + await Signal(monitor, Watcher.Event.KeeperState.Disconnected); + Assert.Equal(admittedCalls, harness.Calls); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + + harness.AssertSnapshot(await read, harness.Entries, rowCount); + Assert.All(harness.Entries, entry => Assert.Equal(2, + harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)))); + Assert.All(harness.Entries, entry => Assert.Equal(1, + harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress)))); + harness.AssertOneOwner(readOnly: true); + } + + [Fact] + public async Task ReadRetry_ReconnectTimeout_ReissuesAndPreservesNativeFailure() + { + var harness = await Harness.CreateAsync(); + var monitorTime = new FakeTimeProvider(); + var monitor = CreateConnectionMonitor(monitorTime, TimeSpan.FromSeconds(1)); + harness.ConnectionMonitor = monitor; + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var failure = new KeeperException.ConnectionLossException(); + harness.BeforeRequest = _ => Task.FromException(failure); + var read = harness.Read(TestContext.Current.CancellationToken); + var completion = Record.ExceptionAsync(() => read); + + foreach (var delay in new[] { 250, 500, 1000, 2000 }) + { + await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(delay)); + monitorTime.Advance(TimeSpan.FromSeconds(1)); + } + + Assert.Same(failure, await completion); + Assert.Equal(Enumerable.Repeat("Sync /", 5), harness.Calls); + harness.AssertOneOwner(readOnly: true); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -656,6 +811,20 @@ public async Task NativeFixture_TeardownTimeout_JoinsActualCompletion(bool fail) private static TaskCompletionSource Gate() => new(TaskCreationOptions.RunContinuationsAsynchronously); + private static ZooKeeperWatcher CreateConnectionMonitor( + TimeProvider? timeProvider = null, + TimeSpan? reconnectTimeout = null) => + new( + NullLogger.Instance, + timeProvider ?? TimeProvider.System, + reconnectTimeout ?? TimeSpan.FromMinutes(1)); + + private static Task Signal(ZooKeeperWatcher watcher, Watcher.Event.KeeperState state) + { + watcher.ProcessConnectionState(state); + return Task.CompletedTask; + } + [Fact] public async Task NativeFixture_PrimaryFailure_IsCapturedBeforeTeardownAndRethrownUnchanged() { @@ -787,6 +956,7 @@ private sealed class Harness internal List Sessions { get; } = []; internal List ReadOnly { get; } = []; internal Action? OnSessionCreated { get; set; } + internal IZooKeeperConnectionMonitor? ConnectionMonitor { get; set; } internal Func? BeforeRequest { get; set; } internal Action? AfterRequest { get; set; } internal Func Close { get; set; } = () => Task.CompletedTask; @@ -844,11 +1014,14 @@ internal ZooKeeperSession CreateSession(bool readOnly) await AfterMulti(); }, native.SetData); - var session = new ZooKeeperSession(operations, () => - { - Interlocked.Increment(ref CloseCount); - return Close(); - }); + var session = new ZooKeeperSession( + operations, + () => + { + Interlocked.Increment(ref CloseCount); + return Close(); + }, + ConnectionMonitor); Sessions.Add(session); OnSessionCreated?.Invoke(session); return session; From 2b295d69c6da5887acc2f087c675d8012bce743b Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Mon, 21 Sep 2026 11:29:41 -0700 Subject: [PATCH 08/10] fix(zookeeper): serialize reconnect recovery reads --- .../ZooKeeperBasedMembershipTable.cs | 43 ++++- .../ZooKeeperConnectionMonitor.cs | 6 + .../ZooKeeperReadRetryPolicy.cs | 37 +++- .../ZooKeeperReadRetryTests.cs | 172 +++++++++++++++++- 4 files changed, 242 insertions(+), 16 deletions(-) diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs index f972a1831ae..b0e6af1d857 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs @@ -149,6 +149,7 @@ await UsingZookeeper(rootConnectionString, async zk => /// /// Connection-loss failures in native reads are retried up to four times on the operation's session. /// Each retry waits for that session's next connected event before issuing another request. + /// Recovery requests use one-at-a-time admission while first-attempt reads remain concurrent. /// The table and child versions fence each complete snapshot pass. /// public Task ReadRowAsync(SiloAddress siloAddress, CancellationToken cancellationToken = default) @@ -173,6 +174,7 @@ public Task ReadRowAsync(SiloAddress siloAddress, Cancellat /// Table and child-version checks fence the complete snapshot. /// Connection-loss failures in native reads are retried up to four times on the same session. /// Each retry waits for that session's next connected event before issuing another request. + /// Recovery requests use one-at-a-time admission while first-attempt rows remain concurrent. /// Caller cancellation stops further requests while admitted requests and client close complete. /// public Task ReadAllAsync(CancellationToken cancellationToken = default) @@ -688,6 +690,7 @@ internal partial class ZooKeeperWatcher : Watcher, IZooKeeperConnectionMonitor private readonly TimeProvider _timeProvider; private readonly TimeSpan _reconnectTimeout; private readonly object _connectionLock = new(); + private readonly SemaphoreSlim _retryAdmission = new(1, 1); private TaskCompletionSource _connectionChanged = NewConnectionChangedSource(); private long _connectedGeneration; private bool _connected; @@ -755,6 +758,37 @@ long IZooKeeperConnectionMonitor.CaptureAttemptGeneration() } } + bool IZooKeeperConnectionMonitor.IsConnectedAfter(long connectedGeneration) + { + lock (_connectionLock) + { + return _connected && _connectedGeneration > connectedGeneration; + } + } + + async ValueTask IZooKeeperConnectionMonitor.AcquireRetryAdmissionAsync( + CancellationToken cancellationToken) + { + await _retryAdmission.WaitAsync(cancellationToken); + return new RetryAdmission(_retryAdmission); + } + + void IZooKeeperConnectionMonitor.ReportConnectionLoss(long connectedGeneration) + { + TaskCompletionSource? changed = null; + lock (_connectionLock) + { + if (_connected && _connectedGeneration <= connectedGeneration) + { + _connected = false; + changed = _connectionChanged; + _connectionChanged = NewConnectionChangedSource(); + } + } + + changed?.TrySetResult(); + } + async ValueTask IZooKeeperConnectionMonitor.WaitForConnectionAfterAsync( long connectedGeneration, CancellationToken cancellationToken) @@ -768,7 +802,7 @@ async ValueTask IZooKeeperConnectionMonitor.WaitForConnectionAfterAsync( Task changed; lock (_connectionLock) { - if (_connectedGeneration > connectedGeneration) + if (_connected && _connectedGeneration > connectedGeneration) { return true; } @@ -800,6 +834,13 @@ async ValueTask IZooKeeperConnectionMonitor.WaitForConnectionAfterAsync( private static TaskCompletionSource NewConnectionChangedSource() => new(TaskCreationOptions.RunContinuationsAsynchronously); + private sealed class RetryAdmission(SemaphoreSlim admission) : IDisposable + { + private SemaphoreSlim? _admission = admission; + + public void Dispose() => Interlocked.Exchange(ref _admission, null)?.Release(); + } + [LoggerMessage( Level = LogLevel.Debug, Message = "{EventString}" diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs index d498204ff26..35cda3591a9 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs @@ -8,5 +8,11 @@ internal interface IZooKeeperConnectionMonitor { long CaptureAttemptGeneration(); + bool IsConnectedAfter(long connectedGeneration); + ValueTask WaitForConnectionAfterAsync(long connectedGeneration, CancellationToken cancellationToken); + + ValueTask AcquireRetryAdmissionAsync(CancellationToken cancellationToken); + + void ReportConnectionLoss(long connectedGeneration); } diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs index f6d5233d982..57fe2ad86a7 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs @@ -60,21 +60,44 @@ private static async Task ExecuteAsync( return await pipeline.ExecuteAsync(async context => { context.CancellationToken.ThrowIfCancellationRequested(); + IDisposable? retryAdmission = null; if (reconnectAfter is { } connectedGeneration && connectionMonitor is not null) { - await connectionMonitor.WaitForConnectionAfterAsync(connectedGeneration, context.CancellationToken); + while (await connectionMonitor.WaitForConnectionAfterAsync( + connectedGeneration, + context.CancellationToken)) + { + // Preserve first-attempt fanout while draining recovery traffic one request at a time. + retryAdmission = await connectionMonitor.AcquireRetryAdmissionAsync(context.CancellationToken); + if (connectionMonitor.IsConnectedAfter(connectedGeneration)) + { + break; + } + + retryAdmission.Dispose(); + retryAdmission = null; + } } - var attemptGeneration = connectionMonitor?.CaptureAttemptGeneration() ?? 0; - // Await actual native completion: cancellation ends admission, not an in-flight request. try { - return await operation(); + context.CancellationToken.ThrowIfCancellationRequested(); + var attemptGeneration = connectionMonitor?.CaptureAttemptGeneration() ?? 0; + // Await actual native completion: cancellation ends admission, not an in-flight request. + try + { + return await operation(); + } + catch (KeeperException.ConnectionLossException) + { + connectionMonitor?.ReportConnectionLoss(attemptGeneration); + reconnectAfter = attemptGeneration; + throw; + } } - catch (KeeperException.ConnectionLossException) + finally { - reconnectAfter = attemptGeneration; - throw; + retryAdmission?.Dispose(); } }, context); } diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index 7ed83f5da2c..380dff6b7b9 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -185,6 +185,24 @@ public async Task ConnectionMonitor_OneReconnectReleasesAllWaiters_AndCanceledWa Assert.All(await Task.WhenAll(peers), Assert.True); } + [Fact] + public async Task ConnectionMonitor_CanceledRetryAdmissionDoesNotPoisonPeers() + { + var monitor = (IZooKeeperConnectionMonitor)CreateConnectionMonitor(); + using var owner = await monitor.AcquireRetryAdmissionAsync(TestContext.Current.CancellationToken); + using var canceled = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var canceledAdmission = monitor.AcquireRetryAdmissionAsync(canceled.Token).AsTask(); + var peerAdmission = monitor.AcquireRetryAdmissionAsync(TestContext.Current.CancellationToken).AsTask(); + + canceled.Cancel(); + var failure = await Assert.ThrowsAnyAsync(() => canceledAdmission); + Assert.Equal(canceled.Token, failure.CancellationToken); + Assert.False(peerAdmission.IsCompleted); + + owner.Dispose(); + using var peer = await peerAdmission; + } + [Fact] public async Task ConnectionMonitor_TimeoutReturnsWithoutReplacingNativeFailure() { @@ -227,22 +245,63 @@ public async Task ReadRetry_ParallelRowsWaitForOneReconnectBeforeReissuing() var monitor = CreateConnectionMonitor(); harness.ConnectionMonitor = monitor; await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); - var failed = new ConcurrentDictionary(StringComparer.Ordinal); - harness.BeforeRequest = request => + var counts = new ConcurrentDictionary(StringComparer.Ordinal); + var releaseFirstAttempts = Gate(); + var allFirstAttempts = Gate(); + var allWarnings = Gate(); + var retryEntered = Channel.CreateUnbounded(); + var releaseRetry = Channel.CreateUnbounded(); + var firstAttempts = 0; + var activeFirstAttempts = 0; + var maxFirstAttempts = 0; + var activeRetryAttempts = 0; + var maxRetryAttempts = 0; + harness.Logger.WarningRecorded = count => + { + if (count == rowCount) + allWarnings.TrySetResult(); + }; + harness.BeforeRequest = async request => { if (request.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) - && !request.EndsWith("/IAmAlive", StringComparison.Ordinal) - && failed.TryAdd(request, 0)) + && !request.EndsWith("/IAmAlive", StringComparison.Ordinal)) { - throw new KeeperException.ConnectionLossException(); + var count = counts.AddOrUpdate(request, 1, static (_, value) => value + 1); + if (count == 1) + { + var active = Interlocked.Increment(ref activeFirstAttempts); + InterlockedExtensions.Max(ref maxFirstAttempts, active); + if (Interlocked.Increment(ref firstAttempts) == rowCount) + allFirstAttempts.TrySetResult(); + await releaseFirstAttempts.Task; + Interlocked.Decrement(ref activeFirstAttempts); + throw new KeeperException.ConnectionLossException(); + } + + if (count == 2) + { + var active = Interlocked.Increment(ref activeRetryAttempts); + InterlockedExtensions.Max(ref maxRetryAttempts, active); + Assert.True(retryEntered.Writer.TryWrite(request)); + await releaseRetry.Reader.ReadAsync(TestContext.Current.CancellationToken); + Interlocked.Decrement(ref activeRetryAttempts); + } } - - return Task.CompletedTask; }; var read = harness.Read(TestContext.Current.CancellationToken); - Assert.Equal(rowCount, harness.Logger.Warnings.Count); + await allFirstAttempts.Task.WaitAsync( + TimeSpan.FromSeconds(10), + TestContext.Current.CancellationToken); + Assert.Equal(rowCount, activeFirstAttempts); + Assert.True(maxFirstAttempts > 1); + releaseFirstAttempts.SetResult(); + await allWarnings.Task.WaitAsync( + TimeSpan.FromSeconds(10), + TestContext.Current.CancellationToken); var admittedCalls = harness.Calls.ToArray(); + for (var i = 0; i < rowCount; i++) + Assert.Equal(TimeSpan.FromMilliseconds(250), await harness.Clock.NextTimerAsync()); harness.Clock.Advance(TimeSpan.FromMilliseconds(250)); Assert.Equal(admittedCalls, harness.Calls); @@ -250,6 +309,25 @@ public async Task ReadRetry_ParallelRowsWaitForOneReconnectBeforeReissuing() Assert.Equal(admittedCalls, harness.Calls); await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + for (var i = 0; i < rowCount; i++) + { + try + { + _ = await retryEntered.Reader.ReadAsync(TestContext.Current.CancellationToken).AsTask().WaitAsync( + TimeSpan.FromSeconds(10), + TestContext.Current.CancellationToken); + } + catch (TimeoutException exception) + { + throw new TimeoutException( + $"Retry {i + 1}/{rowCount} did not enter; calls={string.Join(", ", counts.OrderBy(pair => pair.Key).Select(pair => $"{pair.Key}={pair.Value}"))}", + exception); + } + Assert.Equal(1, activeRetryAttempts); + Assert.Equal(1, maxRetryAttempts); + Assert.True(releaseRetry.Writer.TryWrite(true)); + } + harness.AssertSnapshot(await read, harness.Entries, rowCount); Assert.All(harness.Entries, entry => Assert.Equal(2, harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)))); @@ -258,6 +336,67 @@ public async Task ReadRetry_ParallelRowsWaitForOneReconnectBeforeReissuing() harness.AssertOneOwner(readOnly: true); } + [Fact] + public async Task ReadRetry_SerializedWaiterRevalidatesAfterEarlierRetryDisconnects() + { + const int rowCount = 2; + var harness = await Harness.CreateAsync(rowCount); + var monitor = CreateConnectionMonitor(); + harness.ConnectionMonitor = monitor; + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + var counts = new ConcurrentDictionary(StringComparer.Ordinal); + var secondRetryWarning = Gate(); + var retryEntered = Channel.CreateUnbounded(); + string? firstRetry = null; + harness.Logger.WarningRecorded = count => + { + if (count == rowCount + 1) + secondRetryWarning.TrySetResult(); + }; + harness.BeforeRequest = request => + { + if (!request.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) + || request.EndsWith("/IAmAlive", StringComparison.Ordinal)) + return Task.CompletedTask; + + var count = counts.AddOrUpdate(request, 1, static (_, value) => value + 1); + if (count == 1) + throw new KeeperException.ConnectionLossException(); + if (count == 2) + { + Assert.True(retryEntered.Writer.TryWrite(request)); + if (Interlocked.CompareExchange(ref firstRetry, request, null) is null) + throw new KeeperException.ConnectionLossException(); + } + + return Task.CompletedTask; + }; + + var read = harness.Read(TestContext.Current.CancellationToken); + Assert.Equal(rowCount, harness.Logger.Warnings.Count); + for (var i = 0; i < rowCount; i++) + Assert.Equal(TimeSpan.FromMilliseconds(250), await harness.Clock.NextTimerAsync()); + harness.Clock.Advance(TimeSpan.FromMilliseconds(250)); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + _ = await retryEntered.Reader.ReadAsync(TestContext.Current.CancellationToken); + await secondRetryWarning.Task.WaitAsync(TestContext.Current.CancellationToken); + + Assert.False(retryEntered.Reader.TryRead(out _)); + Assert.Equal(rowCount + 1, harness.Calls.Count(call => + call.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) + && !call.EndsWith("/IAmAlive", StringComparison.Ordinal))); + + await Signal(monitor, Watcher.Event.KeeperState.Disconnected); + await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); + _ = await retryEntered.Reader.ReadAsync(TestContext.Current.CancellationToken); + await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(500)); + + harness.AssertSnapshot(await read, harness.Entries, rowCount); + Assert.All(harness.Entries, entry => Assert.True( + harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)) is 2 or 3)); + harness.AssertOneOwner(readOnly: true); + } + [Fact] public async Task ReadRetry_ReconnectTimeout_ReissuesAndPreservesNativeFailure() { @@ -1112,6 +1251,7 @@ private sealed class RecordingLogger : ILogger { internal sealed record Warning(Exception? Exception, IReadOnlyDictionary Values); internal ConcurrentQueue Warnings { get; } = new(); + internal Action? WarningRecorded { get; set; } public IDisposable? BeginScope(TState state) where TState : notnull => null; public bool IsEnabled(LogLevel logLevel) => true; public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) @@ -1119,6 +1259,22 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except Assert.Equal(LogLevel.Warning, logLevel); var values = Assert.IsAssignableFrom>>(state); Warnings.Enqueue(new(exception, values.ToDictionary(pair => pair.Key, pair => pair.Value))); + WarningRecorded?.Invoke(Warnings.Count); + } + } + + private static class InterlockedExtensions + { + internal static void Max(ref int location, int value) + { + var current = Volatile.Read(ref location); + while (current < value) + { + var observed = Interlocked.CompareExchange(ref location, value, current); + if (observed == current) + return; + current = observed; + } } } } From 5856ccd4844817b5237b3a0f5aed1e551659a2b2 Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Mon, 21 Sep 2026 11:50:04 -0700 Subject: [PATCH 09/10] fix(zookeeper): read membership rows sequentially --- .../ZooKeeperBasedMembershipTable.cs | 84 +++-- .../ZooKeeperConnectionMonitor.cs | 4 - .../ZooKeeperReadRetryPolicy.cs | 39 +-- .../ZooKeeperBasedMembershipTableUnitTests.cs | 244 ++++++++++----- .../ZooKeeperReadRetryTests.cs | 295 +++--------------- 5 files changed, 245 insertions(+), 421 deletions(-) diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs index b0e6af1d857..8829216728c 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs @@ -42,6 +42,8 @@ public partial class ZooKeeperBasedMembershipTable : IMembershipTable private readonly ILogger logger; internal const int ZOOKEEPER_SESSION_TIMEOUT = 10_000; + internal const int MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS = 5; + internal const int MAX_CLEANUP_ROW_ATTEMPTS = 5; private readonly ZooKeeperWatcher watcher; private readonly Func _createSession; @@ -149,8 +151,8 @@ await UsingZookeeper(rootConnectionString, async zk => /// /// Connection-loss failures in native reads are retried up to four times on the operation's session. /// Each retry waits for that session's next connected event before issuing another request. - /// Recovery requests use one-at-a-time admission while first-attempt reads remain concurrent. - /// The table and child versions fence each complete snapshot pass. + /// The table and child versions fence each complete snapshot pass. Concurrent canonical + /// modifications restart the pass up to five total attempts. /// public Task ReadRowAsync(SiloAddress siloAddress, CancellationToken cancellationToken = default) { @@ -169,12 +171,12 @@ public Task ReadRowAsync(SiloAddress siloAddress, Cancellat /// /// - /// Rows are read concurrently on an operation-owned connection, - /// with each membership record read before its heartbeat. - /// Table and child-version checks fence the complete snapshot. + /// Rows are read sequentially on an operation-owned connection. + /// Each membership record is read before its heartbeat. + /// Table and child-version checks fence the complete snapshot. Concurrent canonical + /// modifications restart the complete sequential pass up to five total attempts. /// Connection-loss failures in native reads are retried up to four times on the same session. /// Each retry waits for that session's next connected event before issuing another request. - /// Recovery requests use one-at-a-time admission while first-attempt rows remain concurrent. /// Caller cancellation stops further requests while admitted requests and client close complete. /// public Task ReadAllAsync(CancellationToken cancellationToken = default) @@ -217,7 +219,7 @@ internal static async Task ReadCoreAsync( cancellationToken.ThrowIfCancellationRequested(); await zk.Sync("/"); // Retries retain this session's ordered view. - while (true) + for (var attempt = 0; attempt < MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS; attempt++) { cancellationToken.ThrowIfCancellationRequested(); Stat before; @@ -236,22 +238,21 @@ internal static async Task ReadCoreAsync( var rows = new List>(); KeeperException.NoNodeException? missingRow = null; - cancellationToken.ThrowIfCancellationRequested(); - var pendingRows = Task.WhenAll(addresses.Select(address => GetRow(zk, address, siloAddress is not null, cancellationToken))); - try + foreach (var address in addresses) { - rows.AddRange((await pendingRows).OfType>()); - } - catch (KeeperException.NoNodeException exception) - { - // Join every admitted read so a missing row cannot hide another native failure. - var failure = pendingRows.Exception!.InnerExceptions.FirstOrDefault(error => error is not KeeperException.NoNodeException); - if (failure is not null) + cancellationToken.ThrowIfCancellationRequested(); + try { - System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(failure).Throw(); + if (await GetRow(zk, address, siloAddress is not null, cancellationToken) is { } row) + { + rows.Add(row); + } + } + catch (KeeperException.NoNodeException exception) + { + missingRow = exception; + break; } - - missingRow = exception; } cancellationToken.ThrowIfCancellationRequested(); @@ -266,6 +267,9 @@ internal static async Task ReadCoreAsync( return new MembershipTableData(rows, ConvertToTableVersion(after.Stat)); } } + + throw new OrleansException( + $"Unable to read a consistent ZooKeeper membership snapshot after {MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS} attempts."); } private static bool SameVersion(Stat before, Stat after) => @@ -586,6 +590,11 @@ internal static T Deserialize(byte[] data) public Task CleanupDefunctSiloEntries(DateTimeOffset beforeDate) => CleanupDefunctSiloEntriesAsync(beforeDate, CancellationToken.None); /// + /// + /// Rows are evaluated sequentially in the order returned by ZooKeeper. A row whose + /// conditional delete conflicts is re-evaluated up to five total attempts before + /// the operation reports contention. + /// public Task CleanupDefunctSiloEntriesAsync(DateTimeOffset beforeDate, CancellationToken cancellationToken = default) { return UsingZookeeper(zk => CleanupCoreAsync(zk, beforeDate, cancellationToken), @@ -597,14 +606,19 @@ internal static async Task CleanupCoreAsync(NativeOperations zk, DateTimeO cancellationToken.ThrowIfCancellationRequested(); var children = await zk.GetChildren("/"); var cutoff = beforeDate.UtcDateTime; - await Task.WhenAll(children.Children.Select(child => CleanupRowAsync(zk, "/" + child, cutoff, cancellationToken))); + foreach (var child in children.Children) + { + cancellationToken.ThrowIfCancellationRequested(); + await CleanupRowAsync(zk, "/" + child, cutoff, cancellationToken); + } + return true; } private static async Task CleanupRowAsync(NativeOperations zk, string rowPath, DateTime cutoff, CancellationToken cancellationToken) { var heartbeatPath = rowPath + "/IAmAlive"; - while (true) + for (var attempt = 0; attempt < MAX_CLEANUP_ROW_ATTEMPTS; attempt++) { cancellationToken.ThrowIfCancellationRequested(); DataResult row; @@ -665,6 +679,9 @@ await zk.Multi( throw; } } + + throw new OrleansException( + $"Unable to clean ZooKeeper membership row '{rowPath}' after {MAX_CLEANUP_ROW_ATTEMPTS} concurrent modifications."); } [LoggerMessage( @@ -690,7 +707,6 @@ internal partial class ZooKeeperWatcher : Watcher, IZooKeeperConnectionMonitor private readonly TimeProvider _timeProvider; private readonly TimeSpan _reconnectTimeout; private readonly object _connectionLock = new(); - private readonly SemaphoreSlim _retryAdmission = new(1, 1); private TaskCompletionSource _connectionChanged = NewConnectionChangedSource(); private long _connectedGeneration; private bool _connected; @@ -758,21 +774,6 @@ long IZooKeeperConnectionMonitor.CaptureAttemptGeneration() } } - bool IZooKeeperConnectionMonitor.IsConnectedAfter(long connectedGeneration) - { - lock (_connectionLock) - { - return _connected && _connectedGeneration > connectedGeneration; - } - } - - async ValueTask IZooKeeperConnectionMonitor.AcquireRetryAdmissionAsync( - CancellationToken cancellationToken) - { - await _retryAdmission.WaitAsync(cancellationToken); - return new RetryAdmission(_retryAdmission); - } - void IZooKeeperConnectionMonitor.ReportConnectionLoss(long connectedGeneration) { TaskCompletionSource? changed = null; @@ -834,13 +835,6 @@ async ValueTask IZooKeeperConnectionMonitor.WaitForConnectionAfterAsync( private static TaskCompletionSource NewConnectionChangedSource() => new(TaskCreationOptions.RunContinuationsAsynchronously); - private sealed class RetryAdmission(SemaphoreSlim admission) : IDisposable - { - private SemaphoreSlim? _admission = admission; - - public void Dispose() => Interlocked.Exchange(ref _admission, null)?.Release(); - } - [LoggerMessage( Level = LogLevel.Debug, Message = "{EventString}" diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs index 35cda3591a9..a1e91dda530 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperConnectionMonitor.cs @@ -8,11 +8,7 @@ internal interface IZooKeeperConnectionMonitor { long CaptureAttemptGeneration(); - bool IsConnectedAfter(long connectedGeneration); - ValueTask WaitForConnectionAfterAsync(long connectedGeneration, CancellationToken cancellationToken); - ValueTask AcquireRetryAdmissionAsync(CancellationToken cancellationToken); - void ReportConnectionLoss(long connectedGeneration); } diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs index 57fe2ad86a7..a9ac4f6243b 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperReadRetryPolicy.cs @@ -60,44 +60,25 @@ private static async Task ExecuteAsync( return await pipeline.ExecuteAsync(async context => { context.CancellationToken.ThrowIfCancellationRequested(); - IDisposable? retryAdmission = null; if (reconnectAfter is { } connectedGeneration && connectionMonitor is not null) { - while (await connectionMonitor.WaitForConnectionAfterAsync( + await connectionMonitor.WaitForConnectionAfterAsync( connectedGeneration, - context.CancellationToken)) - { - // Preserve first-attempt fanout while draining recovery traffic one request at a time. - retryAdmission = await connectionMonitor.AcquireRetryAdmissionAsync(context.CancellationToken); - if (connectionMonitor.IsConnectedAfter(connectedGeneration)) - { - break; - } - - retryAdmission.Dispose(); - retryAdmission = null; - } + context.CancellationToken); } + context.CancellationToken.ThrowIfCancellationRequested(); + var attemptGeneration = connectionMonitor?.CaptureAttemptGeneration() ?? 0; + // Await actual native completion: cancellation ends admission, not an in-flight request. try { - context.CancellationToken.ThrowIfCancellationRequested(); - var attemptGeneration = connectionMonitor?.CaptureAttemptGeneration() ?? 0; - // Await actual native completion: cancellation ends admission, not an in-flight request. - try - { - return await operation(); - } - catch (KeeperException.ConnectionLossException) - { - connectionMonitor?.ReportConnectionLoss(attemptGeneration); - reconnectAfter = attemptGeneration; - throw; - } + return await operation(); } - finally + catch (KeeperException.ConnectionLossException) { - retryAdmission?.Dispose(); + connectionMonitor?.ReportConnectionLoss(attemptGeneration); + reconnectAfter = attemptGeneration; + throw; } }, context); } diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs index 11e277cd7a5..ac0459edced 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperBasedMembershipTableUnitTests.cs @@ -407,7 +407,7 @@ public async Task Read_ConcurrentHeartbeat_PreservesCanonicalFence(bool pointRea } [Fact] - public async Task ReadAll_PipelinesRowsWithSequentialMemberAndHeartbeatReads() + public async Task ReadAll_ReadsRowsSequentially_WithMemberBeforeHeartbeat() { var (fake, first) = await CreateNativeTable(); var second = CreateTimedEntry(12346); @@ -415,12 +415,22 @@ public async Task ReadAll_PipelinesRowsWithSequentialMemberAndHeartbeatReads() fake.Calls.Clear(); var rowStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var releaseRow = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var activeReads = 0; fake.BeforeRead = async path => { - if (path == ZooKeeperNativeFake.RowPath(first.SiloAddress)) + var active = Interlocked.Increment(ref activeReads); + Assert.Equal(1, active); + try + { + if (path == ZooKeeperNativeFake.RowPath(first.SiloAddress)) + { + rowStarted.TrySetResult(); + await releaseRow.Task.WaitAsync(TestContext.Current.CancellationToken); + } + } + finally { - rowStarted.TrySetResult(); - await releaseRow.Task.WaitAsync(TestContext.Current.CancellationToken); + Interlocked.Decrement(ref activeReads); } }; @@ -429,9 +439,7 @@ public async Task ReadAll_PipelinesRowsWithSequentialMemberAndHeartbeatReads() { await rowStarted.Task.WaitAsync(TestContext.Current.CancellationToken); Assert.Equal( - new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), - "read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), - "read " + ZooKeeperNativeFake.HeartbeatPath(second.SiloAddress) }, + new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress) }, fake.Calls); Assert.False(read.IsCompleted); } @@ -453,9 +461,9 @@ public async Task ReadAll_PipelinesRowsWithSequentialMemberAndHeartbeatReads() } Assert.Equal( new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), + "read " + ZooKeeperNativeFake.HeartbeatPath(first.SiloAddress), "read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), - "read " + ZooKeeperNativeFake.HeartbeatPath(second.SiloAddress), - "read " + ZooKeeperNativeFake.HeartbeatPath(first.SiloAddress), "read /" }, + "read " + ZooKeeperNativeFake.HeartbeatPath(second.SiloAddress), "read /" }, fake.Calls); } @@ -464,7 +472,7 @@ public async Task ReadAll_PipelinesRowsWithSequentialMemberAndHeartbeatReads() [InlineData(9, true)] [InlineData(128, false)] [InlineData(128, true)] - public async Task ReadAll_StartsEveryRowBeforeAwaitingNativeCompletion(int rowCount, bool cancel) + public async Task ReadAll_ReadsOneRowAtATime(int rowCount, bool cancel) { var fake = new ZooKeeperNativeFake(); var entries = Enumerable.Range(0, rowCount).Select(index => CreateTimedEntry(12345 + index)).ToArray(); @@ -476,32 +484,21 @@ public async Task ReadAll_StartsEveryRowBeforeAwaitingNativeCompletion(int rowCo fake.Calls.Clear(); var releaseMembers = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using var cancellation = new CancellationTokenSource(); + var firstPath = ZooKeeperNativeFake.RowPath(entries[0].SiloAddress); fake.BeforeRead = async path => { - if (path != "/" && !path.EndsWith("/IAmAlive", StringComparison.Ordinal)) + if (path == firstPath) { await releaseMembers.Task.WaitAsync(TestContext.Current.CancellationToken); } }; - var native = fake.Operations; - Task ReadData(string path) - { - // Protect the fake's synchronous call log while native completions remain independently gated. - lock (fake.Calls) - { - return native.GetData(path); - } - } - var operations = new ZooKeeperBasedMembershipTable.NativeOperations( - ReadData, native.GetChildren, native.Sync, native.Multi, native.SetData); - var read = ZooKeeperBasedMembershipTable.ReadCoreAsync(operations, null, cancellation.Token); + var read = ZooKeeperBasedMembershipTable.ReadCoreAsync(fake.Operations, null, cancellation.Token); var completion = Record.ExceptionAsync(() => read); try { Assert.Equal( - new[] { "sync /", "children /" }.Concat( - entries.Select(entry => "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))), + new[] { "sync /", "children /", "read " + firstPath }, fake.Calls); if (cancel) { @@ -520,7 +517,7 @@ Task ReadData(string path) { Assert.Equal(cancellation.Token, Assert.IsAssignableFrom(await completion).CancellationToken); - Assert.Equal(2 + rowCount, fake.Calls.Count); + Assert.Equal(3, fake.Calls.Count); } else { @@ -539,71 +536,29 @@ Task ReadData(string path) } } - [Theory] - [InlineData("missing")] - [InlineData("authorization")] - [InlineData("cancellation")] - public async Task Read_MissingRow_AwaitsOtherAdmittedRowsAndPreservesFailures(string otherOutcome) + [Fact] + public async Task Read_MissingRow_StopsBeforeLaterRowsAndPreservesFailure() { var (fake, first) = await CreateNativeTable(); var second = CreateTimedEntry(12346); Assert.True(await Insert(fake, second, 1)); fake.Calls.Clear(); var missingRow = new KeeperException.NoNodeException(ZooKeeperNativeFake.RowPath(first.SiloAddress)); - var releaseOther = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var authorizationFailure = new KeeperException.NoAuthException(); - using var cancellation = new CancellationTokenSource(); - fake.BeforeRead = async path => + fake.BeforeRead = path => { if (path == ZooKeeperNativeFake.RowPath(first.SiloAddress)) { throw missingRow; } - if (path == ZooKeeperNativeFake.RowPath(second.SiloAddress)) - { - await releaseOther.Task.WaitAsync(TestContext.Current.CancellationToken); - if (otherOutcome == "authorization") - { - throw authorizationFailure; - } - - if (otherOutcome == "cancellation") - { - cancellation.Cancel(); - cancellation.Token.ThrowIfCancellationRequested(); - } - - throw new KeeperException.NoNodeException(path); - } + return Task.CompletedTask; }; - var read = ZooKeeperBasedMembershipTable.ReadCoreAsync(fake.Operations, null, cancellation.Token); - var completion = Record.ExceptionAsync(() => read); - try - { - Assert.Contains("read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), fake.Calls); - Assert.False(read.IsCompleted); - } - finally - { - releaseOther.TrySetResult(); - await completion; - } - - var failure = await completion; - if (otherOutcome == "authorization") - { - Assert.Same(authorizationFailure, failure); - } - else if (otherOutcome == "cancellation") - { - Assert.Equal(cancellation.Token, Assert.IsAssignableFrom(failure).CancellationToken); - } - else - { - Assert.Same(missingRow, failure); - } + Assert.Same(missingRow, await Record.ExceptionAsync(() => Read(fake))); + Assert.Equal( + new[] { "sync /", "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), "read /" }, + fake.Calls); + Assert.DoesNotContain("read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), fake.Calls); } [Fact] @@ -701,6 +656,62 @@ public async Task ReadAll_UpdateDuringLaterRow_RetriesTheWholeSnapshot(int rowCo fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)))); } + [Fact] + public async Task ReadAll_PerpetualCanonicalChurn_StopsAfterMaximumAttempts() + { + var (fake, entry) = await CreateNativeTable(); + fake.BeforeRead = path => + { + if (path == "/") + { + var root = fake.Nodes["/"]; + fake.Nodes["/"] = root with { Version = root.Version + 1 }; + } + + return Task.CompletedTask; + }; + + var failure = await Assert.ThrowsAsync(() => Read(fake)); + + Assert.Equal( + $"Unable to read a consistent ZooKeeper membership snapshot after {ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS} attempts.", + failure.Message); + Assert.Equal(1, fake.Calls.Count(call => call == "sync /")); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS, + fake.Calls.Count(call => call == "children /")); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS, + fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS, + fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress))); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS, + fake.Calls.Count(call => call == "read /")); + } + + [Fact] + public async Task ReadAll_StabilizesOnFinalAllowedAttempt() + { + var (fake, entry) = await CreateNativeTable(); + var closingReads = 0; + fake.BeforeRead = path => + { + if (path == "/" && ++closingReads < ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS) + { + var root = fake.Nodes["/"]; + fake.Nodes["/"] = root with { Version = root.Version + 1 }; + } + + return Task.CompletedTask; + }; + + var result = await Read(fake); + + Assert.Single(result.Members); + Assert.Equal(entry.SiloAddress, result.Members[0].Item1.SiloAddress); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS, closingReads); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_MEMBERSHIP_SNAPSHOT_ATTEMPTS, + fake.Calls.Count(call => call == "children /")); + } + [Fact] public async Task ReadAll_CancellationStopsBeforeRequestingTheNextRow() { @@ -730,20 +741,22 @@ public async Task ReadAll_CancellationStopsBeforeRequestingTheNextRow() } [Fact] - public async Task Read_MissingRowAndAuthorizationFailure_PropagatesAuthorizationFailure() + public async Task Read_MissingRow_DoesNotObserveLaterAuthorizationFailure() { var (fake, entry) = await CreateNativeTable(); var second = CreateTimedEntry(12346); Assert.True(await Insert(fake, second, 1)); - var failure = new KeeperException.NoAuthException(); + var missing = new KeeperException.NoNodeException(ZooKeeperNativeFake.RowPath(entry.SiloAddress)); + var authorization = new KeeperException.NoAuthException(); fake.BeforeRead = path => path == ZooKeeperNativeFake.RowPath(entry.SiloAddress) - ? Task.FromException(new KeeperException.NoNodeException(path)) + ? Task.FromException(missing) : path == ZooKeeperNativeFake.RowPath(second.SiloAddress) - ? Task.FromException(failure) : Task.CompletedTask; + ? Task.FromException(authorization) : Task.CompletedTask; var actual = await Record.ExceptionAsync(() => Read(fake)); - Assert.Same(failure, actual); + Assert.Same(missing, actual); + Assert.DoesNotContain("read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), fake.Calls); Assert.Equal(1, fake.Calls.Count(call => call == "sync /")); } @@ -1364,13 +1377,13 @@ await ZooKeeperBasedMembershipTable.CleanupCoreAsync( } [Fact] - public async Task Cleanup_ParallelFailure_WaitsForOutstandingRequests() + public async Task Cleanup_ReadsRowsSequentiallyAndStopsOnFailure() { var (fake, first) = await CreateNativeTable(); var second = CreateTimedEntry(12346); Assert.True(await Insert(fake, second, 1)); + fake.Calls.Clear(); var firstStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var failed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var completeFirst = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var failure = new KeeperException.NoAuthException(); fake.BeforeRead = path => @@ -1383,7 +1396,6 @@ public async Task Cleanup_ParallelFailure_WaitsForOutstandingRequests() if (path == ZooKeeperNativeFake.RowPath(second.SiloAddress)) { - failed.SetResult(); return Task.FromException(failure); } @@ -1394,8 +1406,9 @@ public async Task Cleanup_ParallelFailure_WaitsForOutstandingRequests() fake.Operations, DateTime.UnixEpoch.AddDays(2), TestContext.Current.CancellationToken); try { - await Task.WhenAll(firstStarted.Task, failed.Task).WaitAsync(TestContext.Current.CancellationToken); + await firstStarted.Task.WaitAsync(TestContext.Current.CancellationToken); Assert.False(operation.IsCompleted); + Assert.DoesNotContain("read " + ZooKeeperNativeFake.RowPath(second.SiloAddress), fake.Calls); } finally { @@ -1403,6 +1416,10 @@ public async Task Cleanup_ParallelFailure_WaitsForOutstandingRequests() } Assert.Same(failure, await Record.ExceptionAsync(() => operation)); + Assert.Equal( + new[] { "children /", "read " + ZooKeeperNativeFake.RowPath(first.SiloAddress), + "read " + ZooKeeperNativeFake.RowPath(second.SiloAddress) }, + fake.Calls); Assert.Equal(5, fake.Nodes.Count); } @@ -1495,6 +1512,61 @@ public async Task MembershipOperations_InfrastructureFailure_PropagatesSameExcep Assert.Equal(1, fake.Nodes["/"].Version); } + [Fact] + public async Task Cleanup_PerpetualConflict_StopsAfterMaximumAttempts() + { + var (fake, entry) = await CreateNativeTable(SiloStatus.Dead); + var later = CreateTimedEntry(12346); + Assert.True(await Insert(fake, later, 1)); + fake.Calls.Clear(); + fake.Transactions.Clear(); + fake.BeforeMulti = _ => Task.FromException( + new KeeperException.BadVersionException(ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress))); + + var failure = await Assert.ThrowsAsync(() => + ZooKeeperBasedMembershipTable.CleanupCoreAsync( + fake.Operations, + DateTime.UnixEpoch.AddDays(2), + TestContext.Current.CancellationToken)); + + Assert.Equal( + $"Unable to clean ZooKeeper membership row '{ZooKeeperNativeFake.RowPath(entry.SiloAddress)}' after {ZooKeeperBasedMembershipTable.MAX_CLEANUP_ROW_ATTEMPTS} concurrent modifications.", + failure.Message); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_CLEANUP_ROW_ATTEMPTS, fake.Transactions.Count); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_CLEANUP_ROW_ATTEMPTS, + fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))); + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_CLEANUP_ROW_ATTEMPTS, + fake.Calls.Count(call => call == "read " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress))); + Assert.True(fake.Nodes.ContainsKey(ZooKeeperNativeFake.RowPath(entry.SiloAddress))); + Assert.True(fake.Nodes.ContainsKey(ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress))); + Assert.DoesNotContain("read " + ZooKeeperNativeFake.RowPath(later.SiloAddress), fake.Calls); + } + + [Fact] + public async Task Cleanup_ConflictOnFinalAllowedAttempt_ThenSucceeds() + { + var (fake, entry) = await CreateNativeTable(SiloStatus.Dead); + var attempts = 0; + fake.BeforeMulti = _ => + { + if (++attempts < ZooKeeperBasedMembershipTable.MAX_CLEANUP_ROW_ATTEMPTS) + { + throw new KeeperException.BadVersionException(ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress)); + } + + return Task.CompletedTask; + }; + + await ZooKeeperBasedMembershipTable.CleanupCoreAsync( + fake.Operations, + DateTime.UnixEpoch.AddDays(2), + TestContext.Current.CancellationToken); + + Assert.Equal(ZooKeeperBasedMembershipTable.MAX_CLEANUP_ROW_ATTEMPTS, attempts); + Assert.Single(fake.Nodes); + Assert.Equal("/", Assert.Single(fake.Nodes).Key); + } + [Theory] [InlineData("update")] [InlineData("cleanup")] diff --git a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs index 380dff6b7b9..c48c978f5c0 100644 --- a/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs +++ b/test/Extensions/Orleans.Clustering.ZooKeeper.Tests/ZooKeeperReadRetryTests.cs @@ -185,24 +185,6 @@ public async Task ConnectionMonitor_OneReconnectReleasesAllWaiters_AndCanceledWa Assert.All(await Task.WhenAll(peers), Assert.True); } - [Fact] - public async Task ConnectionMonitor_CanceledRetryAdmissionDoesNotPoisonPeers() - { - var monitor = (IZooKeeperConnectionMonitor)CreateConnectionMonitor(); - using var owner = await monitor.AcquireRetryAdmissionAsync(TestContext.Current.CancellationToken); - using var canceled = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); - var canceledAdmission = monitor.AcquireRetryAdmissionAsync(canceled.Token).AsTask(); - var peerAdmission = monitor.AcquireRetryAdmissionAsync(TestContext.Current.CancellationToken).AsTask(); - - canceled.Cancel(); - var failure = await Assert.ThrowsAnyAsync(() => canceledAdmission); - Assert.Equal(canceled.Token, failure.CancellationToken); - Assert.False(peerAdmission.IsCompleted); - - owner.Dispose(); - using var peer = await peerAdmission; - } - [Fact] public async Task ConnectionMonitor_TimeoutReturnsWithoutReplacingNativeFailure() { @@ -237,166 +219,6 @@ public async Task ConnectionMonitor_InitialConnectionDoesNotSatisfyPostFailureRe Assert.True(await wait); } - [Fact] - public async Task ReadRetry_ParallelRowsWaitForOneReconnectBeforeReissuing() - { - const int rowCount = 9; - var harness = await Harness.CreateAsync(rowCount); - var monitor = CreateConnectionMonitor(); - harness.ConnectionMonitor = monitor; - await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); - var counts = new ConcurrentDictionary(StringComparer.Ordinal); - var releaseFirstAttempts = Gate(); - var allFirstAttempts = Gate(); - var allWarnings = Gate(); - var retryEntered = Channel.CreateUnbounded(); - var releaseRetry = Channel.CreateUnbounded(); - var firstAttempts = 0; - var activeFirstAttempts = 0; - var maxFirstAttempts = 0; - var activeRetryAttempts = 0; - var maxRetryAttempts = 0; - harness.Logger.WarningRecorded = count => - { - if (count == rowCount) - allWarnings.TrySetResult(); - }; - harness.BeforeRequest = async request => - { - if (request.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) - && !request.EndsWith("/IAmAlive", StringComparison.Ordinal)) - { - var count = counts.AddOrUpdate(request, 1, static (_, value) => value + 1); - if (count == 1) - { - var active = Interlocked.Increment(ref activeFirstAttempts); - InterlockedExtensions.Max(ref maxFirstAttempts, active); - if (Interlocked.Increment(ref firstAttempts) == rowCount) - allFirstAttempts.TrySetResult(); - await releaseFirstAttempts.Task; - Interlocked.Decrement(ref activeFirstAttempts); - throw new KeeperException.ConnectionLossException(); - } - - if (count == 2) - { - var active = Interlocked.Increment(ref activeRetryAttempts); - InterlockedExtensions.Max(ref maxRetryAttempts, active); - Assert.True(retryEntered.Writer.TryWrite(request)); - await releaseRetry.Reader.ReadAsync(TestContext.Current.CancellationToken); - Interlocked.Decrement(ref activeRetryAttempts); - } - } - }; - - var read = harness.Read(TestContext.Current.CancellationToken); - await allFirstAttempts.Task.WaitAsync( - TimeSpan.FromSeconds(10), - TestContext.Current.CancellationToken); - Assert.Equal(rowCount, activeFirstAttempts); - Assert.True(maxFirstAttempts > 1); - releaseFirstAttempts.SetResult(); - await allWarnings.Task.WaitAsync( - TimeSpan.FromSeconds(10), - TestContext.Current.CancellationToken); - var admittedCalls = harness.Calls.ToArray(); - for (var i = 0; i < rowCount; i++) - Assert.Equal(TimeSpan.FromMilliseconds(250), await harness.Clock.NextTimerAsync()); - harness.Clock.Advance(TimeSpan.FromMilliseconds(250)); - Assert.Equal(admittedCalls, harness.Calls); - - await Signal(monitor, Watcher.Event.KeeperState.Disconnected); - Assert.Equal(admittedCalls, harness.Calls); - await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); - - for (var i = 0; i < rowCount; i++) - { - try - { - _ = await retryEntered.Reader.ReadAsync(TestContext.Current.CancellationToken).AsTask().WaitAsync( - TimeSpan.FromSeconds(10), - TestContext.Current.CancellationToken); - } - catch (TimeoutException exception) - { - throw new TimeoutException( - $"Retry {i + 1}/{rowCount} did not enter; calls={string.Join(", ", counts.OrderBy(pair => pair.Key).Select(pair => $"{pair.Key}={pair.Value}"))}", - exception); - } - Assert.Equal(1, activeRetryAttempts); - Assert.Equal(1, maxRetryAttempts); - Assert.True(releaseRetry.Writer.TryWrite(true)); - } - - harness.AssertSnapshot(await read, harness.Entries, rowCount); - Assert.All(harness.Entries, entry => Assert.Equal(2, - harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)))); - Assert.All(harness.Entries, entry => Assert.Equal(1, - harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress)))); - harness.AssertOneOwner(readOnly: true); - } - - [Fact] - public async Task ReadRetry_SerializedWaiterRevalidatesAfterEarlierRetryDisconnects() - { - const int rowCount = 2; - var harness = await Harness.CreateAsync(rowCount); - var monitor = CreateConnectionMonitor(); - harness.ConnectionMonitor = monitor; - await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); - var counts = new ConcurrentDictionary(StringComparer.Ordinal); - var secondRetryWarning = Gate(); - var retryEntered = Channel.CreateUnbounded(); - string? firstRetry = null; - harness.Logger.WarningRecorded = count => - { - if (count == rowCount + 1) - secondRetryWarning.TrySetResult(); - }; - harness.BeforeRequest = request => - { - if (!request.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) - || request.EndsWith("/IAmAlive", StringComparison.Ordinal)) - return Task.CompletedTask; - - var count = counts.AddOrUpdate(request, 1, static (_, value) => value + 1); - if (count == 1) - throw new KeeperException.ConnectionLossException(); - if (count == 2) - { - Assert.True(retryEntered.Writer.TryWrite(request)); - if (Interlocked.CompareExchange(ref firstRetry, request, null) is null) - throw new KeeperException.ConnectionLossException(); - } - - return Task.CompletedTask; - }; - - var read = harness.Read(TestContext.Current.CancellationToken); - Assert.Equal(rowCount, harness.Logger.Warnings.Count); - for (var i = 0; i < rowCount; i++) - Assert.Equal(TimeSpan.FromMilliseconds(250), await harness.Clock.NextTimerAsync()); - harness.Clock.Advance(TimeSpan.FromMilliseconds(250)); - await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); - _ = await retryEntered.Reader.ReadAsync(TestContext.Current.CancellationToken); - await secondRetryWarning.Task.WaitAsync(TestContext.Current.CancellationToken); - - Assert.False(retryEntered.Reader.TryRead(out _)); - Assert.Equal(rowCount + 1, harness.Calls.Count(call => - call.StartsWith("GetData /127.0.0.1", StringComparison.Ordinal) - && !call.EndsWith("/IAmAlive", StringComparison.Ordinal))); - - await Signal(monitor, Watcher.Event.KeeperState.Disconnected); - await Signal(monitor, Watcher.Event.KeeperState.SyncConnected); - _ = await retryEntered.Reader.ReadAsync(TestContext.Current.CancellationToken); - await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(500)); - - harness.AssertSnapshot(await read, harness.Entries, rowCount); - Assert.All(harness.Entries, entry => Assert.True( - harness.Calls.Count(call => call == "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress)) is 2 or 3)); - harness.AssertOneOwner(readOnly: true); - } - [Fact] public async Task ReadRetry_ReconnectTimeout_ReissuesAndPreservesNativeFailure() { @@ -464,13 +286,12 @@ public async Task ReadOwner_CanceledCaller_JoinsNativeTasksBeforeClose() var harness = await Harness.CreateAsync(); using var cancellation = new CancellationTokenSource(); var first = Gate(); - var second = Gate(); var firstCompleted = Gate(); var closeStarted = Gate(); var releaseClose = Gate(); var firstKey = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); var secondKey = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[1].SiloAddress); - harness.BeforeRequest = request => request == firstKey ? first.Task : request == secondKey ? second.Task : Task.CompletedTask; + harness.BeforeRequest = request => request == firstKey ? first.Task : Task.CompletedTask; harness.AfterRequest = request => { if (request == firstKey) @@ -486,7 +307,7 @@ public async Task ReadOwner_CanceledCaller_JoinsNativeTasksBeforeClose() var owner = Assert.Single(harness.Sessions); try { - Assert.Equal(new[] { "Sync /", "GetChildren /", firstKey, secondKey }, harness.Calls); + Assert.Equal(new[] { "Sync /", "GetChildren /", firstKey }, harness.Calls); cancellation.Cancel(); var failure = await Assert.ThrowsAnyAsync(() => read); Assert.Equal(cancellation.Token, failure.CancellationToken); @@ -496,21 +317,20 @@ public async Task ReadOwner_CanceledCaller_JoinsNativeTasksBeforeClose() await firstCompleted.Task.WaitAsync(TestContext.Current.CancellationToken); Assert.False(owner.Completion.IsCompleted); Assert.False(closeStarted.Task.IsCompleted); - second.SetResult(); await closeStarted.Task.WaitAsync(TestContext.Current.CancellationToken); Assert.False(owner.Completion.IsCompleted); + Assert.DoesNotContain(secondKey, harness.Calls); } finally { first.TrySetResult(); - second.TrySetResult(); releaseClose.TrySetResult(); await Record.ExceptionAsync(() => owner.Completion); } var ownedFailure = await Assert.ThrowsAnyAsync(() => owner.Completion); Assert.Equal(cancellation.Token, ownedFailure.CancellationToken); - Assert.Equal(new[] { "Sync /", "GetChildren /", firstKey, secondKey }, harness.Calls); + Assert.Equal(new[] { "Sync /", "GetChildren /", firstKey }, harness.Calls); harness.AssertOneOwner(readOnly: true); } @@ -671,7 +491,7 @@ public async Task Read_ConnectionLossDuringConcurrentMutation_RefencesWholeSnaps [Theory] [InlineData(9)] [InlineData(128)] - public async Task Read_RetryingOneRow_PreservesSuccessfulSiblings(int rowCount) + public async Task Read_RetryingRow_ContinuesSequentialSnapshotAfterRecovery(int rowCount) { var harness = await Harness.CreateAsync(rowCount); var key = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); @@ -688,7 +508,7 @@ public async Task Read_RetryingOneRow_PreservesSuccessfulSiblings(int rowCount) var read = harness.Read(TestContext.Current.CancellationToken); Assert.False(read.IsCompleted); Assert.All(harness.Entries.Skip(1), entry => - Assert.Contains("GetData " + ZooKeeperNativeFake.HeartbeatPath(entry.SiloAddress), harness.Calls)); + Assert.DoesNotContain("GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress), harness.Calls)); await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(250)); harness.AssertSnapshot(await read, harness.Entries, rowCount); @@ -703,7 +523,7 @@ public async Task Read_RetryingOneRow_PreservesSuccessfulSiblings(int rowCount) [InlineData(9, true)] [InlineData(128, false)] [InlineData(128, true)] - public async Task ReadOwner_StartsEveryRowBeforeAwaitingNativeCompletion(int rowCount, bool cancel) + public async Task ReadOwner_ReadsOneRowAtATimeAndOwnsAdmittedRequest(int rowCount, bool cancel) { var harness = await Harness.CreateAsync(rowCount); using var cancellation = new CancellationTokenSource(); @@ -714,8 +534,9 @@ public async Task ReadOwner_StartsEveryRowBeforeAwaitingNativeCompletion(int row var owner = Assert.Single(harness.Sessions); try { - Assert.Equal(new[] { "Sync /", "GetChildren /" }.Concat( - harness.Entries.Select(entry => "GetData " + ZooKeeperNativeFake.RowPath(entry.SiloAddress))), harness.Calls); + Assert.Equal( + new[] { "Sync /", "GetChildren /", "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress) }, + harness.Calls); if (cancel) { cancellation.Cancel(); @@ -733,7 +554,7 @@ public async Task ReadOwner_StartsEveryRowBeforeAwaitingNativeCompletion(int row { var failure = await Assert.ThrowsAnyAsync(() => owner.Completion); Assert.Equal(cancellation.Token, failure.CancellationToken); - Assert.Equal(2 + rowCount, harness.Calls.Count); + Assert.Equal(3, harness.Calls.Count); } else { @@ -743,45 +564,26 @@ public async Task ReadOwner_StartsEveryRowBeforeAwaitingNativeCompletion(int row harness.AssertOneOwner(readOnly: true); } - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task Read_MissingRow_PreservesOtherNativeFailureAfterJoining(bool connectionLoss) + [Fact] + public async Task Read_MissingRow_StopsBeforeLaterNativeFailure() { var harness = await Harness.CreateAsync(); - var release = Gate(); - Exception failure = connectionLoss ? new KeeperException.ConnectionLossException() : new KeeperException.NoAuthException(); + var missing = new KeeperException.NoNodeException(ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress)); + var laterFailure = new KeeperException.NoAuthException(); var first = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress); var second = "GetData " + ZooKeeperNativeFake.RowPath(harness.Entries[1].SiloAddress); - harness.BeforeRequest = async request => + harness.BeforeRequest = request => { if (request == first) - throw new KeeperException.NoNodeException(ZooKeeperNativeFake.RowPath(harness.Entries[0].SiloAddress)); + throw missing; if (request == second) - { - await release.Task; - throw failure; - } + throw laterFailure; + return Task.CompletedTask; }; - var read = harness.Read(TestContext.Current.CancellationToken); - var completion = Record.ExceptionAsync(() => read); - try - { - Assert.Contains(second, harness.Calls); - Assert.False(read.IsCompleted); - Assert.Equal(0, harness.CloseCount); - } - finally - { - release.TrySetResult(); - } - if (connectionLoss) - { - foreach (var delay in new[] { 250, 500, 1000, 2000 }) - await harness.Clock.AdvanceNextAsync(TimeSpan.FromMilliseconds(delay)); - } - Assert.Same(failure, await completion); - Assert.DoesNotContain("GetData /", harness.Calls); + + Assert.Same(missing, await Record.ExceptionAsync(() => harness.Read(TestContext.Current.CancellationToken))); + Assert.DoesNotContain(second, harness.Calls); + Assert.Contains("GetData /", harness.Calls); harness.AssertOneOwner(readOnly: true); } @@ -1014,8 +816,7 @@ public async Task NativeFixture_CanceledCleanup_PreservesAdmissionAndOwnsClose(b harness.Fake.Nodes[path] = harness.Fake.Nodes[path] with { Data = ZooKeeperBasedMembershipTable.Serialize(entry) }; } var first = Gate(); - var second = Gate(); - var firstCompleted = Gate(); + var firstSettled = Gate(); var closeStarted = Gate(); var releaseClose = Gate(); var failure = new KeeperException.NoAuthException(); @@ -1024,20 +825,19 @@ public async Task NativeFixture_CanceledCleanup_PreservesAdmissionAndOwnsClose(b harness.Fake.BeforeRead = async path => { if (path == firstPath) - await first.Task; - if (path == secondPath) { - await second.Task; - if (fail) - throw failure; + try + { + await first.Task; + if (fail) + throw failure; + } + finally + { + firstSettled.TrySetResult(); + } } }; - harness.Fake.AfterRead = path => - { - if (path == firstPath) - firstCompleted.SetResult(); - return Task.CompletedTask; - }; using var cancellation = new CancellationTokenSource(); async Task NativeOperation() { @@ -1058,22 +858,20 @@ async Task NativeOperation() try { Assert.Same(native, observed); - Assert.Equal(new[] { "children /", "read " + firstPath, "read " + secondPath }, harness.Fake.Calls); + Assert.Equal(new[] { "children /", "read " + firstPath }, harness.Fake.Calls); cancellation.Cancel(); var canceled = await Assert.ThrowsAnyAsync(() => caller); Assert.Equal(cancellation.Token, canceled.CancellationToken); Assert.False(observed!.IsCompleted); first.SetResult(); - await firstCompleted.Task.WaitAsync(TestContext.Current.CancellationToken); - Assert.False(closeStarted.Task.IsCompleted); - second.SetResult(); + await firstSettled.Task.WaitAsync(TestContext.Current.CancellationToken); await closeStarted.Task.WaitAsync(TestContext.Current.CancellationToken); Assert.False(observed.IsCompleted); + Assert.DoesNotContain("read " + secondPath, harness.Fake.Calls); } finally { first.TrySetResult(); - second.TrySetResult(); releaseClose.TrySetResult(); await Record.ExceptionAsync(() => native); } @@ -1083,7 +881,7 @@ async Task NativeOperation() else Assert.Equal(cancellation.Token, Assert.IsAssignableFrom(actual).CancellationToken); Assert.Empty(harness.Fake.Transactions); - Assert.Equal(new[] { "children /", "read " + firstPath, "read " + secondPath }, harness.Fake.Calls); + Assert.Equal(new[] { "children /", "read " + firstPath }, harness.Fake.Calls); } private sealed class Harness @@ -1251,7 +1049,6 @@ private sealed class RecordingLogger : ILogger { internal sealed record Warning(Exception? Exception, IReadOnlyDictionary Values); internal ConcurrentQueue Warnings { get; } = new(); - internal Action? WarningRecorded { get; set; } public IDisposable? BeginScope(TState state) where TState : notnull => null; public bool IsEnabled(LogLevel logLevel) => true; public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) @@ -1259,22 +1056,6 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except Assert.Equal(LogLevel.Warning, logLevel); var values = Assert.IsAssignableFrom>>(state); Warnings.Enqueue(new(exception, values.ToDictionary(pair => pair.Key, pair => pair.Value))); - WarningRecorded?.Invoke(Warnings.Count); - } - } - - private static class InterlockedExtensions - { - internal static void Max(ref int location, int value) - { - var current = Volatile.Read(ref location); - while (current < value) - { - var observed = Interlocked.CompareExchange(ref location, value, current); - if (observed == current) - return; - current = observed; - } } } } From 0972cfa6a8aa2ef4faafb79e3cdc4792cd8d87db Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Mon, 21 Sep 2026 12:00:03 -0700 Subject: [PATCH 10/10] docs(zookeeper): clarify bounded reconnect wait --- .../ZooKeeperBasedMembershipTable.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs index 8829216728c..18c83e15b6a 100644 --- a/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs +++ b/src/Orleans.Clustering.ZooKeeper/ZooKeeperBasedMembershipTable.cs @@ -150,7 +150,8 @@ await UsingZookeeper(rootConnectionString, async zk => /// /// /// Connection-loss failures in native reads are retried up to four times on the operation's session. - /// Each retry waits for that session's next connected event before issuing another request. + /// Before each retry, the operation waits up to the session timeout for that session's next connected event. + /// When the wait expires or the session becomes terminal, the retry proceeds and preserves the native outcome. /// The table and child versions fence each complete snapshot pass. Concurrent canonical /// modifications restart the pass up to five total attempts. /// @@ -176,7 +177,8 @@ public Task ReadRowAsync(SiloAddress siloAddress, Cancellat /// Table and child-version checks fence the complete snapshot. Concurrent canonical /// modifications restart the complete sequential pass up to five total attempts. /// Connection-loss failures in native reads are retried up to four times on the same session. - /// Each retry waits for that session's next connected event before issuing another request. + /// Before each retry, the operation waits up to the session timeout for that session's next connected event. + /// When the wait expires or the session becomes terminal, the retry proceeds and preserves the native outcome. /// Caller cancellation stops further requests while admitted requests and client close complete. /// public Task ReadAllAsync(CancellationToken cancellationToken = default)