Skip to content
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.

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

ListenBucketNotificationsAsync usage #649

Closed
DominikHerold opened this issue Jun 20, 2022 · 10 comments
Closed

ListenBucketNotificationsAsync usage #649

DominikHerold opened this issue Jun 20, 2022 · 10 comments
Assignees

Comments

@DominikHerold
Copy link

Hi,

I'm running locally a MinIO instance and implemented the ListenBucketNotifications like it is described here
I'm uploading a file via the MinIO console but the onNext Action does not kick in due to onCompleted is already executed.
Actually ListenBucketNotificationsAsync is intended to get informed in case EventType is EventType.ObjectCreatedAll, isn't it?
Did I missed something or is there another way to get informed when a file is uploaded?
Thanks in advance.

@DominikHerold
Copy link
Author

Interestingly when enabling trace logs via SetTraceOn() in the minioClient everything works as expected and I get notified. With the trade off having a lot of logs.

@ebozduman
Copy link
Collaborator

Looks like some timing issue. Working on it

@ebozduman
Copy link
Collaborator

@DominikHerold

I cannot reproduce the issue here with my local setup.
Could you please attach here the code you've seen the problem with?

@ebozduman
Copy link
Collaborator

I reproduced the issue.
It turns out that this is just a simple timing issue as anticipated. So, subscriber needs some time to create notification for the event to be captured.
That is, if you add some sleep time after the event happens, then it will be captured and reported as expected.

Example:

using System.Threading;
                :
                :
                await _minio.PutObjectAsync(putObjectArgs);
                Console.WriteLine("Successfully uploaded");
                Thread.Sleep(350);

Please give it a try and let us know. You may close the issue if it works.

@ebozduman
Copy link
Collaborator

@DominikHerold

Did you have a chance to check it out?

@ebozduman
Copy link
Collaborator

Closing...
Everything works as expected.

@codemnk
Copy link

codemnk commented Jul 6, 2022

Having same issue. onCompleted is executed almost immediately after observable.Subscribe.
I am pretty sure the issue is in using of async void lambda in

requestMessageBuilder.ResponseWriter = async responseStream =>

ResponseWriter is Action<Stream>.
Basically response is disposed before stream is processed since calling code does not "await" on async void lambda.
https://devblogs.microsoft.com/pfxteam/potential-pitfalls-to-avoid-when-passing-around-async-lambdas/
https://www.jetbrains.com/help/resharper/AsyncVoidLambda.html

@ebozduman
Copy link
Collaborator

Thank you @codemnk
I'll check it out and fix the issue accordingly.

@ebozduman
Copy link
Collaborator

@codemnk ,

To investigate the void async lambda function issue in our code and a possible for it, I first created a new wrapper function that encapsulates the response writer and calls and waits on it with .Wait(),

public void ProcessFunctionResponseWriter(Func<Stream, Task> func, Stream stream)
{
    func(stream).Wait();
}

and added a new response writer, FunctionResponseWriter, without touching the existing definition ResponseWriter, so that the existing code is not touched, and hence any chance to cause regression problems are eliminated.

public Func<Stream, Task> FunctionResponseWriter { get; set; }

Then I called ProcessFunctionResponseWriter method exactly the same way it is done for ResponseWriter.

if (requestMessageBuilder.FunctionResponseWriter != null)
{
    requestMessageBuilder.ProcessFunctionResponseWriter(requestMessageBuilder.FunctionResponseWriter, responseResult.ContentStream);
}

However, when I run both old and the new code, I don’t see any behavior differences and when the execution stops prematurely.

So, I think the problem is not the void async lambda. The subscriber’s OnNext action is interrupted and the interrupt comes from the observable created by Observable.Create.

After our team discussed the issue, the decision is that waiting for the subscriber rx action is the easiest solution or the workaround at this point.
This is just an example how to workaround the issue almost always:

subscription = observable.Subscribe(
    ev => 
    {
        Thread thread = new Thread( () => Notify(ev));
        thread.Start();
},
:
:

I’ll still be looking into this part of the code and logic to see what can be done.

Please don't hesitate to discuss. You are more than welcome to give us your opinion.

@ebozduman ebozduman reopened this Jul 20, 2022
@ebozduman
Copy link
Collaborator

ebozduman commented Jul 20, 2022

Thanks to @codemnk for providing a new scenario where void as lambda function may cause an issue for bucket notifications and for all his guidance, references, tips and the fix suggestion.
So, ,reopened this issue to refer it in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants