Skip to content

fix event lost - #1083

Open
ningmingxiao wants to merge 1 commit into
fsouza:mainfrom
ningmingxiao:print_event_lost
Open

fix event lost#1083
ningmingxiao wants to merge 1 commit into
fsouza:mainfrom
ningmingxiao:print_event_lost

Conversation

@ningmingxiao

@ningmingxiao ningmingxiao commented Aug 20, 2024

Copy link
Copy Markdown
Contributor

sometimes if listener chan is full,event will lost.
@fsouza @bufdev
can you review my pr? thank you

@ningmingxiao
ningmingxiao force-pushed the print_event_lost branch 3 times, most recently from b932fb8 to e2dd0d9 Compare August 20, 2024 09:10
@ningmingxiao ningmingxiao changed the title add some log print event lost fix event lost Aug 20, 2024
@bufdev

bufdev commented Aug 28, 2024

Copy link
Copy Markdown
Contributor

@ningmingxiao I have not been involved in this repo for about a decade, it's not really professional to tag people here. Will leave this to others.

@ningmingxiao

Copy link
Copy Markdown
Contributor Author

sory,but thank you anyway.

@fsouza

fsouza commented Sep 22, 2024

Copy link
Copy Markdown
Owner

Thanks for contributing! This is a big change that can lead to slow consumers hanging. I recommend either making sure your caller is always fast enough to consume the events, or putting the new behavior behind a knob.

@ningmingxiao

ningmingxiao commented Sep 23, 2024

Copy link
Copy Markdown
Contributor Author

sometimes I can't make sure consumers run faster, if run plenty of containers, will generate many events, channel will be filled. At least we should let user know event is dropped when chan is filled.

@fsouza

fsouza commented Oct 28, 2024

Copy link
Copy Markdown
Owner

Can you add a knob in that case?

@ningmingxiao

Copy link
Copy Markdown
Contributor Author

Can you add a knob in that case?

Could you describe it in more detail? Thank you

@fsouza

fsouza commented Oct 29, 2024

Copy link
Copy Markdown
Owner

@ningmingxiao I imagine we'd have a new option when adding the listener. Internally the listener would become some struct with the channel + some listening options, and externally we'd probably need to have versions of AddEventListenerWithOptions and RemoveEventListener that take a struct with "listening options" instead of just the channel.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants