From 95289da7c313a08278f4d6f0c207e3aaa505c053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Zborek?= Date: Mon, 10 Aug 2026 20:49:18 +0200 Subject: [PATCH 1/5] refactor(csharp): cleanup tcp connection after vsr implementation --- .../ConnectionStream/TcpConnectionStream.cs | 53 -- .../csharp/Iggy_SDK/Consumers/IggyConsumer.cs | 46 ++ .../Implementations/TcpMessageStream.Vsr.cs | 420 +++------------ .../Implementations/TcpMessageStream.cs | 477 ++++++------------ foreign/csharp/Iggy_SDK/Iggy_SDK.csproj | 12 +- .../csharp/Iggy_SDK/Mappers/BinaryMapper.cs | 4 +- .../{BufferSizes.cs => ArrayPoolHelper.cs} | 27 +- .../Iggy_SDK/Utils/TcpMessageStreamHelpers.cs | 19 - .../csharp/Iggy_SDK/Vsr/ConsensusSession.cs | 18 + .../ISessionEpochProvider.cs} | 14 +- foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs | 355 +++++++++++++ foreign/csharp/Iggy_SDK/Vsr/VsrHeader.cs | 12 +- foreign/csharp/Iggy_SDK/Vsr/VsrNamespace.cs | 51 +- .../ConsumerSessionEpochTests.cs | 197 ++++++++ .../ConsumerTests/RentedConsumerTests.cs | 4 +- .../MapperTests/BinaryMapper.cs | 10 +- .../UtilityTests/SlicedMemoryOwnerTests.cs | 17 +- .../Utils/Errors/ErrorModelFactory.cs | 33 -- .../VsrTests/ConsensusSessionTests.cs | 20 + .../Iggy_SDK_Tests/VsrTests/VsrHeaderTests.cs | 52 +- .../VsrTests/VsrNamespaceTests.cs | 53 +- 21 files changed, 1047 insertions(+), 847 deletions(-) delete mode 100644 foreign/csharp/Iggy_SDK/ConnectionStream/TcpConnectionStream.cs rename foreign/csharp/Iggy_SDK/Utils/{BufferSizes.cs => ArrayPoolHelper.cs} (51%) rename foreign/csharp/Iggy_SDK/{ConnectionStream/IConnectionStream.cs => Vsr/ISessionEpochProvider.cs} (64%) create mode 100644 foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs create mode 100644 foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs delete mode 100644 foreign/csharp/Iggy_SDK_Tests/Utils/Errors/ErrorModelFactory.cs diff --git a/foreign/csharp/Iggy_SDK/ConnectionStream/TcpConnectionStream.cs b/foreign/csharp/Iggy_SDK/ConnectionStream/TcpConnectionStream.cs deleted file mode 100644 index ce016a70f4..0000000000 --- a/foreign/csharp/Iggy_SDK/ConnectionStream/TcpConnectionStream.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -namespace Apache.Iggy.ConnectionStream; - -internal sealed class TcpConnectionStream : IConnectionStream -{ - private readonly Stream _stream; - - public TcpConnectionStream(Stream stream) - { - _stream = stream; - } - - public ValueTask SendAsync(ReadOnlyMemory payload, CancellationToken cancellationToken = default) - { - return _stream.WriteAsync(payload, cancellationToken); - } - - public ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) - { - return _stream.ReadAsync(buffer, cancellationToken); - } - - public Task FlushAsync(CancellationToken cancellationToken = default) - { - return _stream.FlushAsync(cancellationToken); - } - - public void Close() - { - _stream.Close(); - } - - public void Dispose() - { - _stream.Dispose(); - } -} diff --git a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs index b5317cfdef..f40cf62279 100644 --- a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs +++ b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs @@ -23,6 +23,7 @@ using Apache.Iggy.IggyClient; using Apache.Iggy.Kinds; using Apache.Iggy.Utils; +using Apache.Iggy.Vsr; using Microsoft.Extensions.Logging; namespace Apache.Iggy.Consumers; @@ -46,6 +47,13 @@ public partial class IggyConsumer : IAsyncDisposable private int _disposeState; private volatile bool _isInitialized; private volatile bool _joinedConsumerGroup; + + /// + /// Consensus session epoch the group membership was established under. A later epoch means the server + /// session that held the membership is gone and the group must be rejoined. + /// + private ulong _joinedSessionEpoch; + private long _lastPolledAtMs; /// Whether this consumer has been initialized via . @@ -263,8 +271,13 @@ private async Task InitializeConsumerGroupAsync(CancellationToken ct = default) return; } + // Captured before the join: a session reset racing the join lands the membership on a later epoch, and + // the mismatch triggers one redundant (idempotent) rejoin instead of a missed one. + var sessionEpoch = (_client as ISessionEpochProvider)?.SessionEpoch ?? 0; + if (_config.Consumer.Type == ConsumerType.Consumer) { + _joinedSessionEpoch = sessionEpoch; _joinedConsumerGroup = true; return; } @@ -306,6 +319,7 @@ private async Task InitializeConsumerGroupAsync(CancellationToken ct = default) await _client.JoinConsumerGroupAsync(_config.StreamId, _config.TopicId, Identifier.String(_consumerGroupName), ct); + _joinedSessionEpoch = sessionEpoch; _joinedConsumerGroup = true; LogConsumerGroupJoined(_consumerGroupName); } @@ -505,6 +519,38 @@ private async Task OnClientConnectionStateChangedAsync(ConnectionStateChangedEve await _connectionStateSemaphore.WaitAsync(); try { + if (_client is ISessionEpochProvider epochProvider) + { + // A plain consumer holds no membership, so its flag must never be cleared: nothing would ever + // set it again and every later poll would be skipped. + if (_config.Consumer.Type == ConsumerType.Consumer) + { + return; + } + + if (e.CurrentState != ConnectionState.Authenticated) + { + // Polling under a dropped session would be refused by the server for as long as the + // reconnect takes to re-authenticate, so the membership is surrendered up front. + if (e.CurrentState == ConnectionState.Disconnected) + { + _joinedConsumerGroup = false; + } + + return; + } + + if (_joinedConsumerGroup && epochProvider.SessionEpoch == _joinedSessionEpoch) + { + return; + } + + _joinedConsumerGroup = false; + await RejoinConsumerGroupOnReconnectionAsync(); + + return; + } + if (e.CurrentState == ConnectionState.Disconnected) { _joinedConsumerGroup = false; diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs index 81898c0994..7582bfaf7c 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs @@ -16,16 +16,15 @@ // under the License. using System.Buffers; -using System.Buffers.Binary; using System.IO.Hashing; using System.Runtime.ExceptionServices; -using Apache.Iggy.ConnectionStream; using Apache.Iggy.Contracts; using Apache.Iggy.Contracts.Auth; using Apache.Iggy.Contracts.Tcp; using Apache.Iggy.Enums; using Apache.Iggy.Exceptions; using Apache.Iggy.Kinds; +using Apache.Iggy.Mappers; using Apache.Iggy.Messages; using Apache.Iggy.Utils; using Apache.Iggy.Vsr; @@ -39,7 +38,7 @@ namespace Apache.Iggy.IggyClient.Implementations; /// register handshake, and the client-side partitioning and consumer-group assignment the broker does not /// resolve server-side. The command surface lives in . /// -public sealed partial class TcpMessageStream +public sealed partial class TcpMessageStream : ISessionEpochProvider { /// /// Upper bound for a whole VSR request: the transient replays and the leader failovers share it, and so @@ -48,15 +47,6 @@ public sealed partial class TcpMessageStream /// private const int VsrRequestTimeoutMs = 30_000; - /// Backoff between replays of a transiently refused request. - private const int VsrTransientRetryIntervalMs = 50; - - /// - /// Largest body still sent as one contiguous frame with its header. Beyond this the copy outweighs the - /// syscall and the extra segment it saves, so header and body go out as two writes. - /// - private const int VsrContiguousFrameLimit = 4 * 1024; - /// /// How long a request replays on the same connection /// before the leader roster is re-checked. A node that stopped being primary refuses forever, so @@ -75,9 +65,8 @@ public sealed partial class TcpMessageStream /// /// Cap on consecutive leader redirects, so a flapping roster cannot spin the connect loop or the - /// transient failover path. The budget is client-wide and resets on a roster check that finds the - /// current node is the leader, and on every request that completes, so a client that outlives more - /// leader changes than the cap does not latch onto a follower for good. + /// transient failover path. Each operation - a request, a connect, a register - spends its own local + /// budget, so a long-lived client never latches onto a follower for good. /// private const int VsrMaxLeaderRedirects = 3; @@ -108,12 +97,9 @@ public sealed partial class TcpMessageStream private readonly ConsensusSession _consensusSession = new(); private readonly ConsumerGroupClientState _groupState = new(); - private readonly byte[] _vsrReplyHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; - // The redirect budget is refunded by a completed request, and the roster check a redirect runs is itself a - // request. Without this the refund lands between the check and the increment that reads the budget, and the - // counter never leaves zero. Nonzero for the duration of a roster read, so that refund is skipped. - private int _leaderProbeDepth; + /// + ulong ISessionEpochProvider.SessionEpoch => _consensusSession.Epoch; /// /// Runs the consensus register handshake and binds the session it commits. Everything before the bind @@ -138,16 +124,13 @@ public sealed partial class TcpMessageStream SetConnectionStateAsync(ConnectionState.Connected); } - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, code); - SetConnectionStateAsync(ConnectionState.Authenticating); LoginRegisterResponse response; try { - Interlocked.Exchange(ref _skipAutoLoginOnce, 1); - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer = + await SendWithResponseAsync(code, 0, message, token, false); response = LoginRegister.Deserialize(responseBuffer.Memory.Span); _consensusSession.Bind(response.Session); @@ -162,10 +145,6 @@ public sealed partial class TcpMessageStream throw; } - finally - { - Interlocked.Exchange(ref _skipAutoLoginOnce, 0); - } _logger.LogInformation( "Authenticated against the server, version {ServerVersion}, protocol version {ServerProtocolVersion}", @@ -328,10 +307,8 @@ private async Task SyncGroupAssignmentAsync(Identifier streamId, Identifier topi CancellationToken token) { var message = TcpContracts.GetGroup(streamId, topicId, groupId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.SYNC_CONSUMER_GROUP_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer = + await SendWithResponseAsync(CommandCodes.SYNC_CONSUMER_GROUP_CODE, message, token); var key = new GroupKey(streamId, topicId, groupId); if (responseBuffer.Memory.Length == 0) @@ -373,9 +350,9 @@ private async Task RefreshGroupAssignmentsAsync(CancellationToken token) /// /// Points the client at the current leader when it is not the node this connection is on, leaving the - /// stream closed for the caller to reconnect. The redirect budget is client-wide: the connect loop and - /// the transient failover path spend the same counter, and it is refunded as soon as a roster check - /// lands on the leader. + /// stream closed for the caller to reconnect. The caller owns the redirect budget: every operation that + /// follows redirects - a request, a connect, a register - counts them locally against + /// . /// private async Task RedirectAsync(CancellationToken token) { @@ -384,20 +361,12 @@ private async Task RedirectAsync(CancellationToken token) var currentLeaderNode = await GetCurrentLeaderNodeAsync(token); if (currentLeaderNode == null) { - Interlocked.Exchange(ref _leaderRedirectCount, 0); return false; } var leaderAddress = ServerAddress.HostPort(currentLeaderNode.Ip, currentLeaderNode.Endpoints.Tcp); if (ServerAddress.IsSame(leaderAddress, _currentAddress)) { - Interlocked.Exchange(ref _leaderRedirectCount, 0); - return false; - } - - if (Interlocked.Increment(ref _leaderRedirectCount) > VsrMaxLeaderRedirects) - { - _logger.LogWarning("Maximum leader redirections reached, continuing on {Address}", _currentAddress); return false; } @@ -410,7 +379,7 @@ private async Task RedirectAsync(CancellationToken token) try { _currentAddress = leaderAddress; - DropVsrConnectionLocked(_stream); + DropVsrConnectionLocked(_connection); } finally { @@ -423,12 +392,11 @@ private async Task RedirectAsync(CancellationToken token) private async Task GetCurrentLeaderNodeAsync(CancellationToken token) { var leaderlessDeadline = Environment.TickCount64 + VsrLeaderlessWaitMs; - Interlocked.Increment(ref _leaderProbeDepth); try { while (true) { - var clusterMetadata = await GetClusterMetadataAsync(token); + var clusterMetadata = await ReadClusterMetadataNoRedirectAsync(token); if (clusterMetadata == null) { return null; @@ -471,32 +439,52 @@ private async Task RedirectAsync(CancellationToken token) return null; } - finally + } + + /// + /// Reads the roster without following redirects: the probe is what redirects are decided from, so a + /// probe that redirected or reconnected would reenter the very loop that called it. A probe the current + /// node keeps refusing simply fails, and the caller stays where it is. + /// + private async Task ReadClusterMetadataNoRedirectAsync(CancellationToken token) + { + using IMemoryOwner responseBuffer = await SendRawAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, 0, + ReadOnlyMemory.Empty, token, false); + + if (responseBuffer.Memory.Length == 0) { - Interlocked.Decrement(ref _leaderProbeDepth); + return null; } + + return BinaryMapper.MapClusterMetadata(responseBuffer.Memory.Span); } /// - /// Sends a consensus-framed request. The call sites still build the classic - /// [size u32][code u32][body] buffer, so the code is read back from it here and the body is written - /// right after the 256-byte consensus header - two writes, no concatenation. + /// Sends a consensus-framed request: the body is written right after the 256-byte consensus header - + /// two writes, no concatenation. /// /// /// One deadline bounds the whole request across transient replays AND leader failovers. Login and /// register replay on this connection for the whole budget instead: the connect flow owns leader /// redirection for the handshake, and reconnecting from underneath it would recurse. /// - private async Task> SendRawVsrAsync(ReadOnlyMemory payload, CancellationToken token) + private async Task> SendRawAsync(int code, ulong ns, ReadOnlyMemory body, + CancellationToken token, bool allowRedirect = true) { - var code = (int)BinaryPrimitives.ReadUInt32LittleEndian(payload.Span.Slice(4, 4)); - ReadOnlyMemory body = payload[8..]; + ObjectDisposedException.ThrowIf(_disposed, this); + + if (_state is ConnectionState.Disconnected or ConnectionState.Connecting) + { + throw new NotConnectedException(); + } + var isLoginRegister = code is CommandCodes.LOGIN_REGISTER_CODE or CommandCodes.LOGIN_REGISTER_WITH_PAT_CODE; var overallDeadline = Environment.TickCount64 + VsrRequestTimeoutMs; var headerBuffer = ArrayPool.Shared.Rent(VsrHeader.HEADER_SIZE); Memory header = headerBuffer.AsMemory(0, VsrHeader.HEADER_SIZE); var requestEncoded = false; - TcpConnectionStream? lastStream = null; + var redirects = 0; + VsrConnection? lastConnection = null; try { @@ -506,20 +494,13 @@ private async Task> SendRawVsrAsync(ReadOnlyMemory payl ? overallDeadline : Math.Min(overallDeadline, Environment.TickCount64 + VsrTransientFailoverCheckMs); - var attempt = await SendVsrAttemptAsync(code, body, header, transientDeadline, overallDeadline, + var attempt = await SendVsrAttemptAsync(code, ns, body, header, transientDeadline, overallDeadline, token); requestEncoded |= attempt.Encoded; - lastStream = attempt.Stream; + lastConnection = attempt.Connection; if (attempt.Error is null) { - // A roster read taken by RedirectAsync must not refund the budget it is about to be charged - // against, or the cap can never be reached. - if (Volatile.Read(ref _leaderRedirectCount) != 0 && Volatile.Read(ref _leaderProbeDepth) == 0) - { - Interlocked.Exchange(ref _leaderRedirectCount, 0); - } - return attempt.Response!; } @@ -528,10 +509,12 @@ private async Task> SendRawVsrAsync(ReadOnlyMemory payl StatusCode: VsrError.TRANSIENT_NOT_ACCEPTED, FromServer: true } && !isLoginRegister + && allowRedirect && Environment.TickCount64 < overallDeadline) { - if (await RedirectAsync(token)) + if (redirects < VsrMaxLeaderRedirects && await RedirectAsync(token)) { + redirects++; await ConnectAsync(token); } @@ -562,7 +545,7 @@ private async Task> SendRawVsrAsync(ReadOnlyMemory payl { if (requestEncoded) { - await DropVsrConnectionAsync(lastStream); + await DropVsrConnectionAsync(lastConnection); } throw; @@ -588,236 +571,31 @@ private static bool IsDefinitiveVerdict(Exception error) } /// - /// One attempt on the current connection: encode the header into , write the - /// frame, and replay it - same session, same request id - while the server answers transiently. + /// One attempt on the current connection, resolved under the sending lock so the frame and its reply + /// cannot be split across two sockets, and so a teardown after the lock is gone can tell this + /// connection from a replacement a reconnect installed since. /// - private async ValueTask SendVsrAttemptAsync(int code, ReadOnlyMemory body, Memory header, - long transientDeadline, long readDeadline, CancellationToken token) + private async ValueTask SendVsrAttemptAsync(int code, ulong ns, ReadOnlyMemory body, + Memory header, long transientDeadline, long readDeadline, CancellationToken token) { await _sendingSemaphore.WaitAsync(token); - - var encoded = false; - var requestStarted = false; - byte[]? frameBuffer = null; - - // Read the stream once for the whole attempt. Nothing may swap the field without the sending lock this - // call holds, so the frame and its reply cannot be split across two sockets, and a teardown after the - // lock is gone can tell this connection from a replacement a reconnect installed since. - var stream = _stream; try { - // A small request goes out as one write. Two writes cost two syscalls and, with Nagle disabled, two - // TCP segments (two TLS records when encrypted) for what the reference encoder sends as a single - // contiguous frame. Above the threshold the copy costs more than the extra write saves. Renting - // inside the try keeps the semaphore paired with its release even if the pool throws. - if (body.Length <= VsrContiguousFrameLimit) + var connection = _connection; + if (connection is null) { - frameBuffer = ArrayPool.Shared.Rent(VsrHeader.HEADER_SIZE + body.Length); + return VsrAttempt.Failed(false, new NotConnectedException(), false, null); } - VsrHeader.EncodeRequestHeader(header.Span, _consensusSession, code, body.Span); - - encoded = true; - - var frame = Memory.Empty; - if (frameBuffer is not null) - { - frame = frameBuffer.AsMemory(0, VsrHeader.HEADER_SIZE + body.Length); - header.CopyTo(frame); - body.CopyTo(frame[VsrHeader.HEADER_SIZE..]); - } - - while (true) - { - try - { - // Everything that fails without reaching the socket has to fail before this point: past it a - // failure is reported as an outcome the server alone knows, which for a replicated write - // tells the caller its request may have committed twice. - token.ThrowIfCancellationRequested(); - requestStarted = true; - - if (frameBuffer is not null) - { - await stream.SendAsync(frame, token); - } - else - { - await stream.SendAsync(header, token); - await stream.SendAsync(body, token); - } - - await stream.FlushAsync(token); - - IMemoryOwner response = await ReadVsrReplyAsync(stream, readDeadline, token); - - return VsrAttempt.Ok(response, stream); - } - catch (IggyInvalidStatusCodeException e) when (IsReplayableTransient(e, transientDeadline, - readDeadline)) - { - var governingDeadline = e.StatusCode == VsrError.TRANSIENT_NOT_COMMITTED - ? readDeadline - : transientDeadline; - var remaining = governingDeadline - Environment.TickCount64; - await Task.Delay((int)Math.Clamp(remaining, 0, VsrTransientRetryIntervalMs), token); - } - catch (Exception e) when (IsConnectionException(e)) - { - DropVsrConnectionLocked(stream); - - return VsrAttempt.Failed(encoded, e, requestStarted, stream); - } - catch (OperationCanceledException e) - { - DropVsrConnectionLocked(stream); - - return VsrAttempt.Failed(encoded, e, requestStarted, stream); - } - catch (Exception e) - { - return VsrAttempt.Failed(encoded, e, requestStarted, stream); - } - } - } - catch (OperationCanceledException e) - { - if (encoded) - { - DropVsrConnectionLocked(stream); - } - - return VsrAttempt.Failed(encoded, e, requestStarted, stream); - } - catch (Exception e) - { - return VsrAttempt.Failed(encoded, e, requestStarted, stream); + return await connection.SendAttemptAsync(code, ns, body, header, transientDeadline, readDeadline, + token); } finally { - if (frameBuffer is not null) - { - ArrayPool.Shared.Return(frameBuffer); - } - _sendingSemaphore.Release(); } } - private async Task> ReadVsrReplyAsync(TcpConnectionStream stream, long readDeadline, - CancellationToken token) - { - var remaining = readDeadline - Environment.TickCount64; - if (remaining <= 0) - { - throw new IOException($"Timed out after {VsrRequestTimeoutMs} ms waiting for a consensus reply."); - } - - // One timer for the whole reply: the deadline covers the frame, not each partial read, so a per-read - // source would both re-arm the budget and allocate a timer per socket read. - using var readCancellation = CancellationTokenSource.CreateLinkedTokenSource(token); - readCancellation.CancelAfter((int)Math.Min(remaining, VsrRequestTimeoutMs)); - - await ReadExactVsrAsync(stream, _vsrReplyHeaderBuffer, readCancellation.Token, token); - - var command = VsrHeader.PeekCommand(_vsrReplyHeaderBuffer); - if (command == Command2.Eviction) - { - var eviction = VsrHeader.ReadEviction(_vsrReplyHeaderBuffer); - _logger.LogWarning("Consensus session evicted by the server: {Reason}", eviction.Reason); - DropVsrConnectionLocked(stream); - - throw new VsrSessionEvictedException(VsrReplyDecoder.ToException(eviction)); - } - - if (command != Command2.Reply) - { - // Neither a reply nor an eviction: this frame was never an answer to the outstanding request, so - // whatever the peer does send for it would be read as the next request's reply and handed to the - // wrong caller. The size field of a frame the client cannot model is no basis for resynchronising. - DropVsrConnectionLocked(stream); - - throw VsrError.Exception(VsrError.INVALID_COMMAND, - $"Unexpected consensus frame {command} on a client connection."); - } - - int bodySize; - try - { - bodySize = VsrReplyDecoder.ReadBodySize(_vsrReplyHeaderBuffer); - if (VsrHeader.HEADER_SIZE + (long)bodySize > _configuration.MaxResponseFrameSize) - { - throw VsrError.Exception(VsrError.INVALID_COMMAND, - $"Reply frame of {VsrHeader.HEADER_SIZE + bodySize} bytes exceeds the configured maximum of " + - $"{_configuration.MaxResponseFrameSize} bytes."); - } - } - catch - { - // An announced size the client refuses to read - undersized, oversized - leaves the body on the - // wire, so the stream no longer sits on a frame boundary and the next reply would decode body bytes - // as a header. - DropVsrConnectionLocked(stream); - - throw; - } - - if (bodySize == 0) - { - VsrReplyDecoder.Decode(_vsrReplyHeaderBuffer, ReadOnlyMemory.Empty); - - return EmptyMemoryOwner.Instance; - } - - var buffer = ArrayPool.Shared.Rent(bodySize); - try - { - await ReadExactVsrAsync(stream, buffer.AsMemory(0, bodySize), readCancellation.Token, token); - ReadOnlyMemory decoded = VsrReplyDecoder.Decode(_vsrReplyHeaderBuffer, buffer.AsMemory(0, bodySize)); - if (decoded.IsEmpty) - { - ArrayPool.Shared.Return(buffer); - - return EmptyMemoryOwner.Instance; - } - - // The decoded payload is always a suffix of the body - the funnel only strips the leading - // committed result section. - return new PooledMemoryOwner(buffer, bodySize - decoded.Length, decoded.Length); - } - catch - { - ArrayPool.Shared.Return(buffer); - throw; - } - } - - private async ValueTask ReadExactVsrAsync(TcpConnectionStream stream, Memory buffer, - CancellationToken readToken, - CancellationToken token) - { - var totalRead = 0; - while (totalRead < buffer.Length) - { - int readBytes; - try - { - readBytes = await stream.ReadAsync(buffer[totalRead..], readToken); - } - catch (OperationCanceledException) when (!token.IsCancellationRequested) - { - throw new IOException($"Timed out after {VsrRequestTimeoutMs} ms waiting for a consensus reply."); - } - - if (readBytes == 0) - { - throw new IggyZeroBytesException(); - } - - totalRead += readBytes; - } - } - /// /// Drops the consensus session and the group state scoped to it. Consumer-group assignments are fenced by /// a generation the coordinator tracks per session, so carrying them into a new session would fence every @@ -873,27 +651,27 @@ private async ValueTask TryEnterSendingSemaphoreAsync() /// /// Drops the connection along with the session. A late or half-read reply would desync the framing of the - /// next request, so the stream cannot be reused. The caller must hold , - /// which owns every write to . + /// next request, so the socket cannot be reused. The caller must hold , + /// which owns every write to . /// - /// + /// /// The connection the caller was using. A reconnect that completed in the meantime already closed it and /// re-armed the session, so dropping anything but the live one would tear down a healthy replacement. /// - private void DropVsrConnectionLocked(TcpConnectionStream? stream) + private void DropVsrConnectionLocked(VsrConnection? connection) { - if (!ReferenceEquals(_stream, stream)) + if (connection is null || !ReferenceEquals(_connection, connection)) { return; } ResetConsensusSession(); - _stream?.Close(); + connection.Close(); SetConnectionStateAsync(ConnectionState.Disconnected); } /// Drops the connection on behalf of a caller that no longer holds the sending lock. - private async ValueTask DropVsrConnectionAsync(TcpConnectionStream? stream) + private async ValueTask DropVsrConnectionAsync(VsrConnection? connection) { // Dispose already closed the stream, and taking a disposed semaphore here would replace the // cancellation the caller is about to rethrow with an ObjectDisposedException. Dispose can still land @@ -906,73 +684,11 @@ private async ValueTask DropVsrConnectionAsync(TcpConnectionStream? stream) try { - DropVsrConnectionLocked(stream); + DropVsrConnectionLocked(connection); } finally { _sendingSemaphore.Release(); } } - - private static bool IsReplayableTransient(IggyInvalidStatusCodeException error, long transientDeadline, - long readDeadline) - { - if (!error.FromServer) - { - return false; - } - - return error.StatusCode switch - { - VsrError.TRANSIENT_NOT_COMMITTED => Environment.TickCount64 < readDeadline, - VsrError.TRANSIENT_NOT_ACCEPTED => Environment.TickCount64 < transientDeadline, - _ => false - }; - } - - /// Outcome of one call on the current connection. - /// Whether the header was encoded, i.e. whether a request id may have been consumed. - /// The decoded reply payload, non-null exactly when is null. - /// The failure that ended the attempt, or null on success. - /// - /// Whether any byte of the frame was written, which makes the server-side outcome unknowable on failure. - /// - /// - /// The connection the attempt ran on, so a caller that drops it after releasing the sending lock can tell - /// its own connection from a replacement a reconnect installed since. - /// - private readonly record struct VsrAttempt( - bool Encoded, - IMemoryOwner? Response, - Exception? Error, - bool RequestStarted, - TcpConnectionStream? Stream) - { - public static VsrAttempt Ok(IMemoryOwner response, TcpConnectionStream stream) - { - return new VsrAttempt(true, response, null, true, stream); - } - - public static VsrAttempt Failed(bool encoded, Exception error, bool requestStarted, - TcpConnectionStream? stream) - { - return new VsrAttempt(encoded, null, error, requestStarted, stream); - } - } - - /// Owns a pooled buffer while exposing only the decoded payload slice inside it. - internal sealed class PooledMemoryOwner(byte[] buffer, int start, int length) : IMemoryOwner - { - private int _disposed; - - public Memory Memory => buffer.AsMemory(start, length); - - public void Dispose() - { - if (Interlocked.Exchange(ref _disposed, 1) == 0) - { - ArrayPool.Shared.Return(buffer); - } - } - } } diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs index 146c93ee13..f0b2fc5cf7 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs @@ -19,11 +19,9 @@ using System.Buffers.Binary; using System.Net.Security; using System.Net.Sockets; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Security.Cryptography.X509Certificates; using Apache.Iggy.Configuration; -using Apache.Iggy.ConnectionStream; using Apache.Iggy.Contracts; using Apache.Iggy.Contracts.Auth; using Apache.Iggy.Contracts.Tcp; @@ -46,8 +44,6 @@ namespace Apache.Iggy.IggyClient.Implementations; /// public sealed partial class TcpMessageStream : IIggyClient { - private const int InvalidCommandStatus = 3; - private static readonly HashSet SessionControlCodes = [ CommandCodes.LOGIN_USER_CODE, @@ -63,20 +59,13 @@ public sealed partial class TcpMessageStream : IIggyClient private readonly SemaphoreSlim _connectionSemaphore; private readonly ILogger _logger; private readonly SemaphoreSlim _sendingSemaphore; + private VsrConnection? _connection; private string _currentAddress = string.Empty; private X509Certificate2Collection _customCaStore = []; private volatile bool _disposed; private int _isConnecting; private DateTimeOffset _lastConnectionTime; - private int _leaderRedirectCount; - - // Both are written by the connect and redirect paths, which do not hold the sending semaphore the request - // paths read them under, so they are accessed through Interlocked rather than as plain fields. Losing an - // update to the skip flag leaves a connection reporting Connected that never authenticated; losing one to - // the redirect counter over- or under-spends the redirect budget. - private int _skipAutoLoginOnce; private volatile ConnectionState _state = ConnectionState.Disconnected; - private TcpConnectionStream _stream = null!; private bool IsConnecting => Volatile.Read(ref _isConnecting) != 0; @@ -96,8 +85,8 @@ internal TcpMessageStream(IggyClientConfigurator configuration, ILoggerFactory l public void Dispose() { _disposed = true; - _stream?.Close(); - _stream?.Dispose(); + _connection?.Close(); + _connection?.Dispose(); SetConnectionStateAsync(ConnectionState.Disconnected); _sendingSemaphore.Dispose(); @@ -131,10 +120,8 @@ public string GetCurrentAddress() public async Task CreateStreamAsync(string name, CancellationToken token = default) { var message = TcpContracts.CreateStream(name); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CREATE_STREAM_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.CREATE_STREAM_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -148,10 +135,8 @@ public string GetCurrentAddress() public async Task GetStreamByIdAsync(Identifier streamId, CancellationToken token = default) { var message = TcpMessageStreamHelpers.GetBytesFromIdentifier(streamId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_STREAM_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_STREAM_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -165,10 +150,8 @@ public string GetCurrentAddress() public async Task> GetStreamsAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_STREAMS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_STREAMS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -182,30 +165,21 @@ public async Task> GetStreamsAsync(CancellationTok public async Task UpdateStreamAsync(Identifier streamId, string name, CancellationToken token = default) { var message = TcpContracts.UpdateStream(streamId, name); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.UPDATE_STREAM_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.UPDATE_STREAM_CODE, message, token); } /// public async Task PurgeStreamAsync(Identifier streamId, CancellationToken token = default) { var message = TcpMessageStreamHelpers.GetBytesFromIdentifier(streamId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.PURGE_STREAM_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.PURGE_STREAM_CODE, message, token); } /// public async Task DeleteStreamAsync(Identifier streamId, CancellationToken token = default) { var message = TcpMessageStreamHelpers.GetBytesFromIdentifier(streamId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_STREAM_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_STREAM_CODE, message, token); } /// @@ -213,10 +187,8 @@ public async Task> GetTopicsAsync(Identifier stream CancellationToken token = default) { var message = TcpMessageStreamHelpers.GetBytesFromIdentifier(streamId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_TOPICS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_TOPICS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -231,10 +203,8 @@ public async Task> GetTopicsAsync(Identifier stream CancellationToken token = default) { var message = TcpContracts.GetTopicById(streamId, topicId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_TOPIC_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_TOPIC_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -252,10 +222,8 @@ public async Task> GetTopicsAsync(Identifier stream var messageExpiryValue = DurationHelpers.ToDuration(messageExpiry); var message = TcpContracts.CreateTopic(streamId, name, partitionsCount, compressionAlgorithm, replicationFactor, messageExpiryValue, maxTopicSize); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CREATE_TOPIC_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.CREATE_TOPIC_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -274,20 +242,14 @@ public async Task UpdateTopicAsync(Identifier streamId, Identifier topicId, stri var messageExpiryValue = DurationHelpers.ToDuration(messageExpiry); var message = TcpContracts.UpdateTopic(streamId, topicId, name, compressionAlgorithm, maxTopicSize, messageExpiryValue, replicationFactor); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.UPDATE_TOPIC_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.UPDATE_TOPIC_CODE, message, token); } /// public async Task DeleteTopicAsync(Identifier streamId, Identifier topicId, CancellationToken token = default) { var message = TcpContracts.DeleteTopic(streamId, topicId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_TOPIC_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_TOPIC_CODE, message, token); _groupState.InvalidatePartitionCount(new TopicKey(streamId, topicId)); } @@ -295,10 +257,7 @@ public async Task DeleteTopicAsync(Identifier streamId, Identifier topicId, Canc public async Task PurgeTopicAsync(Identifier streamId, Identifier topicId, CancellationToken token = default) { var message = TcpContracts.PurgeTopic(streamId, topicId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.PURGE_TOPIC_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.PURGE_TOPIC_CODE, message, token); } @@ -371,11 +330,9 @@ public Task PollMessagesRentedAsync(Identifier streamId, I public async Task StoreOffsetAsync(Consumer consumer, Identifier streamId, Identifier topicId, ulong offset, uint? partitionId, CancellationToken token = default) { + var ns = ConsumerOffsetNamespace(streamId, topicId, partitionId); var message = TcpContracts.UpdateOffset(streamId, topicId, consumer, offset, partitionId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.STORE_CONSUMER_OFFSET_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.STORE_CONSUMER_OFFSET_CODE, ns, message, token); } /// @@ -383,10 +340,8 @@ public async Task StoreOffsetAsync(Consumer consumer, Identifier streamId, Ident uint? partitionId, CancellationToken token = default) { var message = TcpContracts.GetOffset(streamId, topicId, consumer, partitionId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_CONSUMER_OFFSET_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_OFFSET_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -400,11 +355,9 @@ public async Task StoreOffsetAsync(Consumer consumer, Identifier streamId, Ident public async Task DeleteOffsetAsync(Consumer consumer, Identifier streamId, Identifier topicId, uint? partitionId, CancellationToken token = default) { + var ns = ConsumerOffsetNamespace(streamId, topicId, partitionId); var message = TcpContracts.DeleteOffset(streamId, topicId, consumer, partitionId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_CONSUMER_OFFSET_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_CONSUMER_OFFSET_CODE, ns, message, token); } /// @@ -413,10 +366,8 @@ public async Task> GetConsumerGroupsAsync(I CancellationToken token = default) { var message = TcpContracts.GetGroups(streamId, topicId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_CONSUMER_GROUPS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_GROUPS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -431,10 +382,8 @@ public async Task> GetConsumerGroupsAsync(I Identifier groupId, CancellationToken token = default) { var message = TcpContracts.GetGroup(streamId, topicId, groupId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_CONSUMER_GROUP_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_GROUP_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -449,10 +398,8 @@ public async Task> GetConsumerGroupsAsync(I string name, CancellationToken token = default) { var message = TcpContracts.CreateGroup(streamId, topicId, name); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CREATE_CONSUMER_GROUP_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.CREATE_CONSUMER_GROUP_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -467,10 +414,7 @@ public async Task DeleteConsumerGroupAsync(Identifier streamId, Identifier topic CancellationToken token = default) { var message = TcpContracts.DeleteGroup(streamId, topicId, groupId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_CONSUMER_GROUP_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_CONSUMER_GROUP_CODE, message, token); _groupState.DeregisterGroup(new GroupKey(streamId, topicId, groupId)); } @@ -479,10 +423,7 @@ public async Task JoinConsumerGroupAsync(Identifier streamId, Identifier topicId CancellationToken token = default) { var message = TcpContracts.JoinGroup(streamId, topicId, groupId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.JOIN_CONSUMER_GROUP_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.JOIN_CONSUMER_GROUP_CODE, message, token); // A join rebalances the group, so whatever this client holds for it is a generation behind and every // poll under it would be fenced until the first re-sync. @@ -494,10 +435,7 @@ public async Task LeaveConsumerGroupAsync(Identifier streamId, Identifier topicI CancellationToken token = default) { var message = TcpContracts.LeaveGroup(streamId, topicId, groupId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.LEAVE_CONSUMER_GROUP_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.LEAVE_CONSUMER_GROUP_CODE, message, token); _groupState.DeregisterGroup(new GroupKey(streamId, topicId, groupId)); } @@ -506,10 +444,7 @@ public async Task DeletePartitionsAsync(Identifier streamId, Identifier topicId, CancellationToken token = default) { var message = TcpContracts.DeletePartitions(streamId, topicId, partitionsCount); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_PARTITIONS_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_PARTITIONS_CODE, message, token); _groupState.InvalidatePartitionCount(new TopicKey(streamId, topicId)); } @@ -518,10 +453,7 @@ public async Task CreatePartitionsAsync(Identifier streamId, Identifier topicId, CancellationToken token = default) { var message = TcpContracts.CreatePartitions(streamId, topicId, partitionsCount); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CREATE_PARTITIONS_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.CREATE_PARTITIONS_CODE, message, token); _groupState.InvalidatePartitionCount(new TopicKey(streamId, topicId)); } @@ -529,21 +461,16 @@ public async Task CreatePartitionsAsync(Identifier streamId, Identifier topicId, public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, uint partitionId, uint segmentsCount, CancellationToken token = default) { + var ns = VsrNamespace.ForPartition(streamId, topicId, partitionId); var message = TcpContracts.DeleteSegments(streamId, topicId, partitionId, segmentsCount); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_SEGMENTS_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_SEGMENTS_CODE, ns, message, token); } /// public async Task GetMeAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_ME_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer = await SendWithResponseAsync(CommandCodes.GET_ME_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -557,10 +484,8 @@ public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, u public async Task GetStatsAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_STATS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_STATS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -574,10 +499,8 @@ public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, u public async Task GetClusterMetadataAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_CLUSTER_METADATA_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -591,10 +514,7 @@ public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, u public async Task PingAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.PING_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.PING_CODE, message, token); await RefreshGroupAssignmentsAsync(token); } @@ -604,10 +524,7 @@ public async Task GetSnapshotAsync(SnapshotCompression compression, IList snapshotTypes, CancellationToken token = default) { var message = TcpContracts.GetSnapshot(compression, snapshotTypes); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_SNAPSHOT_CODE); - - using IMemoryOwner result = await SendWithResponseAsync(payload, token); + using IMemoryOwner result = await SendWithResponseAsync(CommandCodes.GET_SNAPSHOT_CODE, message, token); return result.Memory.Span.ToArray(); } @@ -617,14 +534,12 @@ public async Task SendBinaryRequestAsync(uint code, byte[] payload, Canc { if (SessionControlCodes.Contains(code)) { - throw new IggyInvalidStatusCodeException(InvalidCommandStatus, - $"Invalid response status code: {InvalidCommandStatus}"); + throw VsrError.Exception(VsrError.INVALID_COMMAND, + $"Command {code} cannot be sent as a raw binary request."); } - var buffer = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + payload.Length]; - TcpMessageStreamHelpers.CreatePayload(buffer, payload, (int)code); - - using IMemoryOwner result = await SendWithResponseAsync(buffer, token); + var ns = VsrNamespace.ForRequest((int)code, payload, VsrOperations.ForCode((int)code)); + using IMemoryOwner result = await SendWithResponseAsync((int)code, ns, payload, token); return result.Memory.Length <= 1 ? [] : result.Memory.Span.ToArray(); } @@ -639,10 +554,8 @@ public Task ConnectAsync(CancellationToken token = default) public async Task> GetClientsAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_CLIENTS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_CLIENTS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -656,10 +569,8 @@ public async Task> GetClientsAsync(CancellationTok public async Task GetClientByIdAsync(uint clientId, CancellationToken token = default) { var message = TcpContracts.GetClient(clientId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_CLIENT_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_CLIENT_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -673,10 +584,8 @@ public async Task> GetClientsAsync(CancellationTok public async Task GetUserAsync(Identifier userId, CancellationToken token = default) { var message = TcpContracts.GetUser(userId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_USER_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_USER_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -690,10 +599,8 @@ public async Task> GetClientsAsync(CancellationTok public async Task> GetUsersAsync(CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_USERS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_USERS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -708,10 +615,8 @@ public async Task> GetUsersAsync(CancellationToken t Permissions? permissions = null, CancellationToken token = default) { var message = TcpContracts.CreateUser(userName, password, status, permissions); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CREATE_USER_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.CREATE_USER_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -725,10 +630,7 @@ public async Task> GetUsersAsync(CancellationToken t public async Task DeleteUserAsync(Identifier userId, CancellationToken token = default) { var message = TcpContracts.DeleteUser(userId); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_USER_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_USER_CODE, message, token); } /// @@ -736,10 +638,7 @@ public async Task UpdateUserAsync(Identifier userId, string? userName = null, Us CancellationToken token = default) { var message = TcpContracts.UpdateUser(userId, userName, status); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.UPDATE_USER_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.UPDATE_USER_CODE, message, token); } /// @@ -747,10 +646,7 @@ public async Task UpdatePermissionsAsync(Identifier userId, Permissions? permiss CancellationToken token = default) { var message = TcpContracts.UpdatePermissions(userId, permissions); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.UPDATE_PERMISSIONS_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.UPDATE_PERMISSIONS_CODE, message, token); } /// @@ -758,10 +654,7 @@ public async Task ChangePasswordAsync(Identifier userId, string currentPassword, CancellationToken token = default) { var message = TcpContracts.ChangePassword(userId, currentPassword, newPassword); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CHANGE_PASSWORD_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.CHANGE_PASSWORD_CODE, message, token); } /// @@ -779,13 +672,9 @@ public async Task ChangePasswordAsync(Identifier userId, string currentPassword, /// public async Task LogoutUserAsync(CancellationToken token = default) { - var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.LOGOUT_USER_CODE); - try { - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.LOGOUT_USER_CODE, ReadOnlyMemory.Empty, token); } finally { @@ -803,10 +692,8 @@ public async Task> GetPersonalAccessT CancellationToken token = default) { var message = Array.Empty(); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.GET_PERSONAL_ACCESS_TOKENS_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.GET_PERSONAL_ACCESS_TOKENS_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -821,10 +708,8 @@ public async Task> GetPersonalAccessT CancellationToken token = default) { var message = TcpContracts.CreatePersonalAccessToken(name, DurationHelpers.ToDuration(expiry)); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.CREATE_PERSONAL_ACCESS_TOKEN_CODE); - - using IMemoryOwner responseBuffer = await SendWithResponseAsync(payload, token); + using IMemoryOwner responseBuffer + = await SendWithResponseAsync(CommandCodes.CREATE_PERSONAL_ACCESS_TOKEN_CODE, message, token); if (responseBuffer.Memory.Length == 0) { @@ -838,10 +723,7 @@ public async Task> GetPersonalAccessT public async Task DeletePersonalAccessTokenAsync(string name, CancellationToken token = default) { var message = TcpContracts.DeletePersonalRequestToken(name); - var payload = new byte[4 + BufferSizes.INITIAL_BYTES_LENGTH + message.Length]; - TcpMessageStreamHelpers.CreatePayload(payload, message, CommandCodes.DELETE_PERSONAL_ACCESS_TOKEN_CODE); - - await SendAckAsync(payload, token); + await SendAckAsync(CommandCodes.DELETE_PERSONAL_ACCESS_TOKEN_CODE, message, token); } /// @@ -897,18 +779,16 @@ private async Task PollPartitionMessagesRentedAsync(Identi CancellationToken token) { var messageBufferSize = CalculateMessageBufferSize(streamId, topicId, consumer); - var payloadBufferSize = CalculatePayloadBufferSize(messageBufferSize); - var payload = ArrayPool.Shared.Rent(payloadBufferSize); + var payload = ArrayPool.Shared.Rent(messageBufferSize); IMemoryOwner? responseBuffer = null; try { - TcpContracts.GetMessages(payload.AsSpan().Slice(8, messageBufferSize), consumer, streamId, + TcpContracts.GetMessages(payload.AsSpan(0, messageBufferSize), consumer, streamId, topicId, pollingStrategy, count, autoCommit, partitionId); - BinaryPrimitives.WriteInt32LittleEndian(payload.AsSpan()[..4], messageBufferSize + 4); - BinaryPrimitives.WriteInt32LittleEndian(payload.AsSpan()[4..8], CommandCodes.POLL_MESSAGES_CODE); - responseBuffer = await SendWithResponseAsync(payload.AsMemory(0, payloadBufferSize), token); + responseBuffer = await SendWithResponseAsync(CommandCodes.POLL_MESSAGES_CODE, + payload.AsMemory(0, messageBufferSize), token); if (responseBuffer.Memory.Length == 0) { responseBuffer.Dispose(); @@ -952,15 +832,14 @@ private Task SendMessagesCoreAsync(Identifier streamId, Id + 2 + partitioning.Length + 4 + 4; var maxMessageBufferSize = TcpMessageStreamHelpers.CalculateMessageBytesCount(messages, encryptor) + metadataLength; - var maxPayloadBufferSize = CalculatePayloadBufferSize(maxMessageBufferSize); - IMemoryOwner payloadBuffer = MemoryPool.Shared.Rent(maxPayloadBufferSize); - int payloadBufferSize; + var ns = SendMessagesNamespace(streamId, topicId, partitioning); + IMemoryOwner payloadBuffer = MemoryPool.Shared.Rent(maxMessageBufferSize); + int bodySize; try { - var messageBufferSize = FillSendMessagesPayload(payloadBuffer.Memory.Span, maxMessageBufferSize, - streamId, topicId, partitioning, messages, encryptor); - payloadBufferSize = CalculatePayloadBufferSize(messageBufferSize); + bodySize = TcpContracts.CreateMessage(payloadBuffer.Memory.Span[..maxMessageBufferSize], streamId, + topicId, partitioning, messages, encryptor); } catch { @@ -968,16 +847,16 @@ private Task SendMessagesCoreAsync(Identifier streamId, Id throw; } - return SendConfirmedAndDisposeAsync(payloadBuffer, payloadBufferSize, token); + return SendConfirmedAndDisposeAsync(ns, payloadBuffer, bodySize, token); } - private async Task SendConfirmedAndDisposeAsync(IMemoryOwner payloadBuffer, - int payloadBufferSize, CancellationToken token) + private async Task SendConfirmedAndDisposeAsync(ulong ns, IMemoryOwner payloadBuffer, + int bodySize, CancellationToken token) { try { - using IMemoryOwner responseBuffer = - await SendWithResponseAsync(payloadBuffer.Memory[..payloadBufferSize], token); + using IMemoryOwner responseBuffer = await SendWithResponseAsync(CommandCodes.SEND_MESSAGES_CODE, + ns, payloadBuffer.Memory[..bodySize], token); return BinaryMapper.MapSendMessages(responseBuffer.Memory.Span); } finally @@ -996,16 +875,42 @@ private static ReadOnlySpan AsSpan(IList messages) }; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int FillSendMessagesPayload(Span buffer, int maxMessageBufferSize, - Identifier streamId, Identifier topicId, Partitioning partitioning, ReadOnlySpan messages, - IMessageEncryptor? encryptor) + /// + /// The routing namespace for a send: callers resolve balanced and message-key partitioning to an + /// explicit partition before framing, so anything else reaching this point is a bug surfaced as the + /// same error the server-side router would raise. + /// + private static ulong SendMessagesNamespace(Identifier streamId, Identifier topicId, Partitioning partitioning) { - var messageBufferSize = TcpContracts.CreateMessage(buffer.Slice(8, maxMessageBufferSize), streamId, topicId, - partitioning, messages, encryptor); - BinaryPrimitives.WriteInt32LittleEndian(buffer[..4], messageBufferSize + 4); - BinaryPrimitives.WriteInt32LittleEndian(buffer[4..8], CommandCodes.SEND_MESSAGES_CODE); - return messageBufferSize; + if (partitioning.Kind != Enums.Partitioning.PartitionId) + { + throw VsrError.Exception(VsrError.FEATURE_UNAVAILABLE, + "Under VSR the partition must be resolved by the client; balanced and message-key partitioning cannot be sent on the wire."); + } + + if (partitioning.Value.Length < 4) + { + throw VsrError.Exception(VsrError.INVALID_COMMAND, + $"Partition id is {partitioning.Value.Length} bytes, expected 4."); + } + + return VsrNamespace.ForPartition(streamId, topicId, + BinaryPrimitives.ReadUInt32LittleEndian(partitioning.Value)); + } + + /// + /// The routing namespace for a consumer-offset write. The broker routes explicit partitions only, so a + /// missing partition id fails client-side before a request id is consumed. + /// + private static ulong ConsumerOffsetNamespace(Identifier streamId, Identifier topicId, uint? partitionId) + { + if (partitionId is null) + { + throw VsrError.Exception(VsrError.INVALID_IDENTIFIER, + "Under VSR a consumer-offset request must carry an explicit partition id."); + } + + return VsrNamespace.ForPartition(streamId, topicId, partitionId.Value); } private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken token) @@ -1015,12 +920,12 @@ private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken var delay = _configuration.ReconnectionSettings.InitialDelay; do { - // The sending semaphore owns every write to _stream, so an in-flight request never observes the - // field changing between its write and its reply. + // The sending semaphore owns every write to _connection, so an in-flight request never observes + // the field changing between its write and its reply. await _sendingSemaphore.WaitAsync(token); try { - _stream?.Dispose(); + _connection?.Dispose(); ResetConsensusSession(); } @@ -1056,16 +961,16 @@ private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); socket.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveTime, 5); - var connectionStream = _configuration.TlsSettings.Enabled switch - { - true => await CreateSslStreamAndAuthenticate(socket, _configuration.TlsSettings), - false => new TcpConnectionStream(new NetworkStream(socket, true)) - }; + var connectionStream = _configuration.TlsSettings.Enabled + ? await CreateSslStreamAndAuthenticate(socket, _configuration.TlsSettings) + : new NetworkStream(socket, true); await _sendingSemaphore.WaitAsync(token); try { - _stream = connectionStream; + _connection = new VsrConnection(connectionStream, _consensusSession, + _configuration.MaxResponseFrameSize, VsrRequestTimeoutMs, DropVsrConnectionLocked, + _logger); } finally { @@ -1083,7 +988,7 @@ private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken continue; } - if (autoLogin && _configuration.AutoLoginSettings.Enabled && !ConsumeSkipAutoLogin()) + if (autoLogin && _configuration.AutoLoginSettings.Enabled) { _logger.LogInformation("Auto login enabled. Trying to login with credentials: {Username}", _configuration.AutoLoginSettings.Username); @@ -1151,23 +1056,7 @@ async Task BackoffOrThrowAsync() } } - /// - /// Whether this connect was triggered by a login or register request that will re-authenticate itself, - /// so the auto-login must sit this one out. Consumes the flag. - /// - private bool ConsumeSkipAutoLogin() - { - if (Interlocked.Exchange(ref _skipAutoLoginOnce, 0) == 0) - { - return false; - } - - _logger.LogInformation("Skipping auto login for a replayed register request"); - - return true; - } - - private async Task CreateSslStreamAndAuthenticate(Socket socket, TlsSettings tlsSettings) + private async Task CreateSslStreamAndAuthenticate(Socket socket, TlsSettings tlsSettings) { ValidateCertificatePath(tlsSettings.CertificatePath); @@ -1178,22 +1067,33 @@ private async Task CreateSslStreamAndAuthenticate(Socket so await sslStream.AuthenticateAsClientAsync(tlsSettings.Hostname); - return new TcpConnectionStream(sslStream); + return sslStream; } - private async Task SendAckAsync(ReadOnlyMemory payload, CancellationToken token = default) + private async Task SendAckAsync(int code, ReadOnlyMemory body, CancellationToken token) { - using IMemoryOwner _ = await SendWithResponseAsync(payload, token); + using IMemoryOwner _ = await SendWithResponseAsync(code, body, token); } - private async Task> SendWithResponseAsync(ReadOnlyMemory payload, - CancellationToken token = default) + private async Task SendAckAsync(int code, ulong ns, ReadOnlyMemory body, CancellationToken token) + { + using IMemoryOwner _ = await SendWithResponseAsync(code, ns, body, token); + } + + private Task> SendWithResponseAsync(int code, ReadOnlyMemory body, + CancellationToken token) + { + return SendWithResponseAsync(code, 0, body, token); + } + + private async Task> SendWithResponseAsync(int code, ulong ns, ReadOnlyMemory body, + CancellationToken token, bool autoLoginOnReconnect = true) { try { - return await SendRawAsync(payload, token); + return await SendRawAsync(code, ns, body, token); } - catch (Exception e) when (IsConnectionException(e) && !IsConnecting && !_disposed) + catch (Exception e) when (VsrConnection.IsConnectionException(e) && !IsConnecting && !_disposed) { _logger.LogWarning("Connection lost"); if (!_configuration.ReconnectionSettings.Enabled) @@ -1203,12 +1103,12 @@ private async Task> SendWithResponseAsync(ReadOnlyMemory> HandleReconnectionAsync(ReadOnlyMemory payload, - CancellationToken token) + private async Task> HandleReconnectionAsync(int code, ulong ns, ReadOnlyMemory body, + bool autoLogin, CancellationToken token) { var currentTime = DateTimeOffset.UtcNow; await _connectionSemaphore.WaitAsync(token); @@ -1219,18 +1119,18 @@ private async Task> HandleReconnectionAsync(ReadOnlyMemory currentTime) { _logger.LogInformation("Connection already established, sending payload"); - return await SendRawAsync(payload, token); + return await SendRawAsync(code, ns, body, token); } SetConnectionStateAsync(ConnectionState.Disconnected); _logger.LogInformation("Reconnecting to the server"); - await ConnectAsync(token); + await ConnectAsync(autoLogin, token); _logger.LogInformation("Reconnected to the server"); await Task.Delay(_configuration.ReconnectionSettings.WaitAfterReconnect, token); - return await SendRawAsync(payload, token); + return await SendRawAsync(code, ns, body, token); } finally { @@ -1238,32 +1138,6 @@ private async Task> HandleReconnectionAsync(ReadOnlyMemory> SendRawAsync(ReadOnlyMemory payload, CancellationToken token) - { - ObjectDisposedException.ThrowIf(_disposed, this); - - if (_state is ConnectionState.Disconnected or ConnectionState.Connecting) - { - throw new NotConnectedException(); - } - - return SendRawVsrAsync(payload, token); - } - - private static bool IsConnectionException(Exception ex) - { - return ex is IggyZeroBytesException or - NotConnectedException or - SocketException or - IOException or - ObjectDisposedException; - } - - private static int CalculatePayloadBufferSize(int messageBufferSize) - { - return messageBufferSize + 4 + BufferSizes.INITIAL_BYTES_LENGTH; - } - private static int CalculateMessageBufferSize(Identifier streamId, Identifier topicId, Consumer consumer) { // Original: 14 + 5 + 2 + streamId.Length + 2 + topicId.Length + 2 + consumer.Id.Length @@ -1361,45 +1235,4 @@ private bool RemoteCertificateValidationCallback(object sender, X509Certificate? return false; } - - internal sealed class EmptyMemoryOwner : IMemoryOwner - { - public static readonly EmptyMemoryOwner Instance = new(); - - private EmptyMemoryOwner() - { - } - - public Memory Memory => Memory.Empty; - - public void Dispose() - { - } - } -} - -internal static class ArrayPoolHelper -{ - public static SlicedMemoryOwner Rent(int minimumLength, bool clearOnReturn = false) - { - return new SlicedMemoryOwner(minimumLength, clearOnReturn); - } - - internal sealed class SlicedMemoryOwner(int minimumLength, bool clearOnReturn = false) : IMemoryOwner - { - private readonly byte[] _value = ArrayPool.Shared.Rent(minimumLength); - private int _disposed; - - public Memory Memory => _value.AsMemory()[..minimumLength]; - - public void Dispose() - { - if (Interlocked.Exchange(ref _disposed, 1) != 0) - { - return; - } - - ArrayPool.Shared.Return(_value, clearOnReturn); - } - } } diff --git a/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj b/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj index 79a6390366..84fd522853 100644 --- a/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj +++ b/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj @@ -27,7 +27,7 @@ net8.0;net10.0 Apache.Iggy Apache.Iggy - 0.9.0-edge.1 + 0.9.0-edge.2 true @@ -68,13 +68,15 @@ - - + + - - + + + + diff --git a/foreign/csharp/Iggy_SDK/Mappers/BinaryMapper.cs b/foreign/csharp/Iggy_SDK/Mappers/BinaryMapper.cs index f5be657f20..257d401930 100644 --- a/foreign/csharp/Iggy_SDK/Mappers/BinaryMapper.cs +++ b/foreign/csharp/Iggy_SDK/Mappers/BinaryMapper.cs @@ -25,9 +25,9 @@ using Apache.Iggy.Exceptions; using Apache.Iggy.Extensions; using Apache.Iggy.Headers; -using Apache.Iggy.IggyClient.Implementations; using Apache.Iggy.Messages; using Apache.Iggy.Utils; +using Apache.Iggy.Vsr; namespace Apache.Iggy.Mappers; @@ -563,7 +563,7 @@ internal static PolledMessagesRental ToRentedMessages(PolledMessages messages) }); } - return new PolledMessagesRental(TcpMessageStream.EmptyMemoryOwner.Instance) + return new PolledMessagesRental(EmptyMemoryOwner.Instance) { PartitionId = messages.PartitionId, CurrentOffset = messages.CurrentOffset, diff --git a/foreign/csharp/Iggy_SDK/Utils/BufferSizes.cs b/foreign/csharp/Iggy_SDK/Utils/ArrayPoolHelper.cs similarity index 51% rename from foreign/csharp/Iggy_SDK/Utils/BufferSizes.cs rename to foreign/csharp/Iggy_SDK/Utils/ArrayPoolHelper.cs index cd5a9e871f..0db7b1cce4 100644 --- a/foreign/csharp/Iggy_SDK/Utils/BufferSizes.cs +++ b/foreign/csharp/Iggy_SDK/Utils/ArrayPoolHelper.cs @@ -15,9 +15,32 @@ // specific language governing permissions and limitations // under the License. +using System.Buffers; + namespace Apache.Iggy.Utils; -internal static class BufferSizes +internal static class ArrayPoolHelper { - internal const int INITIAL_BYTES_LENGTH = 4; + public static SlicedMemoryOwner Rent(int minimumLength, bool clearOnReturn = false) + { + return new SlicedMemoryOwner(minimumLength, clearOnReturn); + } + + internal sealed class SlicedMemoryOwner(int minimumLength, bool clearOnReturn = false) : IMemoryOwner + { + private readonly byte[] _value = ArrayPool.Shared.Rent(minimumLength); + private int _disposed; + + public Memory Memory => _value.AsMemory()[..minimumLength]; + + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0) + { + return; + } + + ArrayPool.Shared.Return(_value, clearOnReturn); + } + } } diff --git a/foreign/csharp/Iggy_SDK/Utils/TcpMessageStreamHelpers.cs b/foreign/csharp/Iggy_SDK/Utils/TcpMessageStreamHelpers.cs index 6f0dc135ca..cbf00ccfa4 100644 --- a/foreign/csharp/Iggy_SDK/Utils/TcpMessageStreamHelpers.cs +++ b/foreign/csharp/Iggy_SDK/Utils/TcpMessageStreamHelpers.cs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -using System.Buffers.Binary; using System.Runtime.CompilerServices; using Apache.Iggy.Contracts.Tcp; using Apache.Iggy.Encryption; @@ -26,24 +25,6 @@ namespace Apache.Iggy.Utils; internal static class TcpMessageStreamHelpers { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static void CreatePayload(Span result, Span message, int command) - { - var messageLength = message.Length + 4; - BinaryPrimitives.WriteInt32LittleEndian(result[..4], messageLength); - BinaryPrimitives.WriteInt32LittleEndian(result[4..8], command); - message.CopyTo(result[8..]); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static (int Status, int Length) GetResponseLengthAndStatus(Span buffer) - { - var status = BinaryPrimitives.ReadInt32LittleEndian(buffer[..4]); - var length = BinaryPrimitives.ReadInt32LittleEndian(buffer[4..]); - - return (status, length); - } - internal static int CalculateMessageBytesCount(ReadOnlySpan messages, IMessageEncryptor? encryptor) { var bytesCount = 0; diff --git a/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs b/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs index 8cdbdcc16c..5eda4d0d40 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs @@ -38,6 +38,7 @@ internal sealed class ConsensusSession private readonly object _gate = new(); #endif private UInt128 _clientId; + private ulong _epoch; private bool _registerPending; private ulong _requestCounter; private ulong? _session; @@ -89,6 +90,22 @@ internal bool IsBound } } + /// + /// Generation of the identity, bumped on every re-arm. Anything scoped to a server session - a + /// consumer-group membership, say - compares the epoch it was established under to detect that the + /// session it lived on is gone, without inferring it from connection-state edges. + /// + internal ulong Epoch + { + get + { + lock (_gate) + { + return _epoch; + } + } + } + internal ConsensusSession() : this(GenerateClientId()) { } @@ -205,6 +222,7 @@ private void ReArmLocked() _session = null; _requestCounter = 1; _registerPending = false; + _epoch++; } private static UInt128 GenerateClientId() diff --git a/foreign/csharp/Iggy_SDK/ConnectionStream/IConnectionStream.cs b/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs similarity index 64% rename from foreign/csharp/Iggy_SDK/ConnectionStream/IConnectionStream.cs rename to foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs index 288ba53c35..f81d2ebf41 100644 --- a/foreign/csharp/Iggy_SDK/ConnectionStream/IConnectionStream.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs @@ -15,12 +15,14 @@ // specific language governing permissions and limitations // under the License. -namespace Apache.Iggy.ConnectionStream; +namespace Apache.Iggy.Vsr; -internal interface IConnectionStream : IDisposable +/// +/// Exposes the consensus session generation of a transport, so session-scoped state - a consumer-group +/// membership - can detect the session it was established under is gone. Transports without a consensus +/// session (HTTP) do not implement it and their consumers fall back to connection-state edges. +/// +internal interface ISessionEpochProvider { - ValueTask SendAsync(ReadOnlyMemory payload, CancellationToken cancellationToken = default); - ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default); - Task FlushAsync(CancellationToken cancellationToken = default); - void Close(); + ulong SessionEpoch { get; } } diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs new file mode 100644 index 0000000000..3029747101 --- /dev/null +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs @@ -0,0 +1,355 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +using System.Buffers; +using System.Net.Sockets; +using Apache.Iggy.Exceptions; +using Microsoft.Extensions.Logging; + +namespace Apache.Iggy.Vsr; + +/// +/// One consensus-framed socket: encodes request frames, replays them while the server answers +/// transiently, and reads and decodes reply frames. The caller owns the sending lock that serialises +/// attempts, the leader redirection above this layer, and the reconnect that replaces a dropped +/// connection. +/// +internal sealed class VsrConnection : IDisposable +{ + /// Backoff between replays of a transiently refused request. + private const int TransientRetryIntervalMs = 50; + + private readonly ILogger _logger; + private readonly long _maxResponseFrameSize; + + /// + /// Tears this connection down at the transport level - session reset, state event - when a frame-level + /// failure makes the socket unusable. Runs under the sending lock the caller holds, and it is the + /// transport's job to ignore the call when a reconnect already replaced this connection. + /// + private readonly Action _onDropped; + + private readonly byte[] _replyHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; + private readonly int _requestTimeoutMs; + private readonly Stream _stream; + + /// The session identity requests on this connection encode from. + internal ConsensusSession Session { get; } + + internal VsrConnection(Stream stream, ConsensusSession session, long maxResponseFrameSize, int requestTimeoutMs, + Action onDropped, ILogger logger) + { + _stream = stream; + Session = session; + _maxResponseFrameSize = maxResponseFrameSize; + _requestTimeoutMs = requestTimeoutMs; + _onDropped = onDropped; + _logger = logger; + } + + public void Dispose() + { + _stream.Dispose(); + } + + internal void Close() + { + _stream.Close(); + } + + internal static bool IsConnectionException(Exception ex) + { + return ex is IggyZeroBytesException or + NotConnectedException or + SocketException or + IOException or + ObjectDisposedException; + } + + /// + /// One attempt on this connection: encode the header into , write the frame, + /// and replay it - same session, same request id - while the server answers transiently. The caller + /// must hold the sending lock. + /// + internal async ValueTask SendAttemptAsync(int code, ulong ns, ReadOnlyMemory body, + Memory header, long transientDeadline, long readDeadline, CancellationToken token) + { + var encoded = false; + var requestStarted = false; + + try + { + VsrHeader.EncodeRequestHeader(header.Span, Session, code, ns, body.Length); + + encoded = true; + + while (true) + { + try + { + // Everything that fails without reaching the socket has to fail before this point: past it a + // failure is reported as an outcome the server alone knows, which for a replicated write + // tells the caller its request may have committed twice. + token.ThrowIfCancellationRequested(); + requestStarted = true; + + // Header and body go out as two writes, straight from the buffers they were encoded into: + // no frame-sized rent and no copy. The server frames by the announced size, and the + // lockstep protocol already pays a full round trip per request, so the extra segment a + // small request costs (with Nagle disabled) is noise against the reply wait. + await _stream.WriteAsync(header, token); + if (!body.IsEmpty) + { + await _stream.WriteAsync(body, token); + } + + await _stream.FlushAsync(token); + + IMemoryOwner response = await ReadReplyAsync(readDeadline, token); + + return VsrAttempt.Ok(response, this); + } + catch (IggyInvalidStatusCodeException e) when (IsReplayableTransient(e, transientDeadline, + readDeadline)) + { + var governingDeadline = e.StatusCode == VsrError.TRANSIENT_NOT_COMMITTED + ? readDeadline + : transientDeadline; + var remaining = governingDeadline - Environment.TickCount64; + await Task.Delay((int)Math.Clamp(remaining, 0, TransientRetryIntervalMs), token); + } + catch (Exception e) when (IsConnectionException(e)) + { + _onDropped(this); + + return VsrAttempt.Failed(encoded, e, requestStarted, this); + } + catch (OperationCanceledException e) + { + _onDropped(this); + + return VsrAttempt.Failed(encoded, e, requestStarted, this); + } + catch (Exception e) + { + return VsrAttempt.Failed(encoded, e, requestStarted, this); + } + } + } + catch (OperationCanceledException e) + { + if (encoded) + { + _onDropped(this); + } + + return VsrAttempt.Failed(encoded, e, requestStarted, this); + } + catch (Exception e) + { + return VsrAttempt.Failed(encoded, e, requestStarted, this); + } + } + + private async Task> ReadReplyAsync(long readDeadline, CancellationToken token) + { + var remaining = readDeadline - Environment.TickCount64; + if (remaining <= 0) + { + throw new IOException($"Timed out after {_requestTimeoutMs} ms waiting for a consensus reply."); + } + + // One timer for the whole reply: the deadline covers the frame, not each partial read, so a per-read + // source would both re-arm the budget and allocate a timer per socket read. + using var readCancellation = CancellationTokenSource.CreateLinkedTokenSource(token); + readCancellation.CancelAfter((int)Math.Min(remaining, _requestTimeoutMs)); + + await ReadExactAsync(_replyHeaderBuffer, readCancellation.Token, token); + + var command = VsrHeader.PeekCommand(_replyHeaderBuffer); + if (command == Command2.Eviction) + { + var eviction = VsrHeader.ReadEviction(_replyHeaderBuffer); + _logger.LogWarning("Consensus session evicted by the server: {Reason}", eviction.Reason); + _onDropped(this); + + throw new VsrSessionEvictedException(VsrReplyDecoder.ToException(eviction)); + } + + if (command != Command2.Reply) + { + // Neither a reply nor an eviction: this frame was never an answer to the outstanding request, so + // whatever the peer does send for it would be read as the next request's reply and handed to the + // wrong caller. The size field of a frame the client cannot model is no basis for resynchronising. + _onDropped(this); + + throw VsrError.Exception(VsrError.INVALID_COMMAND, + $"Unexpected consensus frame {command} on a client connection."); + } + + int bodySize; + try + { + bodySize = VsrReplyDecoder.ReadBodySize(_replyHeaderBuffer); + if (VsrHeader.HEADER_SIZE + (long)bodySize > _maxResponseFrameSize) + { + throw VsrError.Exception(VsrError.INVALID_COMMAND, + $"Reply frame of {VsrHeader.HEADER_SIZE + bodySize} bytes exceeds the configured maximum of " + + $"{_maxResponseFrameSize} bytes."); + } + } + catch + { + // An announced size the client refuses to read - undersized, oversized - leaves the body on the + // wire, so the stream no longer sits on a frame boundary and the next reply would decode body bytes + // as a header. + _onDropped(this); + + throw; + } + + if (bodySize == 0) + { + VsrReplyDecoder.Decode(_replyHeaderBuffer, ReadOnlyMemory.Empty); + + return EmptyMemoryOwner.Instance; + } + + var buffer = ArrayPool.Shared.Rent(bodySize); + try + { + await ReadExactAsync(buffer.AsMemory(0, bodySize), readCancellation.Token, token); + ReadOnlyMemory decoded = VsrReplyDecoder.Decode(_replyHeaderBuffer, buffer.AsMemory(0, bodySize)); + if (decoded.IsEmpty) + { + ArrayPool.Shared.Return(buffer); + + return EmptyMemoryOwner.Instance; + } + + // The decoded payload is always a suffix of the body - the funnel only strips the leading + // committed result section. + return new PooledMemoryOwner(buffer, bodySize - decoded.Length, decoded.Length); + } + catch + { + ArrayPool.Shared.Return(buffer); + throw; + } + } + + private async ValueTask ReadExactAsync(Memory buffer, CancellationToken readToken, CancellationToken token) + { + var totalRead = 0; + while (totalRead < buffer.Length) + { + int readBytes; + try + { + readBytes = await _stream.ReadAsync(buffer[totalRead..], readToken); + } + catch (OperationCanceledException) when (!token.IsCancellationRequested) + { + throw new IOException($"Timed out after {_requestTimeoutMs} ms waiting for a consensus reply."); + } + + if (readBytes == 0) + { + throw new IggyZeroBytesException(); + } + + totalRead += readBytes; + } + } + + private static bool IsReplayableTransient(IggyInvalidStatusCodeException error, long transientDeadline, + long readDeadline) + { + if (!error.FromServer) + { + return false; + } + + return error.StatusCode switch + { + VsrError.TRANSIENT_NOT_COMMITTED => Environment.TickCount64 < readDeadline, + VsrError.TRANSIENT_NOT_ACCEPTED => Environment.TickCount64 < transientDeadline, + _ => false + }; + } +} + +/// Outcome of one call. +/// Whether the header was encoded, i.e. whether a request id may have been consumed. +/// The decoded reply payload, non-null exactly when is null. +/// The failure that ended the attempt, or null on success. +/// +/// Whether any byte of the frame was written, which makes the server-side outcome unknowable on failure. +/// +/// +/// The connection the attempt ran on, so a caller that drops it after releasing the sending lock can tell +/// its own connection from a replacement a reconnect installed since. +/// +internal readonly record struct VsrAttempt( + bool Encoded, + IMemoryOwner? Response, + Exception? Error, + bool RequestStarted, + VsrConnection? Connection) +{ + public static VsrAttempt Ok(IMemoryOwner response, VsrConnection connection) + { + return new VsrAttempt(true, response, null, true, connection); + } + + public static VsrAttempt Failed(bool encoded, Exception error, bool requestStarted, VsrConnection? connection) + { + return new VsrAttempt(encoded, null, error, requestStarted, connection); + } +} + +/// Shared empty reply payload; disposes to nothing, so it is safe to hand out repeatedly. +internal sealed class EmptyMemoryOwner : IMemoryOwner +{ + public static readonly EmptyMemoryOwner Instance = new(); + + private EmptyMemoryOwner() + { + } + + public Memory Memory => Memory.Empty; + + public void Dispose() + { + } +} + +/// Owns a pooled buffer while exposing only the decoded payload slice inside it. +internal sealed class PooledMemoryOwner(byte[] buffer, int start, int length) : IMemoryOwner +{ + private int _disposed; + + public Memory Memory => buffer.AsMemory(start, length); + + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 0) + { + ArrayPool.Shared.Return(buffer); + } + } +} diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrHeader.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrHeader.cs index 5ebb96e839..dbd00f28e7 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/VsrHeader.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrHeader.cs @@ -49,8 +49,8 @@ internal static class VsrHeader internal const int EVICTION_REASON_OFFSET = 255; /// - /// Encodes the request header for a classic command code and its body. Returns the total frame size - /// (header plus body). + /// Encodes the request header for a command code, the namespace the caller resolved from its typed + /// identifiers, and the body length. Returns the total frame size (header plus body). /// /// /// Everything that can fail runs before a request id is consumed. The primary accepts any id above the @@ -58,7 +58,7 @@ internal static class VsrHeader /// different request would let the client table answer that request from the first one's cached reply. /// internal static int EncodeRequestHeader(Span header, ConsensusSession session, int code, - ReadOnlySpan payload) + ulong partitionNamespace, int bodyLength) { if (header.Length < HEADER_SIZE) { @@ -69,15 +69,15 @@ internal static int EncodeRequestHeader(Span header, ConsensusSession sess header.Clear(); var operation = VsrOperations.ForCode(code); - var ns = VsrNamespace.ForRequest(code, payload, operation); + var ns = VsrNamespace.ForOperation(operation, partitionNamespace); - if (payload.Length > int.MaxValue - HEADER_SIZE) + if (bodyLength > int.MaxValue - HEADER_SIZE) { throw VsrError.Exception(VsrError.INVALID_COMMAND, "Request body exceeds the maximum frame size."); } var frame = session.Resolve(operation); - var totalSize = HEADER_SIZE + payload.Length; + var totalSize = HEADER_SIZE + bodyLength; BinaryPrimitives.WriteUInt32LittleEndian(header[SIZE_OFFSET..], (uint)totalSize); header[COMMAND_OFFSET] = (byte)Command2.Request; diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrNamespace.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrNamespace.cs index 34187591dd..b27bcd038b 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/VsrNamespace.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrNamespace.cs @@ -56,9 +56,38 @@ internal static class VsrNamespace private const byte ConsumerGroupKind = 2; /// - /// The namespace a request header must carry, peeking into the classic payload for the ops the server - /// routes by partition. Named (string) stream or topic identifiers resolve to 0; the server resolves - /// them itself. + /// The namespace a request header carries for its operation. Register and logout ride the metadata + /// consensus group; non-replicated and metadata operations route by 0; partition-plane operations carry + /// the namespace the caller packed from its typed identifiers. + /// + internal static ulong ForOperation(VsrOperation operation, ulong partitionNamespace) + { + if (operation is VsrOperation.Register or VsrOperation.Logout) + { + return METADATA_CONSENSUS_NAMESPACE; + } + + if (operation == VsrOperation.NonReplicated || operation.IsMetadata()) + { + return 0; + } + + return partitionNamespace; + } + + /// + /// Packs the namespace for a partition-plane request from its typed identifiers. Named (string) stream + /// or topic identifiers resolve to 0; the server resolves them itself. + /// + internal static ulong ForPartition(Identifier streamId, Identifier topicId, uint partitionId) + { + return FromPartition(NumericValue(streamId), NumericValue(topicId), partitionId); + } + + /// + /// The namespace for a raw binary request, peeking into the serialized body for the ops the server + /// routes by partition. Only SendBinaryRequestAsync lands here: the typed + /// command surface packs its namespace from typed identifiers instead. /// internal static ulong ForRequest(int code, ReadOnlySpan payload, VsrOperation operation) { @@ -175,6 +204,22 @@ private static ulong FromDeleteSegments(ReadOnlySpan payload) return FromPartition(streamId, topicId, ReadUInt32(payload, position)); } + /// Numeric identifier value, or null for a named identifier the server has to resolve. + private static uint? NumericValue(Identifier identifier) + { + if (identifier.Kind != IdKind.Numeric) + { + return null; + } + + if (identifier.Length != 4 || identifier.Value.Length < 4) + { + throw Malformed(); + } + + return BinaryPrimitives.ReadUInt32LittleEndian(identifier.Value); + } + private static ulong FromPartition(uint? streamId, uint? topicId, uint partitionId) { if (streamId is null || topicId is null) diff --git a/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs b/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs new file mode 100644 index 0000000000..8f0f47682b --- /dev/null +++ b/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs @@ -0,0 +1,197 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +using Apache.Iggy.Consumers; +using Apache.Iggy.Contracts; +using Apache.Iggy.Enums; +using Apache.Iggy.IggyClient; +using Apache.Iggy.Kinds; +using Apache.Iggy.Messages; +using Apache.Iggy.Vsr; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace Apache.Iggy.Tests.ConsumerTests; + +/// +/// Group membership on a transport that exposes a consensus session is keyed off the session epoch rather +/// than off connection-state edges, so these cover both arms of that branch. +/// +public sealed class ConsumerSessionEpochTests +{ + [Fact] + public async Task + given_group_consumer_when_reauthenticated_on_the_same_session_epoch_should_not_rejoin_the_group() + { + var client = new EpochClient(); + var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); + await consumer.InitAsync(TestContext.Current.CancellationToken); + Assert.Equal(1, client.JoinCount); + + await client.RaiseAsync(ConnectionState.Connected, ConnectionState.Authenticated); + + Assert.Equal(1, client.JoinCount); + await consumer.DisposeAsync(); + } + + [Fact] + public async Task given_group_consumer_when_the_session_epoch_moved_should_rejoin_the_group() + { + var client = new EpochClient(); + var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); + await consumer.InitAsync(TestContext.Current.CancellationToken); + Assert.Equal(1, client.JoinCount); + + // The session the membership was established under is gone. + client.SessionEpoch++; + await client.RaiseAsync(ConnectionState.Connected, ConnectionState.Authenticated); + + Assert.Equal(2, client.JoinCount); + await consumer.DisposeAsync(); + } + + /// + /// A disconnect surrenders the membership even though the epoch has not moved yet. Holding it would let + /// the poll loop keep issuing requests the server refuses for as long as the reconnect takes. + /// + [Fact] + public async Task given_group_consumer_when_disconnected_should_surrender_membership_and_rejoin_on_reconnect() + { + var client = new EpochClient(); + var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); + await consumer.InitAsync(TestContext.Current.CancellationToken); + Assert.Equal(1, client.JoinCount); + + await client.RaiseAsync(ConnectionState.Authenticated, ConnectionState.Disconnected); + await client.RaiseAsync(ConnectionState.Connecting, ConnectionState.Authenticated); + + Assert.Equal(2, client.JoinCount); + await consumer.DisposeAsync(); + } + + /// + /// A plain consumer holds no membership, so nothing may clear its joined flag: no code path would ever + /// set it again and every later poll would be skipped. + /// + [Fact] + public async Task given_plain_consumer_when_disconnected_should_keep_polling_after_reconnect() + { + var client = new EpochClient(); + var config = BuildGroupConfig(); + config.Consumer = Consumer.New(1); + var consumer = new IggyConsumer(client.Object, config, NullLoggerFactory.Instance); + await consumer.InitAsync(TestContext.Current.CancellationToken); + Assert.Equal(0, client.JoinCount); + + await client.RaiseAsync(ConnectionState.Authenticated, ConnectionState.Disconnected); + client.SessionEpoch++; + await client.RaiseAsync(ConnectionState.Connecting, ConnectionState.Authenticated); + + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); + await using IAsyncEnumerator messages = consumer.ReceiveAsync(cts.Token) + .GetAsyncEnumerator(TestContext.Current.CancellationToken); + + Assert.True(await messages.MoveNextAsync()); + Assert.Equal(0, client.JoinCount); + await consumer.DisposeAsync(); + } + + private static IggyConsumerConfig BuildGroupConfig() + { + return new IggyConsumerConfig + { + StreamId = Identifier.Numeric(1), + TopicId = Identifier.Numeric(1), + Consumer = Consumer.Group("group-1"), + PollingStrategy = PollingStrategy.Next(), + BatchSize = 10, + AutoCommitMode = AutoCommitMode.Disabled, + AutoCommit = false, + PollingIntervalMs = 0 + }; + } + + /// + /// A client that carries a consensus session, counts group joins, and replays connection-state events on + /// demand so a test can drive the reconnection handler without a socket. + /// + private sealed class EpochClient + { + private readonly List> _subscribers = []; + + public IIggyClient Object { get; } + + public ulong SessionEpoch { get; set; } + + public int JoinCount { get; private set; } + + public EpochClient() + { + var mock = new Mock(MockBehavior.Loose); + mock.As().SetupGet(c => c.SessionEpoch).Returns(() => SessionEpoch); + mock.Setup(c => c.ConnectAsync(It.IsAny())).Returns(Task.CompletedTask); + mock.Setup(c => c.SubscribeConnectionEvents(It.IsAny>())) + .Callback>(_subscribers.Add); + mock.Setup(c => c.UnsubscribeConnectionEvents(It.IsAny>())) + .Callback>(callback => _subscribers.Remove(callback)); + mock.Setup(c => c.GetConsumerGroupByIdAsync(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .ReturnsAsync(new ConsumerGroupResponse + { + Id = 1, + Name = "group-1", + MembersCount = 1, + PartitionsCount = 1 + }); + mock.Setup(c => c.JoinConsumerGroupAsync(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())) + .Callback(() => JoinCount++) + .Returns(Task.CompletedTask); + mock.Setup(c => c.PollMessagesAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny())) + .ReturnsAsync(() => new PolledMessages + { + PartitionId = 1, + CurrentOffset = 0, + Messages = + [ + new MessageResponse + { + Header = new MessageHeader + { + Offset = 0, + PayloadLength = 1 + }, + Payload = new byte[] { 1 }, + UserHeaders = null + } + ] + }); + + Object = mock.Object; + } + + public async Task RaiseAsync(ConnectionState previousState, ConnectionState currentState) + { + foreach (Func subscriber in _subscribers.ToArray()) + { + await subscriber(new ConnectionStateChangedEventArgs(previousState, currentState)); + } + } + } +} diff --git a/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/RentedConsumerTests.cs b/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/RentedConsumerTests.cs index 7d65beed3a..f4adb538d1 100644 --- a/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/RentedConsumerTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/RentedConsumerTests.cs @@ -22,9 +22,9 @@ using Apache.Iggy.Encryption; using Apache.Iggy.Exceptions; using Apache.Iggy.IggyClient; -using Apache.Iggy.IggyClient.Implementations; using Apache.Iggy.Kinds; using Apache.Iggy.Messages; +using Apache.Iggy.Vsr; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Moq; @@ -377,7 +377,7 @@ internal static Mock BuildClientMock(Queue re return rentals.Dequeue(); } - return new PolledMessagesRental(TcpMessageStream.EmptyMemoryOwner.Instance) + return new PolledMessagesRental(EmptyMemoryOwner.Instance) { PartitionId = 1, CurrentOffset = 0, diff --git a/foreign/csharp/Iggy_SDK_Tests/MapperTests/BinaryMapper.cs b/foreign/csharp/Iggy_SDK_Tests/MapperTests/BinaryMapper.cs index e37aa35750..e0054919a3 100644 --- a/foreign/csharp/Iggy_SDK_Tests/MapperTests/BinaryMapper.cs +++ b/foreign/csharp/Iggy_SDK_Tests/MapperTests/BinaryMapper.cs @@ -23,13 +23,13 @@ using Apache.Iggy.Enums; using Apache.Iggy.Exceptions; using Apache.Iggy.Extensions; -using Apache.Iggy.IggyClient.Implementations; using Apache.Iggy.Shared; using Apache.Iggy.Tests.Utils; using Apache.Iggy.Tests.Utils.Groups; using Apache.Iggy.Tests.Utils.Messages; using Apache.Iggy.Tests.Utils.Stats; using Apache.Iggy.Tests.Utils.Topics; +using Apache.Iggy.Vsr; using StreamFactory = Apache.Iggy.Tests.Utils.Streams.StreamFactory; namespace Apache.Iggy.Tests.MapperTests; @@ -91,7 +91,7 @@ public void MapMessages_NoHeaders_ReturnsValidMessageResponses() // Act var responses - = Mappers.BinaryMapper.MapRentedMessages(combinedPayload, TcpMessageStream.EmptyMemoryOwner.Instance); + = Mappers.BinaryMapper.MapRentedMessages(combinedPayload, EmptyMemoryOwner.Instance); // Assert Assert.NotNull(responses); @@ -366,7 +366,7 @@ public void MapRentedMessages_WithEncryptor_DecryptsPayloadsAndHeadersIntoPooled frame1.CopyTo(combined.AsSpan(16)); frame2.CopyTo(combined.AsSpan(16 + frame1.Length)); - using var rental = Mappers.BinaryMapper.MapRentedMessages(combined, TcpMessageStream.EmptyMemoryOwner.Instance, + using var rental = Mappers.BinaryMapper.MapRentedMessages(combined, EmptyMemoryOwner.Instance, encryptor); Assert.Equal(7, rental.PartitionId); @@ -407,7 +407,7 @@ public void MapRentedMessages_WithEncryptor_NegativePayloadLength_ThrowsInsteadO frame.CopyTo(combined.AsSpan(16)); Assert.Throws(() => - Mappers.BinaryMapper.MapRentedMessages(combined, TcpMessageStream.EmptyMemoryOwner.Instance, encryptor)); + Mappers.BinaryMapper.MapRentedMessages(combined, EmptyMemoryOwner.Instance, encryptor)); } [Fact] @@ -425,7 +425,7 @@ public void MapRentedMessages_WithEncryptor_TamperedCiphertext_ThrowsMessageDecr frame.CopyTo(combined.AsSpan(16)); var ex = Assert.Throws(() => - Mappers.BinaryMapper.MapRentedMessages(combined, TcpMessageStream.EmptyMemoryOwner.Instance, encryptor)); + Mappers.BinaryMapper.MapRentedMessages(combined, EmptyMemoryOwner.Instance, encryptor)); Assert.Equal(42ul, ex.Offset); Assert.Equal(7u, ex.PartitionId); diff --git a/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs b/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs index 73e9ba3b9c..7463b1df75 100644 --- a/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs @@ -19,7 +19,7 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using Apache.Iggy.IggyClient.Implementations; +using Apache.Iggy.Utils; namespace Apache.Iggy.Tests.UtilityTests; @@ -31,7 +31,7 @@ public class SlicedMemoryOwnerTests public void Dispose_ReturnsBufferToPool() { var owner = ArrayPoolHelper.Rent(BufferSize); - byte[] underlying = GetUnderlyingArray(owner); + var underlying = GetUnderlyingArray(owner); owner.Dispose(); using var second = ArrayPoolHelper.Rent(BufferSize); @@ -50,10 +50,10 @@ public void Dispose_IsIdempotent() [Fact] public void FinalizerIsDeclared() { - Type sliced = typeof(ArrayPoolHelper) + var sliced = typeof(ArrayPoolHelper) .GetNestedType("SlicedMemoryOwner", BindingFlags.NonPublic)!; - MethodInfo? finalizer = sliced.GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance); + var finalizer = sliced.GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance); Assert.NotNull(finalizer); } @@ -61,7 +61,7 @@ public void FinalizerIsDeclared() [Fact] public void ForgotDispose_FinalizerRunsAndReclaimsInstance() { - WeakReference weakRef = RentWeak(); + var weakRef = RentWeak(); GC.Collect(); GC.WaitForPendingFinalizers(); @@ -70,12 +70,15 @@ public void ForgotDispose_FinalizerRunsAndReclaimsInstance() Assert.False(weakRef.IsAlive); [MethodImpl(MethodImplOptions.NoInlining)] - static WeakReference RentWeak() => new(ArrayPoolHelper.Rent(BufferSize)); + static WeakReference RentWeak() + { + return new WeakReference(ArrayPoolHelper.Rent(BufferSize)); + } } private static byte[] GetUnderlyingArray(IMemoryOwner owner) { - if (!MemoryMarshal.TryGetArray(owner.Memory, out var segment) || segment.Array is null) + if (!MemoryMarshal.TryGetArray(owner.Memory, out ArraySegment segment) || segment.Array is null) { throw new InvalidOperationException("SlicedMemoryOwner.Memory must be array-backed."); } diff --git a/foreign/csharp/Iggy_SDK_Tests/Utils/Errors/ErrorModelFactory.cs b/foreign/csharp/Iggy_SDK_Tests/Utils/Errors/ErrorModelFactory.cs deleted file mode 100644 index 137a2bb2d2..0000000000 --- a/foreign/csharp/Iggy_SDK_Tests/Utils/Errors/ErrorModelFactory.cs +++ /dev/null @@ -1,33 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -using Apache.Iggy.Errors; - -namespace Apache.Iggy.Tests.Utils.Errors; - -public static class ErrorModelFactory -{ - public static ErrorModel CreateErrorModelBadRequest() - { - return new ErrorModel(69, "bad_request", "Bad Request"); - } - - public static ErrorModel CreateErrorModelNotFound() - { - return new ErrorModel(69, "not_found", "Not Found"); - } -} diff --git a/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs b/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs index 51e4d7f0a1..aaf438213c 100644 --- a/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs @@ -215,4 +215,24 @@ public void Reset_ClearsBindingAndCounter() Assert.Equal(1UL, session.RequestCounter); Assert.NotEqual((UInt128)1, session.ClientId); } + + [Fact] + public void Epoch_AdvancesOnEveryReArmAndNotOnBind() + { + var session = new ConsensusSession(1); + var initialEpoch = session.Epoch; + + session.Resolve(VsrOperation.Register); + session.Bind(10); + Assert.Equal(initialEpoch, session.Epoch); + + session.Reset(); + Assert.Equal(initialEpoch + 1, session.Epoch); + + // A register on a previously bound session re-arms the identity, which is a new epoch too. + session.Resolve(VsrOperation.Register); + session.Bind(11); + session.Resolve(VsrOperation.Register); + Assert.Equal(initialEpoch + 2, session.Epoch); + } } diff --git a/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrHeaderTests.cs b/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrHeaderTests.cs index f35843ff3b..3f06e70036 100644 --- a/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrHeaderTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrHeaderTests.cs @@ -33,10 +33,11 @@ private static ConsensusSession BoundSession(ulong session = 5) return consensusSession; } - private static byte[] Encode(ConsensusSession session, int code, byte[] payload, out int totalSize) + private static byte[] Encode(ConsensusSession session, int code, int bodyLength, out int totalSize, + ulong ns = 0) { var header = new byte[VsrHeader.HEADER_SIZE]; - totalSize = VsrHeader.EncodeRequestHeader(header, session, code, payload); + totalSize = VsrHeader.EncodeRequestHeader(header, session, code, ns, bodyLength); return header; } @@ -52,27 +53,20 @@ private static uint ReadUInt32(byte[] header, int offset) } /// - /// A namespace failure must leave the session exactly as it was. Consuming the request id would gap the - /// next metadata request, and dropping the binding would leave the client unable to send or re-login. + /// The caller resolves the routing namespace from its typed identifiers, but only partition-plane + /// operations carry it: a stray namespace on a metadata or non-replicated request would route it off + /// the metadata consensus group. /// [Fact] - public void Encode_NamespaceFailureConsumesNothingAndKeepsTheSession() + public void Encode_NonPartitionOpsIgnoreTheCallerNamespace() { var session = BoundSession(); - var payload = VsrTestPayloads.ConsumerOffset(VsrTestPayloads.NumericIdentifier(4), - VsrTestPayloads.NumericIdentifier(5), null); - var header = new byte[VsrHeader.HEADER_SIZE]; - - var exception = Assert.Throws(() => - VsrHeader.EncodeRequestHeader(header, session, CommandCodes.STORE_CONSUMER_OFFSET_CODE, payload)); - Assert.Equal(VsrError.INVALID_IDENTIFIER, exception.StatusCode); - Assert.True(session.IsBound); - Assert.Equal(1UL, session.RequestCounter); - - VsrHeader.EncodeRequestHeader(header, session, CommandCodes.CREATE_STREAM_CODE, [1]); + var metadataHeader = Encode(session, CommandCodes.CREATE_STREAM_CODE, 1, out _, VsrNamespace.Pack(1, 2, 3)); + var nonReplicatedHeader = Encode(session, CommandCodes.PING_CODE, 0, out _, VsrNamespace.Pack(1, 2, 3)); - Assert.Equal(1UL, ReadUInt64(header, VsrHeader.REQUEST_ID_OFFSET)); + Assert.Equal(0UL, ReadUInt64(metadataHeader, VsrHeader.REQUEST_NAMESPACE_OFFSET)); + Assert.Equal(0UL, ReadUInt64(nonReplicatedHeader, VsrHeader.REQUEST_NAMESPACE_OFFSET)); } [Fact] @@ -81,7 +75,7 @@ public void Encode_RegisterUsesZeroRequestAndSessionOnMetadataNamespace() var session = new ConsensusSession(7); var payload = LoginRegister.Serialize("admin", "secret"); - var header = Encode(session, CommandCodes.LOGIN_REGISTER_CODE, payload, out var totalSize); + var header = Encode(session, CommandCodes.LOGIN_REGISTER_CODE, payload.Length, out var totalSize); Assert.Equal(VsrHeader.HEADER_SIZE + payload.Length, totalSize); Assert.Equal((uint)totalSize, ReadUInt32(header, VsrHeader.SIZE_OFFSET)); @@ -100,7 +94,7 @@ public void Encode_WritesClientIdAsTwoLittleEndianHalvesLowFirst() { var session = new ConsensusSession(new UInt128(0xAABB_CCDD_EEFF_0011, 0x1122_3344_5566_7788)); - var header = Encode(session, CommandCodes.PING_CODE, [], out _); + var header = Encode(session, CommandCodes.PING_CODE, 0, out _); Assert.Equal(0x1122_3344_5566_7788UL, ReadUInt64(header, VsrHeader.REQUEST_CLIENT_OFFSET)); Assert.Equal(0xAABB_CCDD_EEFF_0011UL, ReadUInt64(header, VsrHeader.REQUEST_CLIENT_OFFSET + 8)); @@ -111,7 +105,7 @@ public void Encode_NonReplicatedDoesNotAdvanceCounterAndCarriesCodeInReserved() { var session = BoundSession(); - var header = Encode(session, CommandCodes.PING_CODE, [], out _); + var header = Encode(session, CommandCodes.PING_CODE, 0, out _); Assert.Equal((byte)VsrOperation.NonReplicated, header[VsrHeader.REQUEST_OPERATION_OFFSET]); Assert.Equal(1UL, ReadUInt64(header, VsrHeader.REQUEST_ID_OFFSET)); @@ -126,7 +120,7 @@ public void Encode_UnknownCodeRidesNonReplicated() { var session = BoundSession(); - var header = Encode(session, 9999, [], out _); + var header = Encode(session, 9999, 0, out _); Assert.Equal((byte)VsrOperation.NonReplicated, header[VsrHeader.REQUEST_OPERATION_OFFSET]); Assert.Equal(9999u, ReadUInt32(header, VsrHeader.REQUEST_RESERVED_OFFSET)); @@ -137,7 +131,7 @@ public void Encode_NonReplicatedWithoutSessionSendsSessionZero() { var session = new ConsensusSession(1); - var header = Encode(session, CommandCodes.PING_CODE, [], out _); + var header = Encode(session, CommandCodes.PING_CODE, 0, out _); Assert.Equal(0UL, ReadUInt64(header, VsrHeader.REQUEST_SESSION_OFFSET)); } @@ -147,7 +141,7 @@ public void Encode_MetadataAdvancesTheCounter() { var session = BoundSession(); - var header = Encode(session, CommandCodes.CREATE_STREAM_CODE, [1, 2, 3], out _); + var header = Encode(session, CommandCodes.CREATE_STREAM_CODE, 3, out _); Assert.Equal((byte)VsrOperation.CreateStream, header[VsrHeader.REQUEST_OPERATION_OFFSET]); Assert.Equal(1UL, ReadUInt64(header, VsrHeader.REQUEST_ID_OFFSET)); @@ -161,7 +155,7 @@ public void Encode_LogoutAdvancesTheCounterOnMetadataNamespace() { var session = BoundSession(); - var header = Encode(session, CommandCodes.LOGOUT_USER_CODE, [], out _); + var header = Encode(session, CommandCodes.LOGOUT_USER_CODE, 0, out _); Assert.Equal((byte)VsrOperation.Logout, header[VsrHeader.REQUEST_OPERATION_OFFSET]); Assert.Equal(1UL, ReadUInt64(header, VsrHeader.REQUEST_ID_OFFSET)); @@ -173,9 +167,8 @@ public void Encode_LogoutAdvancesTheCounterOnMetadataNamespace() public void Encode_PartitionOpDoesNotAdvanceTheCounter() { var session = BoundSession(); - var payload = VsrTestPayloads.SendMessagesToPartition(2, 3, 4); - var header = Encode(session, CommandCodes.SEND_MESSAGES_CODE, payload, out _); + var header = Encode(session, CommandCodes.SEND_MESSAGES_CODE, 16, out _, VsrNamespace.Pack(2, 3, 4)); Assert.Equal((byte)VsrOperation.SendMessages, header[VsrHeader.REQUEST_OPERATION_OFFSET]); Assert.Equal(1UL, ReadUInt64(header, VsrHeader.REQUEST_ID_OFFSET)); @@ -189,7 +182,7 @@ public void Encode_ReplicatedOpWithoutSessionIsUnauthenticated() var session = new ConsensusSession(1); var exception = Assert.Throws(() => - Encode(session, CommandCodes.CREATE_STREAM_CODE, [1], out _)); + Encode(session, CommandCodes.CREATE_STREAM_CODE, 1, out _)); Assert.Equal(VsrError.UNAUTHENTICATED, exception.StatusCode); Assert.Equal(1UL, session.RequestCounter); @@ -202,7 +195,7 @@ public void Encode_ClearsStaleBytesFromAReusedBuffer() Array.Fill(header, (byte)0xFF); var session = BoundSession(); - VsrHeader.EncodeRequestHeader(header, session, CommandCodes.CREATE_STREAM_CODE, [1]); + VsrHeader.EncodeRequestHeader(header, session, CommandCodes.CREATE_STREAM_CODE, 0, 1); Assert.Equal(0u, ReadUInt32(header, VsrHeader.REQUEST_RESERVED_OFFSET)); Assert.All(header[..VsrHeader.SIZE_OFFSET], stale => Assert.Equal(0, stale)); @@ -214,7 +207,8 @@ public void Encode_RejectsAShortBuffer() var session = BoundSession(); Assert.Throws(() => - VsrHeader.EncodeRequestHeader(new byte[VsrHeader.HEADER_SIZE - 1], session, CommandCodes.PING_CODE, [])); + VsrHeader.EncodeRequestHeader(new byte[VsrHeader.HEADER_SIZE - 1], session, CommandCodes.PING_CODE, 0, + 0)); } [Fact] diff --git a/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrNamespaceTests.cs b/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrNamespaceTests.cs index 3ce7095600..5527e6262d 100644 --- a/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrNamespaceTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/VsrTests/VsrNamespaceTests.cs @@ -39,6 +39,57 @@ public void MetadataSentinel_SitsAboveThePackedRange() Assert.True(VsrNamespace.METADATA_CONSENSUS_NAMESPACE > packedMax); } + [Fact] + public void ForPartition_PacksNumericIdentifiers() + { + Assert.Equal(VsrNamespace.Pack(2, 3, 4), + VsrNamespace.ForPartition(Identifier.Numeric(2), Identifier.Numeric(3), 4)); + } + + [Fact] + public void ForPartition_NamedIdentifierResolvesToZero() + { + Assert.Equal(0UL, VsrNamespace.ForPartition(Identifier.String("orders"), Identifier.Numeric(3), 4)); + Assert.Equal(0UL, VsrNamespace.ForPartition(Identifier.Numeric(2), Identifier.String("events"), 4)); + } + + [Theory] + [InlineData((uint)VsrNamespace.MAX_STREAMS, 3u, 4u)] + [InlineData(2u, (uint)VsrNamespace.MAX_TOPICS, 4u)] + [InlineData(2u, 3u, (uint)VsrNamespace.MAX_PARTITIONS)] + public void ForPartition_RejectsIdentifiersOutsideThePackableRange(uint streamId, uint topicId, uint partitionId) + { + var exception = Assert.Throws(() => + VsrNamespace.ForPartition(Identifier.Numeric(streamId), Identifier.Numeric(topicId), partitionId)); + + Assert.Equal(VsrError.INVALID_IDENTIFIER, exception.StatusCode); + } + + [Theory] + [InlineData((byte)VsrOperation.Register)] + [InlineData((byte)VsrOperation.Logout)] + public void ForOperation_ControlPlaneOpsTargetTheMetadataReplica(byte operation) + { + Assert.Equal(VsrNamespace.METADATA_CONSENSUS_NAMESPACE, + VsrNamespace.ForOperation((VsrOperation)operation, VsrNamespace.Pack(1, 2, 3))); + } + + [Fact] + public void ForOperation_MetadataAndNonReplicatedOpsIgnoreThePartitionNamespace() + { + Assert.Equal(0UL, VsrNamespace.ForOperation(VsrOperation.CreateStream, VsrNamespace.Pack(1, 2, 3))); + Assert.Equal(0UL, VsrNamespace.ForOperation(VsrOperation.NonReplicated, VsrNamespace.Pack(1, 2, 3))); + } + + [Fact] + public void ForOperation_PartitionOpsCarryTheCallerNamespace() + { + Assert.Equal(VsrNamespace.Pack(1, 2, 3), + VsrNamespace.ForOperation(VsrOperation.SendMessages, VsrNamespace.Pack(1, 2, 3))); + Assert.Equal(VsrNamespace.Pack(1, 2, 3), + VsrNamespace.ForOperation(VsrOperation.StoreConsumerOffset, VsrNamespace.Pack(1, 2, 3))); + } + [Theory] [InlineData((byte)VsrOperation.Register)] [InlineData((byte)VsrOperation.Logout)] @@ -160,7 +211,7 @@ public void ForRequest_ConsumerOffsetWithoutAPartitionIsRejected() [Fact] public void ForRequest_ConsumerOffsetWithoutAPartitionIsRejectedEvenWhenTheBodyEndsThere() { - byte[] full = VsrTestPayloads.ConsumerOffset(VsrTestPayloads.NumericIdentifier(4), + var full = VsrTestPayloads.ConsumerOffset(VsrTestPayloads.NumericIdentifier(4), VsrTestPayloads.NumericIdentifier(5), null); // Drop the four padding bytes that follow the absent-partition flag. From b1340e1fb51d914835ae986e8c34e01e12b63894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Zborek?= Date: Tue, 11 Aug 2026 15:07:29 +0200 Subject: [PATCH 2/5] refactor(csharp): remove unused namespace parameters from TCP message handling --- .../Implementations/TcpMessageStream.Vsr.cs | 12 +-- .../Implementations/TcpMessageStream.cs | 80 +++---------------- foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs | 4 +- 3 files changed, 21 insertions(+), 75 deletions(-) diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs index 7582bfaf7c..0b4e814f02 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs @@ -130,7 +130,7 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider try { using IMemoryOwner responseBuffer = - await SendWithResponseAsync(code, 0, message, token, false); + await SendWithResponseAsync(code, message, token, false); response = LoginRegister.Deserialize(responseBuffer.Memory.Span); _consensusSession.Bind(response.Session); @@ -448,7 +448,7 @@ private async Task RedirectAsync(CancellationToken token) /// private async Task ReadClusterMetadataNoRedirectAsync(CancellationToken token) { - using IMemoryOwner responseBuffer = await SendRawAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, 0, + using IMemoryOwner responseBuffer = await SendRawAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, ReadOnlyMemory.Empty, token, false); if (responseBuffer.Memory.Length == 0) @@ -468,7 +468,7 @@ private async Task RedirectAsync(CancellationToken token) /// register replay on this connection for the whole budget instead: the connect flow owns leader /// redirection for the handshake, and reconnecting from underneath it would recurse. /// - private async Task> SendRawAsync(int code, ulong ns, ReadOnlyMemory body, + private async Task> SendRawAsync(int code, ReadOnlyMemory body, CancellationToken token, bool allowRedirect = true) { ObjectDisposedException.ThrowIf(_disposed, this); @@ -494,7 +494,7 @@ private async Task> SendRawAsync(int code, ulong ns, ReadOnly ? overallDeadline : Math.Min(overallDeadline, Environment.TickCount64 + VsrTransientFailoverCheckMs); - var attempt = await SendVsrAttemptAsync(code, ns, body, header, transientDeadline, overallDeadline, + var attempt = await SendVsrAttemptAsync(code, body, header, transientDeadline, overallDeadline, token); requestEncoded |= attempt.Encoded; lastConnection = attempt.Connection; @@ -575,7 +575,7 @@ private static bool IsDefinitiveVerdict(Exception error) /// cannot be split across two sockets, and so a teardown after the lock is gone can tell this /// connection from a replacement a reconnect installed since. /// - private async ValueTask SendVsrAttemptAsync(int code, ulong ns, ReadOnlyMemory body, + private async ValueTask SendVsrAttemptAsync(int code, ReadOnlyMemory body, Memory header, long transientDeadline, long readDeadline, CancellationToken token) { await _sendingSemaphore.WaitAsync(token); @@ -587,7 +587,7 @@ private async ValueTask SendVsrAttemptAsync(int code, ulong ns, Read return VsrAttempt.Failed(false, new NotConnectedException(), false, null); } - return await connection.SendAttemptAsync(code, ns, body, header, transientDeadline, readDeadline, + return await connection.SendAttemptAsync(code, body, header, transientDeadline, readDeadline, token); } finally diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs index f0b2fc5cf7..5777e10872 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs @@ -330,9 +330,8 @@ public Task PollMessagesRentedAsync(Identifier streamId, I public async Task StoreOffsetAsync(Consumer consumer, Identifier streamId, Identifier topicId, ulong offset, uint? partitionId, CancellationToken token = default) { - var ns = ConsumerOffsetNamespace(streamId, topicId, partitionId); var message = TcpContracts.UpdateOffset(streamId, topicId, consumer, offset, partitionId); - await SendAckAsync(CommandCodes.STORE_CONSUMER_OFFSET_CODE, ns, message, token); + await SendAckAsync(CommandCodes.STORE_CONSUMER_OFFSET_CODE, message, token); } /// @@ -355,9 +354,8 @@ public async Task StoreOffsetAsync(Consumer consumer, Identifier streamId, Ident public async Task DeleteOffsetAsync(Consumer consumer, Identifier streamId, Identifier topicId, uint? partitionId, CancellationToken token = default) { - var ns = ConsumerOffsetNamespace(streamId, topicId, partitionId); var message = TcpContracts.DeleteOffset(streamId, topicId, consumer, partitionId); - await SendAckAsync(CommandCodes.DELETE_CONSUMER_OFFSET_CODE, ns, message, token); + await SendAckAsync(CommandCodes.DELETE_CONSUMER_OFFSET_CODE, message, token); } /// @@ -461,9 +459,8 @@ public async Task CreatePartitionsAsync(Identifier streamId, Identifier topicId, public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, uint partitionId, uint segmentsCount, CancellationToken token = default) { - var ns = VsrNamespace.ForPartition(streamId, topicId, partitionId); var message = TcpContracts.DeleteSegments(streamId, topicId, partitionId, segmentsCount); - await SendAckAsync(CommandCodes.DELETE_SEGMENTS_CODE, ns, message, token); + await SendAckAsync(CommandCodes.DELETE_SEGMENTS_CODE, message, token); } /// @@ -538,8 +535,7 @@ public async Task SendBinaryRequestAsync(uint code, byte[] payload, Canc $"Command {code} cannot be sent as a raw binary request."); } - var ns = VsrNamespace.ForRequest((int)code, payload, VsrOperations.ForCode((int)code)); - using IMemoryOwner result = await SendWithResponseAsync((int)code, ns, payload, token); + using IMemoryOwner result = await SendWithResponseAsync((int)code, payload, token); return result.Memory.Length <= 1 ? [] : result.Memory.Span.ToArray(); } @@ -833,7 +829,6 @@ private Task SendMessagesCoreAsync(Identifier streamId, Id var maxMessageBufferSize = TcpMessageStreamHelpers.CalculateMessageBytesCount(messages, encryptor) + metadataLength; - var ns = SendMessagesNamespace(streamId, topicId, partitioning); IMemoryOwner payloadBuffer = MemoryPool.Shared.Rent(maxMessageBufferSize); int bodySize; try @@ -847,16 +842,16 @@ private Task SendMessagesCoreAsync(Identifier streamId, Id throw; } - return SendConfirmedAndDisposeAsync(ns, payloadBuffer, bodySize, token); + return SendConfirmedAndDisposeAsync(payloadBuffer, bodySize, token); } - private async Task SendConfirmedAndDisposeAsync(ulong ns, IMemoryOwner payloadBuffer, + private async Task SendConfirmedAndDisposeAsync(IMemoryOwner payloadBuffer, int bodySize, CancellationToken token) { try { using IMemoryOwner responseBuffer = await SendWithResponseAsync(CommandCodes.SEND_MESSAGES_CODE, - ns, payloadBuffer.Memory[..bodySize], token); + payloadBuffer.Memory[..bodySize], token); return BinaryMapper.MapSendMessages(responseBuffer.Memory.Span); } finally @@ -875,44 +870,6 @@ private static ReadOnlySpan AsSpan(IList messages) }; } - /// - /// The routing namespace for a send: callers resolve balanced and message-key partitioning to an - /// explicit partition before framing, so anything else reaching this point is a bug surfaced as the - /// same error the server-side router would raise. - /// - private static ulong SendMessagesNamespace(Identifier streamId, Identifier topicId, Partitioning partitioning) - { - if (partitioning.Kind != Enums.Partitioning.PartitionId) - { - throw VsrError.Exception(VsrError.FEATURE_UNAVAILABLE, - "Under VSR the partition must be resolved by the client; balanced and message-key partitioning cannot be sent on the wire."); - } - - if (partitioning.Value.Length < 4) - { - throw VsrError.Exception(VsrError.INVALID_COMMAND, - $"Partition id is {partitioning.Value.Length} bytes, expected 4."); - } - - return VsrNamespace.ForPartition(streamId, topicId, - BinaryPrimitives.ReadUInt32LittleEndian(partitioning.Value)); - } - - /// - /// The routing namespace for a consumer-offset write. The broker routes explicit partitions only, so a - /// missing partition id fails client-side before a request id is consumed. - /// - private static ulong ConsumerOffsetNamespace(Identifier streamId, Identifier topicId, uint? partitionId) - { - if (partitionId is null) - { - throw VsrError.Exception(VsrError.INVALID_IDENTIFIER, - "Under VSR a consumer-offset request must carry an explicit partition id."); - } - - return VsrNamespace.ForPartition(streamId, topicId, partitionId.Value); - } - private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken token) { var retryCount = 0; @@ -1075,23 +1032,12 @@ private async Task SendAckAsync(int code, ReadOnlyMemory body, Cancellatio using IMemoryOwner _ = await SendWithResponseAsync(code, body, token); } - private async Task SendAckAsync(int code, ulong ns, ReadOnlyMemory body, CancellationToken token) - { - using IMemoryOwner _ = await SendWithResponseAsync(code, ns, body, token); - } - - private Task> SendWithResponseAsync(int code, ReadOnlyMemory body, - CancellationToken token) - { - return SendWithResponseAsync(code, 0, body, token); - } - - private async Task> SendWithResponseAsync(int code, ulong ns, ReadOnlyMemory body, + private async Task> SendWithResponseAsync(int code, ReadOnlyMemory body, CancellationToken token, bool autoLoginOnReconnect = true) { try { - return await SendRawAsync(code, ns, body, token); + return await SendRawAsync(code, body, token); } catch (Exception e) when (VsrConnection.IsConnectionException(e) && !IsConnecting && !_disposed) { @@ -1103,11 +1049,11 @@ private async Task> SendWithResponseAsync(int code, ulong ns, throw; } - return await HandleReconnectionAsync(code, ns, body, autoLoginOnReconnect, token); + return await HandleReconnectionAsync(code, body, autoLoginOnReconnect, token); } } - private async Task> HandleReconnectionAsync(int code, ulong ns, ReadOnlyMemory body, + private async Task> HandleReconnectionAsync(int code, ReadOnlyMemory body, bool autoLogin, CancellationToken token) { var currentTime = DateTimeOffset.UtcNow; @@ -1119,7 +1065,7 @@ private async Task> HandleReconnectionAsync(int code, ulong n && _lastConnectionTime > currentTime) { _logger.LogInformation("Connection already established, sending payload"); - return await SendRawAsync(code, ns, body, token); + return await SendRawAsync(code, body, token); } SetConnectionStateAsync(ConnectionState.Disconnected); @@ -1130,7 +1076,7 @@ private async Task> HandleReconnectionAsync(int code, ulong n await Task.Delay(_configuration.ReconnectionSettings.WaitAfterReconnect, token); - return await SendRawAsync(code, ns, body, token); + return await SendRawAsync(code, body, token); } finally { diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs index 3029747101..8c8fae82bc 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs @@ -85,7 +85,7 @@ IOException or /// and replay it - same session, same request id - while the server answers transiently. The caller /// must hold the sending lock. /// - internal async ValueTask SendAttemptAsync(int code, ulong ns, ReadOnlyMemory body, + internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory body, Memory header, long transientDeadline, long readDeadline, CancellationToken token) { var encoded = false; @@ -93,7 +93,7 @@ internal async ValueTask SendAttemptAsync(int code, ulong ns, ReadOn try { - VsrHeader.EncodeRequestHeader(header.Span, Session, code, ns, body.Length); + VsrHeader.EncodeRequestHeader(header.Span, Session, code, body.Span); encoded = true; From a7c320a81dce4e19d475b4272d0dc309d16e26dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Zborek?= Date: Tue, 11 Aug 2026 21:35:29 +0200 Subject: [PATCH 3/5] refactor(csharp): enhance session epoch handling and clean up connection management --- .../csharp/Iggy_SDK/Consumers/IggyConsumer.cs | 46 ++++++---- .../Implementations/TcpMessageStream.Vsr.cs | 43 +++++---- .../Implementations/TcpMessageStream.cs | 3 +- foreign/csharp/Iggy_SDK/Iggy_SDK.csproj | 2 - .../Iggy_SDK/Vsr/ISessionEpochProvider.cs | 8 +- foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs | 25 +++--- .../ConsumerSessionEpochTests.cs | 13 ++- .../UtilityTests/SlicedMemoryOwnerTests.cs | 88 ------------------- 8 files changed, 82 insertions(+), 146 deletions(-) delete mode 100644 foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs diff --git a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs index f40cf62279..14b8ffcc76 100644 --- a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs +++ b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs @@ -277,7 +277,7 @@ private async Task InitializeConsumerGroupAsync(CancellationToken ct = default) if (_config.Consumer.Type == ConsumerType.Consumer) { - _joinedSessionEpoch = sessionEpoch; + Interlocked.Exchange(ref _joinedSessionEpoch, sessionEpoch); _joinedConsumerGroup = true; return; } @@ -319,7 +319,7 @@ private async Task InitializeConsumerGroupAsync(CancellationToken ct = default) await _client.JoinConsumerGroupAsync(_config.StreamId, _config.TopicId, Identifier.String(_consumerGroupName), ct); - _joinedSessionEpoch = sessionEpoch; + Interlocked.Exchange(ref _joinedSessionEpoch, sessionEpoch); _joinedConsumerGroup = true; LogConsumerGroupJoined(_consumerGroupName); } @@ -378,7 +378,7 @@ private void ThrowIfAutoCommitWithEncryptor() /// private async Task PollMessagesAsync(CancellationToken ct) { - if (!_joinedConsumerGroup) + if (!_joinedConsumerGroup || !IsGroupMembershipEpochCurrent()) { LogConsumerGroupNotJoinedYetSkippingPolling(); return; @@ -466,6 +466,24 @@ private async Task PollMessagesAsync(CancellationToken ct) } } + /// + /// Whether the session epoch the group membership was stamped under is still the transport's current + /// one. Gating the poll here instead of clearing the joined flag on a Disconnected event survives a + /// late event landing after a rejoin already re-stamped the epoch: state events are published outside + /// the state lock, so their order is not guaranteed. Runs outside , + /// hence the interlocked read. + /// + private bool IsGroupMembershipEpochCurrent() + { + if (_config.Consumer.Type != ConsumerType.ConsumerGroup + || _client is not ISessionEpochProvider epochProvider) + { + return true; + } + + return epochProvider.SessionEpoch == Interlocked.Read(ref _joinedSessionEpoch); + } + /// /// Implements polling interval throttling to avoid excessive server requests. /// Uses monotonic time tracking to ensure proper intervals even with clock adjustments. @@ -515,32 +533,24 @@ private async Task WaitBeforePollingAsync(CancellationToken ct) private async Task OnClientConnectionStateChangedAsync(ConnectionStateChangedEventArgs e) { LogConnectionStateChanged(e.PreviousState, e.CurrentState); + + if (_config.Consumer.Type == ConsumerType.Consumer) + { + return; + } await _connectionStateSemaphore.WaitAsync(); try { if (_client is ISessionEpochProvider epochProvider) { - // A plain consumer holds no membership, so its flag must never be cleared: nothing would ever - // set it again and every later poll would be skipped. - if (_config.Consumer.Type == ConsumerType.Consumer) - { - return; - } - if (e.CurrentState != ConnectionState.Authenticated) { - // Polling under a dropped session would be refused by the server for as long as the - // reconnect takes to re-authenticate, so the membership is surrendered up front. - if (e.CurrentState == ConnectionState.Disconnected) - { - _joinedConsumerGroup = false; - } - return; } - if (_joinedConsumerGroup && epochProvider.SessionEpoch == _joinedSessionEpoch) + if (_joinedConsumerGroup + && epochProvider.SessionEpoch == Interlocked.Read(ref _joinedSessionEpoch)) { return; } diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs index 0b4e814f02..4a4f355b18 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs @@ -130,7 +130,7 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider try { using IMemoryOwner responseBuffer = - await SendWithResponseAsync(code, message, token, false); + await SendWithResponseAsync(code, message, token, autoLoginOnReconnect: false); response = LoginRegister.Deserialize(responseBuffer.Memory.Span); _consensusSession.Bind(response.Session); @@ -161,16 +161,21 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider { _logger.LogWarning("Maximum leader redirections reached while registering, staying on {Address}", _currentAddress); - - return authResponse; + } + else if (await RedirectAsync(token)) + { + await ConnectAsync(false, token); + continue; } - if (!await RedirectAsync(token)) + // The redirect probe can tear the connection down without throwing, and success on a client that is + // no longer bound would leave the caller unauthenticated with nothing left to re-authenticate it. + if (_state != ConnectionState.Authenticated) { - return authResponse; + throw new NotConnectedException(); } - await ConnectAsync(false, token); + return authResponse; } } @@ -433,7 +438,7 @@ private async Task RedirectAsync(CancellationToken token) { return null; } - catch (Exception e) when (e is not OperationCanceledException) + catch (Exception e) when (e is not OperationCanceledException && !VsrConnection.IsConnectionException(e)) { _logger.LogWarning(e, "Failed to read the cluster metadata, continuing on {Address}", _currentAddress); @@ -449,7 +454,7 @@ private async Task RedirectAsync(CancellationToken token) private async Task ReadClusterMetadataNoRedirectAsync(CancellationToken token) { using IMemoryOwner responseBuffer = await SendRawAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, - ReadOnlyMemory.Empty, token, false); + ReadOnlyMemory.Empty, token, allowRedirect: false); if (responseBuffer.Memory.Length == 0) { @@ -480,10 +485,9 @@ private async Task> SendRawAsync(int code, ReadOnlyMemory.Shared.Rent(VsrHeader.HEADER_SIZE); - Memory header = headerBuffer.AsMemory(0, VsrHeader.HEADER_SIZE); var requestEncoded = false; var redirects = 0; + var redirectBudgetLogged = false; VsrConnection? lastConnection = null; try @@ -494,7 +498,7 @@ private async Task> SendRawAsync(int code, ReadOnlyMemory> SendRawAsync(int code, ReadOnlyMemory= VsrMaxLeaderRedirects && !redirectBudgetLogged) + { + redirectBudgetLogged = true; + _logger.LogWarning("Maximum leader redirections reached, continuing on {Address}", + _currentAddress); + } continue; } @@ -550,10 +560,6 @@ private async Task> SendRawAsync(int code, ReadOnlyMemory.Shared.Return(headerBuffer); - } } /// @@ -576,7 +582,7 @@ private static bool IsDefinitiveVerdict(Exception error) /// connection from a replacement a reconnect installed since. /// private async ValueTask SendVsrAttemptAsync(int code, ReadOnlyMemory body, - Memory header, long transientDeadline, long readDeadline, CancellationToken token) + long transientDeadline, long readDeadline, CancellationToken token) { await _sendingSemaphore.WaitAsync(token); try @@ -587,7 +593,7 @@ private async ValueTask SendVsrAttemptAsync(int code, ReadOnlyMemory return VsrAttempt.Failed(false, new NotConnectedException(), false, null); } - return await connection.SendAttemptAsync(code, body, header, transientDeadline, readDeadline, + return await connection.SendAttemptAsync(code, body, transientDeadline, readDeadline, token); } finally @@ -666,8 +672,9 @@ private void DropVsrConnectionLocked(VsrConnection? connection) } ResetConsensusSession(); - connection.Close(); + _connection = null; SetConnectionStateAsync(ConnectionState.Disconnected); + connection.Dispose(); } /// Drops the connection on behalf of a caller that no longer holds the sending lock. diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs index 5777e10872..2930e0afc6 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs @@ -85,8 +85,8 @@ internal TcpMessageStream(IggyClientConfigurator configuration, ILoggerFactory l public void Dispose() { _disposed = true; - _connection?.Close(); _connection?.Dispose(); + _connection = null; SetConnectionStateAsync(ConnectionState.Disconnected); _sendingSemaphore.Dispose(); @@ -883,6 +883,7 @@ private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken try { _connection?.Dispose(); + _connection = null; ResetConsensusSession(); } diff --git a/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj b/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj index 84fd522853..a05dac8165 100644 --- a/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj +++ b/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj @@ -75,8 +75,6 @@ - - diff --git a/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs b/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs index f81d2ebf41..0bf2cf943e 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs @@ -19,10 +19,12 @@ namespace Apache.Iggy.Vsr; /// /// Exposes the consensus session generation of a transport, so session-scoped state - a consumer-group -/// membership - can detect the session it was established under is gone. Transports without a consensus -/// session (HTTP) do not implement it and their consumers fall back to connection-state edges. +/// membership - can detect the session it was established under is gone. A client that does not implement +/// it gets edge-based group rejoin from the connection-state events it publishes; a transport that +/// publishes no such events (the built-in HTTP client) gets neither and keeps its membership as-is. /// -internal interface ISessionEpochProvider +public interface ISessionEpochProvider { + /// Generation of the transport's consensus session, bumped on every session re-arm. ulong SessionEpoch { get; } } diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs index 8c8fae82bc..19e27df9d2 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs @@ -44,17 +44,19 @@ internal sealed class VsrConnection : IDisposable private readonly Action _onDropped; private readonly byte[] _replyHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; + private readonly byte[] _requestHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; private readonly int _requestTimeoutMs; - private readonly Stream _stream; /// The session identity requests on this connection encode from. - internal ConsensusSession Session { get; } + private readonly ConsensusSession _session; + + private readonly Stream _stream; internal VsrConnection(Stream stream, ConsensusSession session, long maxResponseFrameSize, int requestTimeoutMs, Action onDropped, ILogger logger) { _stream = stream; - Session = session; + _session = session; _maxResponseFrameSize = maxResponseFrameSize; _requestTimeoutMs = requestTimeoutMs; _onDropped = onDropped; @@ -66,11 +68,6 @@ public void Dispose() _stream.Dispose(); } - internal void Close() - { - _stream.Close(); - } - internal static bool IsConnectionException(Exception ex) { return ex is IggyZeroBytesException or @@ -81,19 +78,19 @@ IOException or } /// - /// One attempt on this connection: encode the header into , write the frame, - /// and replay it - same session, same request id - while the server answers transiently. The caller - /// must hold the sending lock. + /// One attempt on this connection: encode the header, write the frame, and replay it - same session, + /// same request id - while the server answers transiently. The caller must hold the sending lock, + /// which also guards the reuse of the per-connection header buffer. /// internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory body, - Memory header, long transientDeadline, long readDeadline, CancellationToken token) + long transientDeadline, long readDeadline, CancellationToken token) { var encoded = false; var requestStarted = false; try { - VsrHeader.EncodeRequestHeader(header.Span, Session, code, body.Span); + VsrHeader.EncodeRequestHeader(_requestHeaderBuffer, _session, code, body.Span); encoded = true; @@ -111,7 +108,7 @@ internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory - /// A disconnect surrenders the membership even though the epoch has not moved yet. Holding it would let - /// the poll loop keep issuing requests the server refuses for as long as the reconnect takes. + /// A disconnect re-arms the transport's session, so the epoch moves and the reconnect rejoins. The + /// membership is not cleared on the Disconnected event itself: the poll gate compares epochs instead, + /// which survives state events arriving out of order. /// [Fact] public async Task given_group_consumer_when_disconnected_should_surrender_membership_and_rejoin_on_reconnect() @@ -77,6 +84,8 @@ public async Task given_group_consumer_when_disconnected_should_surrender_member Assert.Equal(1, client.JoinCount); await client.RaiseAsync(ConnectionState.Authenticated, ConnectionState.Disconnected); + // The transport re-arms the consensus session when the connection drops. + client.SessionEpoch++; await client.RaiseAsync(ConnectionState.Connecting, ConnectionState.Authenticated); Assert.Equal(2, client.JoinCount); diff --git a/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs b/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs deleted file mode 100644 index 7463b1df75..0000000000 --- a/foreign/csharp/Iggy_SDK_Tests/UtilityTests/SlicedMemoryOwnerTests.cs +++ /dev/null @@ -1,88 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -using System.Buffers; -using System.Reflection; -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; -using Apache.Iggy.Utils; - -namespace Apache.Iggy.Tests.UtilityTests; - -public class SlicedMemoryOwnerTests -{ - private const int BufferSize = 4096; - - [Fact] - public void Dispose_ReturnsBufferToPool() - { - var owner = ArrayPoolHelper.Rent(BufferSize); - var underlying = GetUnderlyingArray(owner); - owner.Dispose(); - - using var second = ArrayPoolHelper.Rent(BufferSize); - Assert.Same(underlying, GetUnderlyingArray(second)); - } - - [Fact] - public void Dispose_IsIdempotent() - { - var owner = ArrayPoolHelper.Rent(BufferSize); - owner.Dispose(); - owner.Dispose(); - owner.Dispose(); - } - - [Fact] - public void FinalizerIsDeclared() - { - var sliced = typeof(ArrayPoolHelper) - .GetNestedType("SlicedMemoryOwner", BindingFlags.NonPublic)!; - - var finalizer = sliced.GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance); - - Assert.NotNull(finalizer); - } - - [Fact] - public void ForgotDispose_FinalizerRunsAndReclaimsInstance() - { - var weakRef = RentWeak(); - - GC.Collect(); - GC.WaitForPendingFinalizers(); - GC.Collect(); - - Assert.False(weakRef.IsAlive); - - [MethodImpl(MethodImplOptions.NoInlining)] - static WeakReference RentWeak() - { - return new WeakReference(ArrayPoolHelper.Rent(BufferSize)); - } - } - - private static byte[] GetUnderlyingArray(IMemoryOwner owner) - { - if (!MemoryMarshal.TryGetArray(owner.Memory, out ArraySegment segment) || segment.Array is null) - { - throw new InvalidOperationException("SlicedMemoryOwner.Memory must be array-backed."); - } - - return segment.Array; - } -} From a68c9b0be6e7bbb1c0219fd3deddc70238ce8644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Zborek?= Date: Wed, 12 Aug 2026 22:48:32 +0200 Subject: [PATCH 4/5] refactor(csharp): improve TCP message handling and coalescing logic --- .../csharp/Iggy_SDK/Consumers/IggyConsumer.cs | 2 +- .../Implementations/TcpMessageStream.Vsr.cs | 15 ++++---- foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs | 35 ++++++++++++++----- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs index 14b8ffcc76..cba4c58c43 100644 --- a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs +++ b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs @@ -533,7 +533,7 @@ private async Task WaitBeforePollingAsync(CancellationToken ct) private async Task OnClientConnectionStateChangedAsync(ConnectionStateChangedEventArgs e) { LogConnectionStateChanged(e.PreviousState, e.CurrentState); - + if (_config.Consumer.Type == ConsumerType.Consumer) { return; diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs index f7795addd6..a753dd17f8 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs @@ -370,11 +370,12 @@ private async Task RedirectAsync(CancellationToken token) } var leaderAddress = ServerAddress.HostPort(currentLeaderNode.Ip, currentLeaderNode.Endpoints.Tcp); - // Compare against the endpoint the socket resolved, not the configured string: a client - // configured with a hostname is otherwise never "on" the leader the roster names by IP, - // and every login would reconnect it to the node it is already talking to. - var connectedAddress = _currentRemoteAddress.Length > 0 ? _currentRemoteAddress : _currentAddress; - if (ServerAddress.IsSame(leaderAddress, connectedAddress)) + // The roster may name the leader by either side of this connection: the address the client dialed + // (a hostname, an advertised_address) or the endpoint the socket resolved to. IsSame does no DNS + // resolution, so a match on either means the client is already on the leader; checking only one + // side redirect-loops the other kind of roster. + if (ServerAddress.IsSame(leaderAddress, _currentAddress) + || (_currentRemoteAddress.Length > 0 && ServerAddress.IsSame(leaderAddress, _currentRemoteAddress))) { return false; } @@ -469,8 +470,8 @@ private async Task RedirectAsync(CancellationToken token) } /// - /// Sends a consensus-framed request: the body is written right after the 256-byte consensus header - - /// two writes, no concatenation. + /// Sends a consensus-framed request: small frames go out as a single coalesced write, larger bodies + /// as a second write straight from the caller's buffer. /// /// /// One deadline bounds the whole request across transient replays AND leader failovers. Login and diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs index 19e27df9d2..4410bc804c 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs @@ -43,8 +43,16 @@ internal sealed class VsrConnection : IDisposable /// private readonly Action _onDropped; + /// + /// Frames up to this size go out as one write: header and body coalesced in + /// . With Nagle disabled every write is its own segment (and its own + /// TLS record under a wrapping stream), so the second write a small request would pay is measurable + /// even against the round trip the lockstep protocol already costs. + /// + private const int CoalescedFrameLimit = 4096; + private readonly byte[] _replyHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; - private readonly byte[] _requestHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; + private readonly byte[] _requestFrameBuffer = new byte[CoalescedFrameLimit]; private readonly int _requestTimeoutMs; /// The session identity requests on this connection encode from. @@ -80,7 +88,7 @@ IOException or /// /// One attempt on this connection: encode the header, write the frame, and replay it - same session, /// same request id - while the server answers transiently. The caller must hold the sending lock, - /// which also guards the reuse of the per-connection header buffer. + /// which also guards the reuse of the per-connection frame buffer. /// internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory body, long transientDeadline, long readDeadline, CancellationToken token) @@ -90,7 +98,14 @@ internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory SendAttemptAsync(int code, ReadOnlyMemory Date: Wed, 12 Aug 2026 23:09:05 +0200 Subject: [PATCH 5/5] refactor(csharp): replace asynchronous connection state updates with synchronous calls --- .../Iggy_SDK/Consumers/IggyConsumer.Rented.cs | 3 +- .../csharp/Iggy_SDK/Consumers/IggyConsumer.cs | 79 +++++++++---- .../Implementations/TcpMessageStream.Vsr.cs | 54 +++++---- .../Implementations/TcpMessageStream.cs | 106 ++++++++++-------- foreign/csharp/Iggy_SDK/Iggy_SDK.csproj | 8 +- .../csharp/Iggy_SDK/Vsr/ConsensusSession.cs | 13 ++- ...vider.cs => ISessionGenerationProvider.cs} | 4 +- foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs | 33 +++--- ...s.cs => ConsumerSessionGenerationTests.cs} | 83 ++++++++++---- .../VsrTests/ConsensusSessionTests.cs | 12 +- 10 files changed, 254 insertions(+), 141 deletions(-) rename foreign/csharp/Iggy_SDK/Vsr/{ISessionEpochProvider.cs => ISessionGenerationProvider.cs} (94%) rename foreign/csharp/Iggy_SDK_Tests/ConsumerTests/{ConsumerSessionEpochTests.cs => ConsumerSessionGenerationTests.cs} (72%) diff --git a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.Rented.cs b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.Rented.cs index 9b4bbfaeb8..a545df5fbb 100644 --- a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.Rented.cs +++ b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.Rented.cs @@ -102,9 +102,10 @@ await _rentedChannel.Writer.WriteAsync(new ReceivedRentedMessage /// protected async Task PollRentedMessagesAsync(CancellationToken ct) { - if (!_joinedConsumerGroup) + if (!_joinedConsumerGroup || !IsGroupMembershipCurrent()) { LogConsumerGroupNotJoinedYetSkippingPolling(); + await TryRecoverGroupMembershipAsync(ct); return; } diff --git a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs index cba4c58c43..91c48a1d57 100644 --- a/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs +++ b/foreign/csharp/Iggy_SDK/Consumers/IggyConsumer.cs @@ -34,6 +34,13 @@ namespace Apache.Iggy.Consumers; /// public partial class IggyConsumer : IAsyncDisposable { + /// + /// Backoff between poll iterations that found the group membership missing. It bounds the retry rate of + /// the poll-side rejoin below and keeps the receive loop from spinning while the membership is + /// unrecoverable (client disconnected, group deleted). + /// + private const int GroupRejoinRetryDelayMs = 1_000; + private readonly Channel _channel; private readonly IIggyClient _client; private readonly IggyConsumerConfig _config; @@ -49,10 +56,10 @@ public partial class IggyConsumer : IAsyncDisposable private volatile bool _joinedConsumerGroup; /// - /// Consensus session epoch the group membership was established under. A later epoch means the server - /// session that held the membership is gone and the group must be rejoined. + /// Consensus session generation the group membership was established under. A later generation means the + /// server session that held the membership is gone and the group must be rejoined. /// - private ulong _joinedSessionEpoch; + private ulong _joinedSessionGeneration; private long _lastPolledAtMs; @@ -271,13 +278,13 @@ private async Task InitializeConsumerGroupAsync(CancellationToken ct = default) return; } - // Captured before the join: a session reset racing the join lands the membership on a later epoch, and - // the mismatch triggers one redundant (idempotent) rejoin instead of a missed one. - var sessionEpoch = (_client as ISessionEpochProvider)?.SessionEpoch ?? 0; + // Captured before the join: a session reset racing the join lands the membership on a later generation, + // and the mismatch triggers one redundant (idempotent) rejoin instead of a missed one. + var sessionGeneration = (_client as ISessionGenerationProvider)?.SessionGeneration ?? 0; if (_config.Consumer.Type == ConsumerType.Consumer) { - Interlocked.Exchange(ref _joinedSessionEpoch, sessionEpoch); + Interlocked.Exchange(ref _joinedSessionGeneration, sessionGeneration); _joinedConsumerGroup = true; return; } @@ -319,7 +326,7 @@ private async Task InitializeConsumerGroupAsync(CancellationToken ct = default) await _client.JoinConsumerGroupAsync(_config.StreamId, _config.TopicId, Identifier.String(_consumerGroupName), ct); - Interlocked.Exchange(ref _joinedSessionEpoch, sessionEpoch); + Interlocked.Exchange(ref _joinedSessionGeneration, sessionGeneration); _joinedConsumerGroup = true; LogConsumerGroupJoined(_consumerGroupName); } @@ -378,9 +385,10 @@ private void ThrowIfAutoCommitWithEncryptor() /// private async Task PollMessagesAsync(CancellationToken ct) { - if (!_joinedConsumerGroup || !IsGroupMembershipEpochCurrent()) + if (!_joinedConsumerGroup || !IsGroupMembershipCurrent()) { LogConsumerGroupNotJoinedYetSkippingPolling(); + await TryRecoverGroupMembershipAsync(ct); return; } @@ -467,21 +475,54 @@ private async Task PollMessagesAsync(CancellationToken ct) } /// - /// Whether the session epoch the group membership was stamped under is still the transport's current - /// one. Gating the poll here instead of clearing the joined flag on a Disconnected event survives a - /// late event landing after a rejoin already re-stamped the epoch: state events are published outside - /// the state lock, so their order is not guaranteed. Runs outside , - /// hence the interlocked read. + /// Whether the session generation the group membership was stamped under is still the transport's + /// current one. Gating the poll here instead of clearing the joined flag on a Disconnected event survives + /// a late event landing after a rejoin already re-stamped the generation: state events are published + /// outside the state lock, so their order is not guaranteed. Runs outside + /// , hence the interlocked read. /// - private bool IsGroupMembershipEpochCurrent() + private bool IsGroupMembershipCurrent() { if (_config.Consumer.Type != ConsumerType.ConsumerGroup - || _client is not ISessionEpochProvider epochProvider) + || _client is not ISessionGenerationProvider generationProvider) { return true; } - return epochProvider.SessionEpoch == Interlocked.Read(ref _joinedSessionEpoch); + return generationProvider.SessionGeneration == Interlocked.Read(ref _joinedSessionGeneration); + } + + /// + /// Poll-side rejoin for a membership found missing or stamped under a dead session. The state-event + /// rejoin swallows its failures so the event loop survives them, and the transport suppresses a repeat + /// event for an unchanged state, so without this backstop one failed rejoin would park the consumer for + /// good. The trailing delay keeps the receive loop from spinning while the membership stays gone. + /// + private async Task TryRecoverGroupMembershipAsync(CancellationToken ct) + { + if (_config.Consumer.Type == ConsumerType.ConsumerGroup && _config.JoinConsumerGroup) + { + await _connectionStateSemaphore.WaitAsync(ct); + try + { + if (!_joinedConsumerGroup || !IsGroupMembershipCurrent()) + { + _joinedConsumerGroup = false; + await RejoinConsumerGroupOnReconnectionAsync(); + } + } + finally + { + _connectionStateSemaphore.Release(); + } + + if (_joinedConsumerGroup && IsGroupMembershipCurrent()) + { + return; + } + } + + await Task.Delay(GroupRejoinRetryDelayMs, ct); } /// @@ -542,7 +583,7 @@ private async Task OnClientConnectionStateChangedAsync(ConnectionStateChangedEve await _connectionStateSemaphore.WaitAsync(); try { - if (_client is ISessionEpochProvider epochProvider) + if (_client is ISessionGenerationProvider generationProvider) { if (e.CurrentState != ConnectionState.Authenticated) { @@ -550,7 +591,7 @@ private async Task OnClientConnectionStateChangedAsync(ConnectionStateChangedEve } if (_joinedConsumerGroup - && epochProvider.SessionEpoch == Interlocked.Read(ref _joinedSessionEpoch)) + && generationProvider.SessionGeneration == Interlocked.Read(ref _joinedSessionGeneration)) { return; } diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs index a753dd17f8..2985703fa6 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.Vsr.cs @@ -38,7 +38,7 @@ namespace Apache.Iggy.IggyClient.Implementations; /// register handshake, and the client-side partitioning and consumer-group assignment the broker does not /// resolve server-side. The command surface lives in . /// -public sealed partial class TcpMessageStream : ISessionEpochProvider +public sealed partial class TcpMessageStream : ISessionGenerationProvider { /// /// Upper bound for a whole VSR request: the transient replays and the leader failovers share it, and so @@ -99,7 +99,7 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider private readonly ConsumerGroupClientState _groupState = new(); /// - ulong ISessionEpochProvider.SessionEpoch => _consensusSession.Epoch; + ulong ISessionGenerationProvider.SessionGeneration => _consensusSession.Generation; /// /// Runs the consensus register handshake and binds the session it commits. Everything before the bind @@ -119,18 +119,18 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider { await LogoutUserAsync(token); } - else if (_state == ConnectionState.Authenticated) + else if (State == ConnectionState.Authenticated) { - SetConnectionStateAsync(ConnectionState.Connected); + SetConnectionState(ConnectionState.Connected); } - SetConnectionStateAsync(ConnectionState.Authenticating); + SetConnectionState(ConnectionState.Authenticating); LoginRegisterResponse response; try { using IMemoryOwner responseBuffer = - await SendWithResponseAsync(code, message, token, autoLoginOnReconnect: false); + await SendWithResponseAsync(code, message, autoLoginOnReconnect: false, token: token); response = LoginRegister.Deserialize(responseBuffer.Memory.Span); _consensusSession.Bind(response.Session); @@ -138,9 +138,9 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider catch { await ResetConsensusSessionAsync(); - if (_state == ConnectionState.Authenticating) + if (State == ConnectionState.Authenticating) { - SetConnectionStateAsync(ConnectionState.Connected); + SetConnectionState(ConnectionState.Connected); } throw; @@ -149,7 +149,7 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider _logger.LogInformation( "Authenticated against the server, version {ServerVersion}, protocol version {ServerProtocolVersion}", response.ServerVersion, response.ServerProtocolVersion); - SetConnectionStateAsync(ConnectionState.Authenticated); + SetConnectionState(ConnectionState.Authenticated); var authResponse = new AuthResponse((int)response.UserId, null); if (IsConnecting) @@ -170,7 +170,7 @@ public sealed partial class TcpMessageStream : ISessionEpochProvider // The redirect probe can tear the connection down without throwing, and success on a client that is // no longer bound would leave the caller unauthenticated with nothing left to re-authenticate it. - if (_state != ConnectionState.Authenticated) + if (State != ConnectionState.Authenticated) { throw new NotConnectedException(); } @@ -313,7 +313,7 @@ private async Task SyncGroupAssignmentAsync(Identifier streamId, Identifier topi { var message = TcpContracts.GetGroup(streamId, topicId, groupId); using IMemoryOwner responseBuffer = - await SendWithResponseAsync(CommandCodes.SYNC_CONSUMER_GROUP_CODE, message, token); + await SendWithResponseAsync(CommandCodes.SYNC_CONSUMER_GROUP_CODE, message, token: token); var key = new GroupKey(streamId, topicId, groupId); if (responseBuffer.Memory.Length == 0) @@ -373,9 +373,11 @@ private async Task RedirectAsync(CancellationToken token) // The roster may name the leader by either side of this connection: the address the client dialed // (a hostname, an advertised_address) or the endpoint the socket resolved to. IsSame does no DNS // resolution, so a match on either means the client is already on the leader; checking only one - // side redirect-loops the other kind of roster. - if (ServerAddress.IsSame(leaderAddress, _currentAddress) - || (_currentRemoteAddress.Length > 0 && ServerAddress.IsSame(leaderAddress, _currentRemoteAddress))) + // side redirect-loops the other kind of roster. Both are evidence of where the socket landed, so + // both come from the connect loop: _currentAddress may already name a leader the client never + // reached, and matching on it would silently refuse the redirect that gets it there. + if ((_currentRemoteAddress.Length > 0 && ServerAddress.IsSame(leaderAddress, _currentRemoteAddress)) + || (_connectedAddress.Length > 0 && ServerAddress.IsSame(leaderAddress, _connectedAddress))) { return false; } @@ -483,12 +485,13 @@ private async Task> SendRawAsync(int code, ReadOnlyMemory> SendRawAsync(int code, ReadOnlyMemory> SendRawAsync(int code, ReadOnlyMemory + /// + /// Whether the reply body may carry a credential - a raw personal access token, a session secret - and + /// therefore must be zeroed before its pooled buffer is handed back for reuse. + /// + private static bool HasSensitiveReply(int code) + { + return code is CommandCodes.LOGIN_USER_CODE + or CommandCodes.LOGIN_REGISTER_CODE + or CommandCodes.LOGIN_WITH_PERSONAL_ACCESS_TOKEN_CODE + or CommandCodes.LOGIN_REGISTER_WITH_PAT_CODE + or CommandCodes.CREATE_PERSONAL_ACCESS_TOKEN_CODE; + } + private static bool IsDefinitiveVerdict(Exception error) { return error is IggyInvalidStatusCodeException @@ -587,7 +603,7 @@ private static bool IsDefinitiveVerdict(Exception error) /// connection from a replacement a reconnect installed since. /// private async ValueTask SendVsrAttemptAsync(int code, ReadOnlyMemory body, - long transientDeadline, long readDeadline, CancellationToken token) + long transientDeadline, long readDeadline, bool clearSensitiveReply, CancellationToken token) { await _sendingSemaphore.WaitAsync(token); try @@ -599,7 +615,7 @@ private async ValueTask SendVsrAttemptAsync(int code, ReadOnlyMemory } return await connection.SendAttemptAsync(code, body, transientDeadline, readDeadline, - token); + clearSensitiveReply, token); } finally { @@ -678,7 +694,7 @@ private void DropVsrConnectionLocked(VsrConnection? connection) ResetConsensusSession(); _connection = null; - SetConnectionStateAsync(ConnectionState.Disconnected); + SetConnectionState(ConnectionState.Disconnected); connection.Dispose(); } diff --git a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs index e2328e9e7f..42b3dd7e50 100644 --- a/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs +++ b/foreign/csharp/Iggy_SDK/IggyClient/Implementations/TcpMessageStream.cs @@ -68,14 +68,21 @@ public sealed partial class TcpMessageStream : IIggyClient // mentions), so leader comparisons made against it would move a client that is already on the // leader. Written only by the connect loop. private string _currentRemoteAddress = string.Empty; + + // The dialed address whose connect actually landed. _currentAddress moves ahead of the socket - + // a redirect rewrites it before the reconnect - so a leader comparison against it would treat a + // node the client never reached as "already there". Written only by the connect loop. + private string _connectedAddress = string.Empty; private X509Certificate2Collection _customCaStore = []; private volatile bool _disposed; private int _isConnecting; private DateTimeOffset _lastConnectionTime; - private volatile ConnectionState _state = ConnectionState.Disconnected; + private int _stateValue = (int)ConnectionState.Disconnected; private bool IsConnecting => Volatile.Read(ref _isConnecting) != 0; + private ConnectionState State => (ConnectionState)Volatile.Read(ref _stateValue); + internal TcpMessageStream(IggyClientConfigurator configuration, ILoggerFactory loggerFactory) { _configuration = configuration; @@ -95,7 +102,7 @@ public void Dispose() _connection?.Dispose(); _connection = null; - SetConnectionStateAsync(ConnectionState.Disconnected); + SetConnectionState(ConnectionState.Disconnected); _sendingSemaphore.Dispose(); _connectionSemaphore.Dispose(); _connectGate.Dispose(); @@ -128,7 +135,7 @@ public string GetCurrentAddress() { var message = TcpContracts.CreateStream(name); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.CREATE_STREAM_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.CREATE_STREAM_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -143,7 +150,7 @@ public string GetCurrentAddress() { var message = TcpMessageStreamHelpers.GetBytesFromIdentifier(streamId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_STREAM_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_STREAM_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -158,7 +165,7 @@ public async Task> GetStreamsAsync(CancellationTok { var message = Array.Empty(); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_STREAMS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_STREAMS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -195,7 +202,7 @@ public async Task> GetTopicsAsync(Identifier stream { var message = TcpMessageStreamHelpers.GetBytesFromIdentifier(streamId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_TOPICS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_TOPICS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -211,7 +218,7 @@ public async Task> GetTopicsAsync(Identifier stream { var message = TcpContracts.GetTopicById(streamId, topicId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_TOPIC_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_TOPIC_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -230,7 +237,7 @@ public async Task> GetTopicsAsync(Identifier stream var message = TcpContracts.CreateTopic(streamId, name, partitionsCount, compressionAlgorithm, replicationFactor, messageExpiryValue, maxTopicSize); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.CREATE_TOPIC_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.CREATE_TOPIC_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -347,7 +354,7 @@ public async Task StoreOffsetAsync(Consumer consumer, Identifier streamId, Ident { var message = TcpContracts.GetOffset(streamId, topicId, consumer, partitionId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_OFFSET_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_OFFSET_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -372,7 +379,7 @@ public async Task> GetConsumerGroupsAsync(I { var message = TcpContracts.GetGroups(streamId, topicId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_GROUPS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_GROUPS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -388,7 +395,7 @@ public async Task> GetConsumerGroupsAsync(I { var message = TcpContracts.GetGroup(streamId, topicId, groupId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_GROUP_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_CONSUMER_GROUP_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -404,7 +411,7 @@ public async Task> GetConsumerGroupsAsync(I { var message = TcpContracts.CreateGroup(streamId, topicId, name); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.CREATE_CONSUMER_GROUP_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.CREATE_CONSUMER_GROUP_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -474,7 +481,7 @@ public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, u public async Task GetMeAsync(CancellationToken token = default) { var message = Array.Empty(); - using IMemoryOwner responseBuffer = await SendWithResponseAsync(CommandCodes.GET_ME_CODE, message, token); + using IMemoryOwner responseBuffer = await SendWithResponseAsync(CommandCodes.GET_ME_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -489,7 +496,7 @@ public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, u { var message = Array.Empty(); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_STATS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_STATS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -504,7 +511,7 @@ public async Task DeleteSegmentsAsync(Identifier streamId, Identifier topicId, u { var message = Array.Empty(); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_CLUSTER_METADATA_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -528,7 +535,7 @@ public async Task GetSnapshotAsync(SnapshotCompression compression, IList snapshotTypes, CancellationToken token = default) { var message = TcpContracts.GetSnapshot(compression, snapshotTypes); - using IMemoryOwner result = await SendWithResponseAsync(CommandCodes.GET_SNAPSHOT_CODE, message, token); + using IMemoryOwner result = await SendWithResponseAsync(CommandCodes.GET_SNAPSHOT_CODE, message, token: token); return result.Memory.Span.ToArray(); } @@ -542,9 +549,9 @@ public async Task SendBinaryRequestAsync(uint code, byte[] payload, Canc $"Command {code} cannot be sent as a raw binary request."); } - using IMemoryOwner result = await SendWithResponseAsync((int)code, payload, token); + using IMemoryOwner result = await SendWithResponseAsync((int)code, payload, token: token); - return result.Memory.Length <= 1 ? [] : result.Memory.Span.ToArray(); + return result.Memory.Length == 0 ? [] : result.Memory.Span.ToArray(); } /// @@ -558,7 +565,7 @@ public async Task> GetClientsAsync(CancellationTok { var message = Array.Empty(); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_CLIENTS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_CLIENTS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -573,7 +580,7 @@ public async Task> GetClientsAsync(CancellationTok { var message = TcpContracts.GetClient(clientId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_CLIENT_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_CLIENT_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -588,7 +595,7 @@ public async Task> GetClientsAsync(CancellationTok { var message = TcpContracts.GetUser(userId); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_USER_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_USER_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -603,7 +610,7 @@ public async Task> GetUsersAsync(CancellationToken t { var message = Array.Empty(); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_USERS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_USERS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -619,7 +626,7 @@ public async Task> GetUsersAsync(CancellationToken t { var message = TcpContracts.CreateUser(userName, password, status, permissions); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.CREATE_USER_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.CREATE_USER_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -663,7 +670,7 @@ public async Task ChangePasswordAsync(Identifier userId, string currentPassword, /// public async Task LoginUserAsync(string userName, string password, CancellationToken token = default) { - if (_state == ConnectionState.Disconnected) + if (State == ConnectionState.Disconnected) { throw new NotConnectedException(); } @@ -683,9 +690,9 @@ public async Task LogoutUserAsync(CancellationToken token = default) { await ResetConsensusSessionAsync(); - if (_state == ConnectionState.Authenticated) + if (State == ConnectionState.Authenticated) { - SetConnectionStateAsync(ConnectionState.Connected); + SetConnectionState(ConnectionState.Connected); } } } @@ -696,7 +703,7 @@ public async Task> GetPersonalAccessT { var message = Array.Empty(); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.GET_PERSONAL_ACCESS_TOKENS_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.GET_PERSONAL_ACCESS_TOKENS_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -712,7 +719,7 @@ public async Task> GetPersonalAccessT { var message = TcpContracts.CreatePersonalAccessToken(name, DurationHelpers.ToDuration(expiry)); using IMemoryOwner responseBuffer - = await SendWithResponseAsync(CommandCodes.CREATE_PERSONAL_ACCESS_TOKEN_CODE, message, token); + = await SendWithResponseAsync(CommandCodes.CREATE_PERSONAL_ACCESS_TOKEN_CODE, message, token: token); if (responseBuffer.Memory.Length == 0) { @@ -743,7 +750,7 @@ public async Task DeletePersonalAccessTokenAsync(string name, CancellationToken /// private async Task ConnectAsync(bool autoLogin, CancellationToken token) { - if (_state is ConnectionState.Connected + if (State is ConnectionState.Connected or ConnectionState.Authenticating or ConnectionState.Authenticated) { @@ -755,7 +762,7 @@ or ConnectionState.Authenticating Interlocked.Exchange(ref _isConnecting, 1); try { - if (_state is ConnectionState.Connected + if (State is ConnectionState.Connected or ConnectionState.Authenticating or ConnectionState.Authenticated) { @@ -767,7 +774,7 @@ or ConnectionState.Authenticating await Task.Delay(_configuration.ReconnectionSettings.InitialDelay, token); } - SetConnectionStateAsync(ConnectionState.Connecting); + SetConnectionState(ConnectionState.Connecting); await TryEstablishConnectionAsync(autoLogin, token); } finally @@ -791,7 +798,7 @@ private async Task PollPartitionMessagesRentedAsync(Identi topicId, pollingStrategy, count, autoCommit, partitionId); responseBuffer = await SendWithResponseAsync(CommandCodes.POLL_MESSAGES_CODE, - payload.AsMemory(0, messageBufferSize), token); + payload.AsMemory(0, messageBufferSize), token: token); if (responseBuffer.Memory.Length == 0) { responseBuffer.Dispose(); @@ -858,7 +865,7 @@ private async Task SendConfirmedAndDisposeAsync(IMemoryOwn try { using IMemoryOwner responseBuffer = await SendWithResponseAsync(CommandCodes.SEND_MESSAGES_CODE, - payloadBuffer.Memory[..bodySize], token); + payloadBuffer.Memory[..bodySize], token: token); return BinaryMapper.MapSendMessages(responseBuffer.Memory.Span); } finally @@ -926,6 +933,7 @@ private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken _currentRemoteAddress = socket.RemoteEndPoint is IPEndPoint remote ? ServerAddress.HostPort(remote.Address.ToString(), (ushort)remote.Port) : string.Empty; + _connectedAddress = _currentAddress; socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true); socket.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveTime, 5); @@ -946,7 +954,7 @@ private async Task TryEstablishConnectionAsync(bool autoLogin, CancellationToken _sendingSemaphore.Release(); } - SetConnectionStateAsync(ConnectionState.Connected); + SetConnectionState(ConnectionState.Connected); _lastConnectionTime = DateTimeOffset.UtcNow; socket = null; @@ -980,7 +988,10 @@ await LoginUserAsync(_configuration.AutoLoginSettings.Username, (_configuration.ReconnectionSettings.MaxRetries > 0 && retryCount >= _configuration.ReconnectionSettings.MaxRetries)) { - SetConnectionStateAsync(ConnectionState.Disconnected); + // A failure past the socket handoff (TLS handshake, auto login, redirect probe) leaves + // _connection holding a live stream that nothing would ever close once this throw lands. + await DropVsrConnectionAsync(_connection); + SetConnectionState(ConnectionState.Disconnected); throw; } @@ -1012,7 +1023,7 @@ async Task BackoffOrThrowAsync() { if (++redirects > VsrMaxLeaderRedirects) { - SetConnectionStateAsync(ConnectionState.Disconnected); + SetConnectionState(ConnectionState.Disconnected); throw new MissingLeaderException(); } @@ -1038,11 +1049,11 @@ private async Task CreateSslStreamAndAuthenticate(Socket socket, TlsSett private async Task SendAckAsync(int code, ReadOnlyMemory body, CancellationToken token) { - using IMemoryOwner _ = await SendWithResponseAsync(code, body, token); + using IMemoryOwner _ = await SendWithResponseAsync(code, body, token: token); } private async Task> SendWithResponseAsync(int code, ReadOnlyMemory body, - CancellationToken token, bool autoLoginOnReconnect = true) + bool autoLoginOnReconnect = true, CancellationToken token = default) { try { @@ -1054,7 +1065,7 @@ private async Task> SendWithResponseAsync(int code, ReadOnlyM if (!_configuration.ReconnectionSettings.Enabled) { _logger.LogWarning("Reconnection is disabled"); - SetConnectionStateAsync(ConnectionState.Disconnected); + SetConnectionState(ConnectionState.Disconnected); throw; } @@ -1070,14 +1081,14 @@ private async Task> HandleReconnectionAsync(int code, ReadOnl try { - if (_state is ConnectionState.Connected or ConnectionState.Authenticated + if (State is ConnectionState.Connected or ConnectionState.Authenticated && _lastConnectionTime > currentTime) { _logger.LogInformation("Connection already established, sending payload"); return await SendRawAsync(code, body, token); } - SetConnectionStateAsync(ConnectionState.Disconnected); + SetConnectionState(ConnectionState.Disconnected); _logger.LogInformation("Reconnecting to the server"); await ConnectAsync(autoLogin, token); @@ -1102,19 +1113,20 @@ private static int CalculateMessageBufferSize(Identifier streamId, Identifier to /// /// Sets the connection state and publishes a ConnectionStateChangedEventArgs to subscribers via the connection event - /// aggregator. + /// aggregator. Callers reach this holding different locks - the connect loop the connection semaphore, a drop the + /// sending one, the login path neither - so the swap has to be atomic: a read-modify-write would let a concurrent + /// transition publish a previous state that never preceded the current one. Only the thread that changed the value + /// publishes, so a state is never announced twice. /// /// The new connection state - private void SetConnectionStateAsync(ConnectionState newState) + private void SetConnectionState(ConnectionState newState) { - if (_state == newState) + var previousState = (ConnectionState)Interlocked.Exchange(ref _stateValue, (int)newState); + if (previousState == newState) { return; } - var previousState = _state; - _state = newState; - _logger.LogInformation("Connection state changed: {PreviousState} -> {CurrentState}", previousState, newState); _connectionEvents.Publish(new ConnectionStateChangedEventArgs(previousState, newState)); } diff --git a/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj b/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj index a05dac8165..4708ee716f 100644 --- a/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj +++ b/foreign/csharp/Iggy_SDK/Iggy_SDK.csproj @@ -68,13 +68,13 @@ - - + + - - + + diff --git a/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs b/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs index 5eda4d0d40..58e1bed51f 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/ConsensusSession.cs @@ -38,7 +38,7 @@ internal sealed class ConsensusSession private readonly object _gate = new(); #endif private UInt128 _clientId; - private ulong _epoch; + private ulong _generation; private bool _registerPending; private ulong _requestCounter; private ulong? _session; @@ -92,16 +92,17 @@ internal bool IsBound /// /// Generation of the identity, bumped on every re-arm. Anything scoped to a server session - a - /// consumer-group membership, say - compares the epoch it was established under to detect that the - /// session it lived on is gone, without inferring it from connection-state edges. + /// consumer-group membership, say - compares the generation it was established under to detect that the + /// session it lived on is gone, without inferring it from connection-state edges. Distinct from the + /// fence epoch the wire protocol carries in . /// - internal ulong Epoch + internal ulong Generation { get { lock (_gate) { - return _epoch; + return _generation; } } } @@ -222,7 +223,7 @@ private void ReArmLocked() _session = null; _requestCounter = 1; _registerPending = false; - _epoch++; + _generation++; } private static UInt128 GenerateClientId() diff --git a/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs b/foreign/csharp/Iggy_SDK/Vsr/ISessionGenerationProvider.cs similarity index 94% rename from foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs rename to foreign/csharp/Iggy_SDK/Vsr/ISessionGenerationProvider.cs index 0bf2cf943e..4b96e72baf 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/ISessionEpochProvider.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/ISessionGenerationProvider.cs @@ -23,8 +23,8 @@ namespace Apache.Iggy.Vsr; /// it gets edge-based group rejoin from the connection-state events it publishes; a transport that /// publishes no such events (the built-in HTTP client) gets neither and keeps its membership as-is. /// -public interface ISessionEpochProvider +public interface ISessionGenerationProvider { /// Generation of the transport's consensus session, bumped on every session re-arm. - ulong SessionEpoch { get; } + ulong SessionGeneration { get; } } diff --git a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs index 4410bc804c..3c6d3c325f 100644 --- a/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs +++ b/foreign/csharp/Iggy_SDK/Vsr/VsrConnection.cs @@ -44,15 +44,15 @@ internal sealed class VsrConnection : IDisposable private readonly Action _onDropped; /// - /// Frames up to this size go out as one write: header and body coalesced in + /// Bodies up to this size go out as one write: header and body coalesced in /// . With Nagle disabled every write is its own segment (and its own /// TLS record under a wrapping stream), so the second write a small request would pay is measurable /// even against the round trip the lockstep protocol already costs. /// - private const int CoalescedFrameLimit = 4096; + private const int CoalescedBodyLimit = 4096 - VsrHeader.HEADER_SIZE; private readonly byte[] _replyHeaderBuffer = new byte[VsrHeader.HEADER_SIZE]; - private readonly byte[] _requestFrameBuffer = new byte[CoalescedFrameLimit]; + private readonly byte[] _requestFrameBuffer = new byte[VsrHeader.HEADER_SIZE + CoalescedBodyLimit]; private readonly int _requestTimeoutMs; /// The session identity requests on this connection encode from. @@ -91,7 +91,7 @@ IOException or /// which also guards the reuse of the per-connection frame buffer. /// internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory body, - long transientDeadline, long readDeadline, CancellationToken token) + long transientDeadline, long readDeadline, bool clearSensitiveReply, CancellationToken token) { var encoded = false; var requestStarted = false; @@ -101,7 +101,7 @@ internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory SendAttemptAsync(int code, ReadOnlyMemory response = await ReadReplyAsync(readDeadline, token); + IMemoryOwner response = await ReadReplyAsync(readDeadline, clearSensitiveReply, token); return VsrAttempt.Ok(response, this); } @@ -179,7 +177,8 @@ internal async ValueTask SendAttemptAsync(int code, ReadOnlyMemory> ReadReplyAsync(long readDeadline, CancellationToken token) + private async Task> ReadReplyAsync(long readDeadline, bool clearSensitiveReply, + CancellationToken token) { var remaining = readDeadline - Environment.TickCount64; if (remaining <= 0) @@ -250,18 +249,18 @@ private async Task> ReadReplyAsync(long readDeadline, Cancell ReadOnlyMemory decoded = VsrReplyDecoder.Decode(_replyHeaderBuffer, buffer.AsMemory(0, bodySize)); if (decoded.IsEmpty) { - ArrayPool.Shared.Return(buffer); + ArrayPool.Shared.Return(buffer, clearSensitiveReply); return EmptyMemoryOwner.Instance; } // The decoded payload is always a suffix of the body - the funnel only strips the leading // committed result section. - return new PooledMemoryOwner(buffer, bodySize - decoded.Length, decoded.Length); + return new PooledMemoryOwner(buffer, bodySize - decoded.Length, decoded.Length, clearSensitiveReply); } catch { - ArrayPool.Shared.Return(buffer); + ArrayPool.Shared.Return(buffer, clearSensitiveReply); throw; } } @@ -352,8 +351,12 @@ public void Dispose() } } -/// Owns a pooled buffer while exposing only the decoded payload slice inside it. -internal sealed class PooledMemoryOwner(byte[] buffer, int start, int length) : IMemoryOwner +/// +/// Owns a pooled buffer while exposing only the decoded payload slice inside it. A buffer that carried a +/// credential is zeroed on the way back to the pool so the secret cannot resurface in a later rental. +/// +internal sealed class PooledMemoryOwner(byte[] buffer, int start, int length, bool clearOnReturn = false) + : IMemoryOwner { private int _disposed; @@ -363,7 +366,7 @@ public void Dispose() { if (Interlocked.Exchange(ref _disposed, 1) == 0) { - ArrayPool.Shared.Return(buffer); + ArrayPool.Shared.Return(buffer, clearOnReturn); } } } diff --git a/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs b/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionGenerationTests.cs similarity index 72% rename from foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs rename to foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionGenerationTests.cs index ee39c96ba9..00fbd147f4 100644 --- a/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionEpochTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/ConsumerTests/ConsumerSessionGenerationTests.cs @@ -28,16 +28,16 @@ namespace Apache.Iggy.Tests.ConsumerTests; /// -/// Group membership on a transport that exposes a consensus session is keyed off the session epoch rather -/// than off connection-state edges, so these cover both arms of that branch. +/// Group membership on a transport that exposes a consensus session is keyed off the session generation +/// rather than off connection-state edges, so these cover both arms of that branch. /// -public sealed class ConsumerSessionEpochTests +public sealed class ConsumerSessionGenerationTests { [Fact] public async Task - given_group_consumer_when_reauthenticated_on_the_same_session_epoch_should_not_rejoin_the_group() + given_group_consumer_when_reauthenticated_on_the_same_session_generation_should_not_rejoin_the_group() { - var client = new EpochClient(); + var client = new GenerationClient(); var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); await consumer.InitAsync(TestContext.Current.CancellationToken); Assert.Equal(1, client.JoinCount); @@ -49,21 +49,21 @@ public async Task } [Fact] - public async Task given_group_consumer_when_the_session_epoch_moved_should_rejoin_the_group() + public async Task given_group_consumer_when_the_session_generation_moved_should_rejoin_the_group() { - var client = new EpochClient(); + var client = new GenerationClient(); var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); await consumer.InitAsync(TestContext.Current.CancellationToken); Assert.Equal(1, client.JoinCount); // The session the membership was established under is gone. - client.SessionEpoch++; + client.SessionGeneration++; await client.RaiseAsync(ConnectionState.Connected, ConnectionState.Authenticated); Assert.Equal(2, client.JoinCount); - // The rejoin must re-stamp the membership epoch: without it every later event would rejoin again, - // triggering a group-wide rebalance per reconnect. + // The rejoin must re-stamp the membership generation: without it every later event would rejoin + // again, triggering a group-wide rebalance per reconnect. await client.RaiseAsync(ConnectionState.Connected, ConnectionState.Authenticated); Assert.Equal(2, client.JoinCount); @@ -71,27 +71,53 @@ public async Task given_group_consumer_when_the_session_epoch_moved_should_rejoi } /// - /// A disconnect re-arms the transport's session, so the epoch moves and the reconnect rejoins. The - /// membership is not cleared on the Disconnected event itself: the poll gate compares epochs instead, - /// which survives state events arriving out of order. + /// A disconnect re-arms the transport's session, so the generation moves and the reconnect rejoins. The + /// membership is not cleared on the Disconnected event itself: the poll gate compares generations + /// instead, which survives state events arriving out of order. /// [Fact] public async Task given_group_consumer_when_disconnected_should_surrender_membership_and_rejoin_on_reconnect() { - var client = new EpochClient(); + var client = new GenerationClient(); var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); await consumer.InitAsync(TestContext.Current.CancellationToken); Assert.Equal(1, client.JoinCount); await client.RaiseAsync(ConnectionState.Authenticated, ConnectionState.Disconnected); // The transport re-arms the consensus session when the connection drops. - client.SessionEpoch++; + client.SessionGeneration++; await client.RaiseAsync(ConnectionState.Connecting, ConnectionState.Authenticated); Assert.Equal(2, client.JoinCount); await consumer.DisposeAsync(); } + /// + /// The event-time rejoin swallows its failures and the transport never republishes an unchanged state, + /// so the poll loop is the only place left that can restore a membership the event handler failed to. + /// + [Fact] + public async Task given_group_consumer_when_the_event_time_rejoin_fails_should_rejoin_from_the_poll_loop() + { + var client = new GenerationClient(); + var consumer = new IggyConsumer(client.Object, BuildGroupConfig(), NullLoggerFactory.Instance); + await consumer.InitAsync(TestContext.Current.CancellationToken); + Assert.Equal(1, client.JoinCount); + + client.SessionGeneration++; + client.FailNextJoin = true; + await client.RaiseAsync(ConnectionState.Connected, ConnectionState.Authenticated); + Assert.Equal(1, client.JoinCount); + + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); + await using IAsyncEnumerator messages = consumer.ReceiveAsync(cts.Token) + .GetAsyncEnumerator(TestContext.Current.CancellationToken); + + Assert.True(await messages.MoveNextAsync()); + Assert.Equal(2, client.JoinCount); + await consumer.DisposeAsync(); + } + /// /// A plain consumer holds no membership, so nothing may clear its joined flag: no code path would ever /// set it again and every later poll would be skipped. @@ -99,7 +125,7 @@ public async Task given_group_consumer_when_disconnected_should_surrender_member [Fact] public async Task given_plain_consumer_when_disconnected_should_keep_polling_after_reconnect() { - var client = new EpochClient(); + var client = new GenerationClient(); var config = BuildGroupConfig(); config.Consumer = Consumer.New(1); var consumer = new IggyConsumer(client.Object, config, NullLoggerFactory.Instance); @@ -107,7 +133,7 @@ public async Task given_plain_consumer_when_disconnected_should_keep_polling_aft Assert.Equal(0, client.JoinCount); await client.RaiseAsync(ConnectionState.Authenticated, ConnectionState.Disconnected); - client.SessionEpoch++; + client.SessionGeneration++; await client.RaiseAsync(ConnectionState.Connecting, ConnectionState.Authenticated); var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); @@ -138,20 +164,24 @@ private static IggyConsumerConfig BuildGroupConfig() /// A client that carries a consensus session, counts group joins, and replays connection-state events on /// demand so a test can drive the reconnection handler without a socket. /// - private sealed class EpochClient + private sealed class GenerationClient { private readonly List> _subscribers = []; public IIggyClient Object { get; } - public ulong SessionEpoch { get; set; } + public ulong SessionGeneration { get; set; } public int JoinCount { get; private set; } - public EpochClient() + /// The next join attempt throws instead of counting, then the flag disarms itself. + public bool FailNextJoin { get; set; } + + public GenerationClient() { var mock = new Mock(MockBehavior.Loose); - mock.As().SetupGet(c => c.SessionEpoch).Returns(() => SessionEpoch); + mock.As().SetupGet(c => c.SessionGeneration) + .Returns(() => SessionGeneration); mock.Setup(c => c.ConnectAsync(It.IsAny())).Returns(Task.CompletedTask); mock.Setup(c => c.SubscribeConnectionEvents(It.IsAny>())) .Callback>(_subscribers.Add); @@ -168,7 +198,16 @@ public EpochClient() }); mock.Setup(c => c.JoinConsumerGroupAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Callback(() => JoinCount++) + .Callback(() => + { + if (FailNextJoin) + { + FailNextJoin = false; + throw new InvalidOperationException("Injected join failure."); + } + + JoinCount++; + }) .Returns(Task.CompletedTask); mock.Setup(c => c.PollMessagesAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), diff --git a/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs b/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs index aaf438213c..3ff5256c85 100644 --- a/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs +++ b/foreign/csharp/Iggy_SDK_Tests/VsrTests/ConsensusSessionTests.cs @@ -217,22 +217,22 @@ public void Reset_ClearsBindingAndCounter() } [Fact] - public void Epoch_AdvancesOnEveryReArmAndNotOnBind() + public void Generation_AdvancesOnEveryReArmAndNotOnBind() { var session = new ConsensusSession(1); - var initialEpoch = session.Epoch; + var initialGeneration = session.Generation; session.Resolve(VsrOperation.Register); session.Bind(10); - Assert.Equal(initialEpoch, session.Epoch); + Assert.Equal(initialGeneration, session.Generation); session.Reset(); - Assert.Equal(initialEpoch + 1, session.Epoch); + Assert.Equal(initialGeneration + 1, session.Generation); - // A register on a previously bound session re-arms the identity, which is a new epoch too. + // A register on a previously bound session re-arms the identity, which is a new generation too. session.Resolve(VsrOperation.Register); session.Bind(11); session.Resolve(VsrOperation.Register); - Assert.Equal(initialEpoch + 2, session.Epoch); + Assert.Equal(initialGeneration + 2, session.Generation); } }