diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs index a6caf226..f588d566 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs @@ -9,6 +9,12 @@ namespace ByteSync.Services.Communications.Transfers.Strategies; public static class UploadFailureClassifier { + private static readonly string[] UnexpectedTransportClosureMessageFragments = + { + "unexpected EOF", + "0 bytes from the transport stream", + }; + public static UploadFileResponse Classify(Exception exception, CancellationToken cancellationToken) { if (exception is OperationCanceledException && cancellationToken.IsCancellationRequested) @@ -21,6 +27,11 @@ public static UploadFileResponse Classify(Exception exception, CancellationToken return UploadFileResponse.ClientTimeout(exception); } + if (cancellationToken.IsCancellationRequested) + { + return UploadFileResponse.ClientCancellation(exception); + } + if (IsClientNetworkError(exception)) { return UploadFileResponse.ClientNetworkError(exception); @@ -43,6 +54,7 @@ private static bool IsClientNetworkError(Exception exception) { return socketException.SocketErrorCode is SocketError.ConnectionReset or SocketError.ConnectionAborted + or SocketError.OperationAborted or SocketError.TimedOut or SocketError.NetworkDown or SocketError.NetworkUnreachable @@ -50,9 +62,27 @@ or SocketError.HostDown or SocketError.HostUnreachable; } + if (current is IOException ioException && HasUnexpectedTransportClosureMessage(ioException)) + { + return true; + } + current = current.InnerException; } return false; } + + private static bool HasUnexpectedTransportClosureMessage(IOException exception) + { + foreach (var fragment in UnexpectedTransportClosureMessageFragments) + { + if (exception.Message.Contains(fragment, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } } diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs index f3e258de..77764db8 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs @@ -15,6 +15,7 @@ public class AdaptiveUploadController : IAdaptiveUploadController private const int MIN_PARALLELISM = 2; private const int MAX_PARALLELISM = 4; private const int CLIENT_NETWORK_ISSUES_BEFORE_DOWNSCALE = 2; + private const double CLIENT_NETWORK_ISSUE_CHUNK_FACTOR = 0.5; private const double MULTIPLIER_2_X = 2.0; private const double MULTIPLIER_1_75_X = 1.75; @@ -182,7 +183,7 @@ private void HandleClientNetworkIssue(string? fileId, UploadFailureKind failureK fileId ?? "-", _consecutiveClientNetworkIssues); _consecutiveClientNetworkIssues = 0; - Downscale(fileId, "client network issues"); + DownscaleForClientNetworkIssue(fileId, failureKind); } private bool HandleBandwidthReset(bool isSuccess, int? statusCode) @@ -259,6 +260,29 @@ private void Downscale(string? fileId, string reason) ResetWindow(); } + + private void DownscaleForClientNetworkIssue(string? fileId, UploadFailureKind failureKind) + { + var previousParallelism = _currentParallelism; + var previousChunkSizeBytes = _currentChunkSizeBytes; + + _currentParallelism = MIN_PARALLELISM; + _currentChunkSizeBytes = Math.Max( + MIN_CHUNK_SIZE_BYTES, + (int)Math.Round(_currentChunkSizeBytes * CLIENT_NETWORK_ISSUE_CHUNK_FACTOR)); + _windowSize = _currentParallelism; + + _logger.LogInformation( + "Adaptive: file {FileId} client network issue downscale ({FailureKind}). Parallelism {PrevParallelism}->{NextParallelism}, chunkSize {PrevChunkKb}->{NextChunkKb} KB", + fileId ?? "-", + failureKind, + previousParallelism, + _currentParallelism, + Math.Round(previousChunkSizeBytes / 1024d), + Math.Round(_currentChunkSizeBytes / 1024d)); + + ResetWindow(); + } private void TryHandleUpscale(string? fileId) { diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs index 62f3b5ad..68fa080d 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs @@ -174,18 +174,22 @@ private async Task ExecuteUploadAttemptAsync(FileUploaderSli slice.MemoryStream.Length, attempt, currentChunkSizeBytes); + var chunkRatio = currentChunkSizeBytes > 0 + ? slice.MemoryStream.Length / (double)currentChunkSizeBytes + : 0d; using var attemptCts = CancellationTokenSource.CreateLinkedTokenSource(globalToken); attemptCts.CancelAfter(TimeSpan.FromSeconds(timeoutSec)); var beforeWait = _uploadSlots.CurrentCount; _logger.LogDebug( - "UploadAvailableSlice: worker {WorkerId} waiting for upload slot (available {Available}), attempt {Attempt}, timeout {TimeoutSec}s, slice {SliceKb} KB, currentChunk {CurrentChunkKb} KB", + "UploadAvailableSlice: worker {WorkerId} waiting for upload slot (available {Available}), attempt {Attempt}, timeout {TimeoutSec}s, slice {SliceKb} KB, currentChunk {CurrentChunkKb} KB, chunkRatio {ChunkRatio}", workerId, beforeWait, attempt, timeoutSec, Math.Round(slice.MemoryStream.Length / 1024d), - Math.Round(currentChunkSizeBytes / 1024d)); + Math.Round(currentChunkSizeBytes / 1024d), + Math.Round(chunkRatio, 2)); var acquired = false; try diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicy.cs b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicy.cs index a1376e81..83add7bf 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicy.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicy.cs @@ -6,22 +6,26 @@ public static class UploadAttemptTimeoutPolicy { private const int AttemptTimeoutFloorSeconds = 60; private const int AttemptTimeoutCeilingSeconds = 180; + private const int ExtendedOversizedSliceTimeoutCeilingSeconds = 300; private const int SecondsPerMegabyteHeuristic = 3; private const int RetryGrowthSeconds = 15; - private const int StaleChunkPenaltySeconds = 5; + private const int OversizedSliceSecondsPerCurrentChunk = 30; + private const int ExtendedOversizedSliceThresholdBytes = 1024 * 1024; + private const int ExtendedOversizedSliceChunkRatioThreshold = 8; public static int ComputeTimeoutSeconds(long sliceLengthBytes, int attempt, int currentChunkSizeBytes) { - var timeoutSec = (long)ComputeBaseTimeoutSeconds(sliceLengthBytes); - if (attempt <= 1) + var timeoutCeilingSeconds = ComputeTimeoutCeilingSeconds(sliceLengthBytes, currentChunkSizeBytes); + var timeoutSec = Math.Max( + (long)ComputeBaseTimeoutSeconds(sliceLengthBytes), + ComputeOversizedSliceTimeoutSeconds(sliceLengthBytes, currentChunkSizeBytes, timeoutCeilingSeconds)); + + if (attempt > 1) { - return (int)timeoutSec; + timeoutSec += (long)(attempt - 1) * RetryGrowthSeconds; } - var staleChunkPenalty = ComputeStaleChunkPenaltySeconds(sliceLengthBytes, currentChunkSizeBytes); - timeoutSec += (long)(attempt - 1) * RetryGrowthSeconds + staleChunkPenalty; - - return (int)Math.Clamp(timeoutSec, AttemptTimeoutFloorSeconds, AttemptTimeoutCeilingSeconds); + return (int)Math.Clamp(timeoutSec, AttemptTimeoutFloorSeconds, timeoutCeilingSeconds); } private static int ComputeBaseTimeoutSeconds(long sliceLengthBytes) @@ -39,7 +43,24 @@ private static int ComputeBaseTimeoutSeconds(long sliceLengthBytes) AttemptTimeoutCeilingSeconds); } - private static long ComputeStaleChunkPenaltySeconds(long sliceLengthBytes, int currentChunkSizeBytes) + private static int ComputeTimeoutCeilingSeconds(long sliceLengthBytes, int currentChunkSizeBytes) + { + if (currentChunkSizeBytes <= 0 || sliceLengthBytes <= currentChunkSizeBytes) + { + return AttemptTimeoutCeilingSeconds; + } + + var chunkRatio = Math.Ceiling(sliceLengthBytes / (double)currentChunkSizeBytes); + if (sliceLengthBytes >= ExtendedOversizedSliceThresholdBytes + && chunkRatio >= ExtendedOversizedSliceChunkRatioThreshold) + { + return ExtendedOversizedSliceTimeoutCeilingSeconds; + } + + return AttemptTimeoutCeilingSeconds; + } + + private static long ComputeOversizedSliceTimeoutSeconds(long sliceLengthBytes, int currentChunkSizeBytes, int timeoutCeilingSeconds) { if (currentChunkSizeBytes <= 0 || sliceLengthBytes <= currentChunkSizeBytes) { @@ -47,11 +68,11 @@ private static long ComputeStaleChunkPenaltySeconds(long sliceLengthBytes, int c } var chunkRatio = Math.Ceiling(sliceLengthBytes / (double)currentChunkSizeBytes); - if (chunkRatio >= AttemptTimeoutCeilingSeconds / (double)StaleChunkPenaltySeconds + 1) + if (chunkRatio >= timeoutCeilingSeconds / (double)OversizedSliceSecondsPerCurrentChunk) { - return AttemptTimeoutCeilingSeconds; + return timeoutCeilingSeconds; } - return (long)(chunkRatio - 1) * StaleChunkPenaltySeconds; + return (long)chunkRatio * OversizedSliceSecondsPerCurrentChunk; } } diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs index 0c6a8acf..d8d0ca73 100644 --- a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs @@ -70,7 +70,6 @@ public void Classify_TaskCanceledException_WithNonCancelledToken_ShouldReturnCli public void Classify_GenericException_ShouldReturnServerError500() { using var cts = new CancellationTokenSource(); - cts.Cancel(); var ex = new InvalidOperationException("broken"); var response = UploadFailureClassifier.Classify(ex, cts.Token); @@ -111,6 +110,22 @@ public void Classify_HttpRequestExceptionWithConnectionReset_ShouldReturnClientN response.Exception.Should().BeSameAs(ex); } + [TestCase("Received an unexpected EOF from the transport stream.")] + [TestCase("Received 0 bytes from the transport stream.")] + public void Classify_HttpRequestExceptionWithUnexpectedTransportClosureMessage_ShouldReturnClientNetworkError(string message) + { + using var cts = new CancellationTokenSource(); + var ioException = new IOException(message); + var ex = new HttpRequestException("The SSL connection could not be established, see inner exception.", ioException); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(0); + response.FailureKind.Should().Be(UploadFailureKind.ClientNetworkError); + response.Exception.Should().BeSameAs(ex); + } + [Test] public void Classify_DirectSocketExceptionWithConnectionReset_ShouldReturnClientNetworkError() { @@ -124,4 +139,33 @@ public void Classify_DirectSocketExceptionWithConnectionReset_ShouldReturnClient response.FailureKind.Should().Be(UploadFailureKind.ClientNetworkError); response.Exception.Should().BeSameAs(ex); } + + [Test] + public void Classify_DirectSocketExceptionWithOperationAborted_ShouldReturnClientNetworkError() + { + using var cts = new CancellationTokenSource(); + var ex = new SocketException((int)SocketError.OperationAborted); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(0); + response.FailureKind.Should().Be(UploadFailureKind.ClientNetworkError); + response.Exception.Should().BeSameAs(ex); + } + + [Test] + public void Classify_DirectSocketExceptionWithOperationAbortedAndCancelledToken_ShouldReturnClientCancellation() + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + var ex = new SocketException((int)SocketError.OperationAborted); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(0); + response.FailureKind.Should().Be(UploadFailureKind.ClientCancellation); + response.Exception.Should().BeSameAs(ex); + } } diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs index 1ddc2442..c0b28138 100644 --- a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs @@ -120,6 +120,7 @@ public void Downscale_ReducesChunkSize_WhenAtMinParallelism() _controller.CurrentParallelism.Should().Be(2); _controller.CurrentChunkSizeBytes.Should().BeLessThan(before); _controller.CurrentChunkSizeBytes.Should().BeGreaterThanOrEqualTo(64 * 1024); + _controller.CurrentChunkSizeBytes.Should().Be(375 * 1024); } [Test] @@ -197,7 +198,7 @@ public void ClientTimeouts_DownscaleBelowInitialChunkSize_WhenAtMinParallelism() // Assert _controller.CurrentParallelism.Should().Be(2); _controller.CurrentChunkSizeBytes.Should().BeLessThan(500 * 1024); - _controller.CurrentChunkSizeBytes.Should().Be(375 * 1024); + _controller.CurrentChunkSizeBytes.Should().Be(250 * 1024); } [Test] @@ -212,11 +213,11 @@ public void ClientNetworkErrors_DownscaleBelowInitialChunkSize_WhenAtMinParallel // Assert _controller.CurrentParallelism.Should().Be(2); - _controller.CurrentChunkSizeBytes.Should().Be(375 * 1024); + _controller.CurrentChunkSizeBytes.Should().Be(250 * 1024); } [Test] - public void ClientTimeouts_ReduceParallelismFirst_WhenAboveMinParallelism() + public void ClientTimeouts_DownscaleParallelismAndChunk_WhenAboveMinParallelism() { // Arrange var safety = 100; @@ -233,8 +234,9 @@ public void ClientTimeouts_ReduceParallelismFirst_WhenAboveMinParallelism() FeedClientTimeouts(_controller, 2); // Assert - _controller.CurrentParallelism.Should().Be(beforeParallelism - 1); - _controller.CurrentChunkSizeBytes.Should().Be(beforeChunk); + _controller.CurrentParallelism.Should().Be(2); + _controller.CurrentParallelism.Should().BeLessThan(beforeParallelism); + _controller.CurrentChunkSizeBytes.Should().Be(Math.Max(64 * 1024, (int)Math.Round(beforeChunk * 0.5))); } [Test] diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicyTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicyTests.cs index decd47d9..a5c485a6 100644 --- a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicyTests.cs +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/UploadAttemptTimeoutPolicyTests.cs @@ -21,16 +21,16 @@ public void ComputeTimeoutSeconds_FirstAttempt_ShouldUseFloorForSmallSlices() } [Test] - public void ComputeTimeoutSeconds_RetryForStaleLargeSlice_ShouldIncreaseBudget() + public void ComputeTimeoutSeconds_FirstAttemptForOversizedSlice_ShouldIncreaseBudgetImmediately() { // Act var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( 625 * 1024, - attempt: 2, + attempt: 1, currentChunkSizeBytes: 64 * 1024); // Assert - timeout.Should().Be(120); + timeout.Should().Be(180); } [Test] @@ -45,35 +45,74 @@ public void ComputeTimeoutSeconds_RetryForCurrentChunkSizedSlice_ShouldGrowGradu // Assert timeout.Should().Be(75); } + + [Test] + public void ComputeTimeoutSeconds_RetryForOversizedSlice_ShouldKeepRetryGrowthBounded() + { + // Act + var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( + 2 * 1024 * 1024, + attempt: 2, + currentChunkSizeBytes: 500 * 1024); + + // Assert + timeout.Should().Be(165); + } + + [Test] + public void ComputeTimeoutSeconds_RetryForModeratelyOversizedSlice_ShouldKeepStandardCeiling() + { + // Act + var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( + 2 * 1024 * 1024, + attempt: 4, + currentChunkSizeBytes: 500 * 1024); + + // Assert + timeout.Should().Be(180); + } + + [Test] + public void ComputeTimeoutSeconds_RetryForLargeStaleOversizedSlice_ShouldUseExtendedCeiling() + { + // Act + var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( + 2 * 1024 * 1024, + attempt: 4, + currentChunkSizeBytes: 250 * 1024); + + // Assert + timeout.Should().Be(300); + } [Test] - public void ComputeTimeoutSeconds_ShouldNotExceedCeiling() + public void ComputeTimeoutSeconds_CurrentChunkSizedLargeSlice_ShouldKeepStandardCeiling() { // Act var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( 16 * 1024 * 1024, attempt: 10, - currentChunkSizeBytes: 64 * 1024); + currentChunkSizeBytes: 16 * 1024 * 1024); // Assert timeout.Should().Be(180); } [Test] - public void ComputeTimeoutSeconds_ForLargeStaleSliceAtLowBandwidth_ShouldAllowMoreThanTwoMinutes() + public void ComputeTimeoutSeconds_FirstAttemptForLargeStaleSlice_ShouldScaleWithChunkRatio() { // Act var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( 1969 * 1024, - attempt: 6, + attempt: 1, currentChunkSizeBytes: 500 * 1024); // Assert - timeout.Should().Be(150); + timeout.Should().Be(120); } [Test] - public void ComputeTimeoutSeconds_WithHugeSlice_ShouldNotOverflow() + public void ComputeTimeoutSeconds_WithHugeStaleSlice_ShouldNotOverflow() { // Act var timeout = UploadAttemptTimeoutPolicy.ComputeTimeoutSeconds( @@ -82,7 +121,7 @@ public void ComputeTimeoutSeconds_WithHugeSlice_ShouldNotOverflow() currentChunkSizeBytes: 64 * 1024); // Assert - timeout.Should().Be(180); + timeout.Should().Be(300); } [Test] @@ -95,6 +134,6 @@ public void ComputeTimeoutSeconds_WithHugeStaleRatio_ShouldNotOverflow() currentChunkSizeBytes: 1); // Assert - timeout.Should().Be(180); + timeout.Should().Be(300); } }