Asynchronously processing GetObjectAsync stream

[luaneko]

Feature request:

GetObjectAsync takes a synchronous stream processor. It defeats the point of the call being asynchronous, unless I'm misunderstanding something (is the stream buffered in memory already?)

Task GetObjectAsync(string bucketName, string objectName, Action<Stream> callback, ServerSideEncryption sse = null, CancellationToken cancellationToken = default(CancellationToken))

It would be great to see an overload like:

Task GetObjectAsync(string bucketName, string objectName, Func<Stream, Task> callback, ServerSideEncryption sse = null, CancellationToken cancellationToken = default(CancellationToken))

Notice how callback is changed Action<Stream> callback -> Func<Stream, Task> callback.

For the time being, I came up with a workaround using TaskCompletionSource, although I cannot test if it is truly asynchronous or blocking.

                var completion = new TaskCompletionSource<object>();

                await _client.GetObjectAsync(_options.BucketName, name, async s =>
                {
                    await s.CopyToAsync(file.Stream, cancellationToken);

                    completion.TrySetResult(null);
                }, null, cancellationToken);

                await completion.Task;

[poornas]

Action callback is being written to by the http response writer without any intermediate buffering. Curious about why Action callback is a bottleneck for your application, it would be reasonable to expect that the stream would be processed right away and hasn't been reported as an issue by other sdk users thus far.

[mariusingjer]

If the callback method contains code that would benefit from coroutine support (like async waiting for a call to a database inside the callback).

Less bug prone: if you add async in front of the Action like in the authors example, your compiler won't complain and you can start adding awaits (like he did), but the code invoking the callback (restsharp i guess) will not respect your awaits and you can end up with a disposed stream before your callback completes.

So I see at least two good reasons for supporting this.

[DCNick3]

I did run into this problem. I want to stream GetObjectAsync result into asp.net core http response writer. It is possible to do it synchronously, but it is not recommended (need to enable it explicitly, see dotnet/aspnetcore#7644) and causes server thread pool starvation and server lockups (the server cannot handle other requests while copying data from GetObjectAsync to client). I see this as a major usability problem. Please introduce some way to use async callbacks.

[DCNick3]

Other issues regarding side-effects of using sync APIs in kestrel (asp.net core http server): aspnet/KestrelHttpServer#2104, dotnet/aspnetcore#17090

[cristipufu]

Same here:

Unsuccessful response from server without XML error: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.: Minio.Exceptions.InternalClientException: Minio API responded with message=Unsuccessful response from server without XML error: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.
   at void Minio.MinioClient.ParseError(IRestResponse response) in /q/.q/sources/minio-dotnet/Minio/MinioClient.cs:line 450
   at void Minio.MinioClient.HandleIfErrorResponse(IRestResponse response, IEnumerable<ApiResponseErrorHandlingDelegate> handlers, DateTime startTime) in /q/.q/sources/minio-dotnet/Minio/MinioClient.cs:line 505
   at async Task<IRestResponse> Minio.MinioClient.ExecuteTaskAsync(IEnumerable<ApiResponseErrorHandlingDelegate> errorHandlers, IRestRequest request, CancellationToken cancellationToken) in /q/.q/sources/minio-dotnet/Minio/MinioClient.cs:line 352
   at async Task Minio.MinioClient.GetObjectAsync(string bucketName, string objectName, Action<Stream> cb, CancellationToken cancellationToken) in /q/.q/sources/minio-dotnet/Minio/ApiEndpoints/ObjectOperations.cs:line 56
   at async Task UiPath.Storage.Minio.MinioStorageClient.ReadObjectAsync(StorageLocation location, Stream stream, CancellationToken cancellationToken)

As a workaround we have to enable AllowSynchronousIO in our web app:

services.Configure<IISServerOptions>(options =>
{
    options.AllowSynchronousIO = true;
});

This limitation comes from the underlying RestSharp client itself, I think it's time to get rid of it and use Microsoft's HttpClient

[harshavardhana]

Please upgrade to the latest release.

[cosminvlad]

@harshavardhana I see the API changed quite a bit in the latest release, but the callback is still an Action<Stream>, so we still have to enable synchronous IO. Am I missing something in the new API?
https://github.com/minio/minio-dotnet/blob/master/Minio/MinioClient.cs#L573

[Biztactix-Ryan]

@harshavardhana I see the API changed quite a bit in the latest release, but the callback is still an Action<Stream>, so we still have to enable synchronous IO. Am I missing something in the new API? https://github.com/minio/minio-dotnet/blob/master/Minio/MinioClient.cs#L573

I agree, Every thread discussing this has been closed saying upgrade to latest release, However it hasn't fixed the use case where we'd like to stream from Minio Directly to the client response, as the response has already closed by the time the callback starts coming back.

Can we please get an example of how the new API is supposed to fix this?

[edemartel]

Indeed, with how the API is currently written, it is impossible to correctly consume the response in a truly asynchronous manner. You can make your callback async void, but this is wrong, because it means the stream will almost surely be disposed while you're still processing it. There is no clean workaround here.

This is especially bad because it means one very common use case, which is to stream the response from Minio straight into the response to an ASP.NET Core request, is impossible without setting AllowSynchronousIO, which is discouraged.

The callback must be changed from Action<Stream> to Func<Stream, Task>, but that's probably a big breaking change.

[haiduong87]

Please upgrade to the latest release.

Can I turn off "AllowSynchronousIO" with latest release?

[Biztactix-Ryan]

@harshavardhana @edemartel @cosminvlad
There's more than one of us sitting here wondering if this is going to get sorted.

Many users will be using this in asp.net of some type, I can only expect that nearly all of them would want to pass data from Minio directly to the user, It would be very rare that we'd need to cache such data. so loading it into a memory stream then writing it to the response stream is double handling and depending on the size of the return may even overrun server memory.

Now if you have already covered this scenario and have an answer, it'd be great to know as I've just checked the docs again and it's still using the callback.
If we can't get such a simple thing sorted, We'll just move on and it'll be with another platform.

[Biztactix-Ryan]

Righto... Well I'm going to find other solutions - Trying this, https://github.com/jchristn/BlobHelper
Anyone else got anything sorted?

[mariusingjer]

Well, I don't have a solution per se, but I was using minio for the wrong reasons, since my storage was actually AWS S3, and I could utilize localstack for local S3 emulation. The aws sdk supports async, so my "solution" was that I didn't need minio for my use cases (sadly this doesn't help all who actually does :-)).

[neoxack]

Why is it closed if the functionality has not been implemented and is not even planned?

[ebozduman]

Looking into it...