Skip to content

Fix upload directory progress inconsistency #4258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
philasmar wants to merge 6 commits into from
Closed

Fix upload directory progress inconsistency #4258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
db333a7
Fix upload directory progress inconsistency
philasmar Dec 23, 2025
c891471
add dev config
philasmar Dec 23, 2025
5e8d8e3
fix dev config
philasmar Dec 23, 2025
14397f2
fix test
philasmar Dec 29, 2025
0b9cea2
Merge branch 'asmarp/fix-upload-progress-consistency' of https://gith…
philasmar Dec 29, 2025
66193b8
fix test
philasmar Dec 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions generator/.DevConfigs/9f9b4605-608f-45c3-80dc-6b8d2bcb036d.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"services": [
{
"serviceName": "S3",
"type": "patch",
"changeLogMessages": [
"Update directory upload progress calculation to properly keep track of the number of files successfully uploaded."
]
}
]
}
7 changes: 1 addition & 6 deletions sdk/src/Services/S3/Custom/Transfer/Internal/UploadDirectoryCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ internal partial class UploadDirectoryCommand : BaseCommand<TransferUtilityUploa
TransferUtilityConfig _config;

int _totalNumberOfFiles;
int _numberOfFilesUploaded;
int _numberOfFilesSuccessfullyUploaded;
long _totalBytes;
long _transferredBytes;
Expand Down Expand Up @@ -165,11 +164,7 @@ private void UploadProgressEventCallback(object sender, UploadProgressArgs e)

var totalTransferredBytes = Interlocked.Add(ref _transferredBytes, e.IncrementTransferred - e.CompensationForRetry);

int numberOfFilesUploaded = _numberOfFilesUploaded;
if (e.TransferredBytes == e.TotalBytes)
{
numberOfFilesUploaded = Interlocked.Increment(ref _numberOfFilesUploaded);
}
int numberOfFilesUploaded = _numberOfFilesSuccessfullyUploaded;
Comment thread
GarrettBeatty marked this conversation as resolved.

UploadDirectoryProgressArgs uploadDirectoryProgressArgs = null;
if (_request.UploadFilesConcurrently)
Expand Down
19 changes: 7 additions & 12 deletions sdk/test/NetStandard/IntegrationTests/IntegrationTests/S3/TransferUtilityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public async Task UploadDirectoryWithProgressTracker()
progressValidator.AssertOnCompletion();
}

private Task UploadDirectoryAsync(string directoryName, long size,
private Task UploadDirectoryAsync(string directoryName, long size,
DirectoryProgressValidator<UploadDirectoryProgressArgs> progressValidator, bool validate = true, bool concurrent = true)
{
var directoryPath = Path.Combine(basePath, directoryName);
Expand Down Expand Up @@ -228,7 +228,7 @@ private Task UploadDirectoryAsync(string directoryName, long size,
HashSet<string> files = new HashSet<string>();
request.UploadDirectoryProgressEvent += (s, e) =>
{
files.Add(e.CurrentFile);
files.Add(e.CurrentFile);
Console.WriteLine("Progress callback = " + e.ToString());
};

Expand Down Expand Up @@ -505,7 +505,8 @@ public static void ConfigureProgressValidator(DirectoryProgressValidator<Downloa
}
}

if (progress.NumberOfFilesDownloaded == progress.TotalNumberOfFiles)
if ((progress.NumberOfFilesDownloaded == progress.TotalNumberOfFiles) ||
(progress.TotalBytes > 0 && progress.TransferredBytes == progress.TotalBytes))
{
Assert.Equal(progress.TransferredBytes, progress.TotalBytes);
progressValidator.IsProgressEventComplete = true;
Expand All @@ -527,17 +528,11 @@ public static void ConfigureProgressValidator(DirectoryProgressValidator<UploadD
{
Assert.True(progress.NumberOfFilesUploaded >= lastProgress.NumberOfFilesUploaded);
Assert.True(progress.TransferredBytes > lastProgress.TransferredBytes);
if (progress.NumberOfFilesUploaded == lastProgress.NumberOfFilesUploaded)
{
Assert.True(progress.TransferredBytes - lastProgress.TransferredBytes >= 100 * KILO_SIZE);
}
else
{
Assert.Equal(progress.TransferredBytesForCurrentFile, progress.TotalNumberOfBytesForCurrentFile);
}
Assert.True(progress.TransferredBytes - lastProgress.TransferredBytes > 0);
}

if (progress.NumberOfFilesUploaded == progress.TotalNumberOfFiles)
if ((progress.NumberOfFilesUploaded == progress.TotalNumberOfFiles) ||
(progress.TotalBytes > 0 && progress.TransferredBytes == progress.TotalBytes))
{
Assert.Equal(progress.TransferredBytes, progress.TotalBytes);
progressValidator.IsProgressEventComplete = true;
Expand Down
17 changes: 6 additions & 11 deletions sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ public void UploadUnSeekableStreamFileSizeEqualToPartBufferSize()

[TestMethod]
[TestCategory("S3")]
public void UploadUnseekableStreamFileSizeBetweenMinPartSizeAndPartBufferSize()
public void UploadUnSeekableStreamFileSizeBetweenMinPartSizeAndPartBufferSize()
{
var client = Client;
var fileName = UtilityMethods.GenerateName(@"SimpleUploadTest\BetweenMinPartSizeAndPartBufferSize");
Expand Down Expand Up @@ -2077,7 +2077,8 @@ public static void ConfigureProgressValidator(DirectoryProgressValidator<Downloa
}
}

if (progress.NumberOfFilesDownloaded == progress.TotalNumberOfFiles)
if ((progress.NumberOfFilesDownloaded == progress.TotalNumberOfFiles) ||
(progress.TotalBytes > 0 && progress.TransferredBytes == progress.TotalBytes))
{
Assert.AreEqual(progress.TransferredBytes, progress.TotalBytes);
progressValidator.IsProgressEventComplete = true;
Expand All @@ -2103,17 +2104,11 @@ public static void ConfigureProgressValidator(DirectoryProgressValidator<UploadD
{
Assert.IsTrue(progress.NumberOfFilesUploaded >= lastProgress.NumberOfFilesUploaded);
Assert.IsTrue(progress.TransferredBytes > lastProgress.TransferredBytes);
if (progress.NumberOfFilesUploaded == lastProgress.NumberOfFilesUploaded)
{
Assert.IsTrue(progress.TransferredBytes - lastProgress.TransferredBytes >= 100 * KILO_SIZE);
}
else
{
Assert.AreEqual(progress.TransferredBytesForCurrentFile, progress.TotalNumberOfBytesForCurrentFile);
Assert.IsTrue(progress.TransferredBytes - lastProgress.TransferredBytes > 0);
}
}

if (progress.NumberOfFilesUploaded == progress.TotalNumberOfFiles)
if ((progress.NumberOfFilesUploaded == progress.TotalNumberOfFiles) ||
(progress.TotalBytes > 0 && progress.TransferredBytes == progress.TotalBytes))
{
Assert.AreEqual(progress.TransferredBytes, progress.TotalBytes);
progressValidator.IsProgressEventComplete = true;
Expand Down