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

Forget to call Sendcallback on a branch of closeAsync #11936

Closed
shoothzj opened this issue Sep 7, 2021 · 4 comments · Fixed by #11939
Closed

Forget to call Sendcallback on a branch of closeAsync #11936

shoothzj opened this issue Sep 7, 2021 · 4 comments · Fixed by #11939
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@shoothzj
Copy link
Member

shoothzj commented Sep 7, 2021

In #11906 , it fixed the update of memory usage, but still one branch of closeAsync doesn't call callback too.

But this seems hard to write an unit test. This needs during the producer close, the broker is ready, and there are still remaining pending messages.

Any idea for how to add tests?

@codelipenghui @merlimat @michaeljmarshall

@codelipenghui
Copy link
Contributor

@shoothzj Maybe we can disable the channel auto_read at the broker side for simulating the messages pending in the queue.

@shoothzj
Copy link
Member Author

shoothzj commented Sep 7, 2021

@shoothzj Maybe we can disable the channel auto_read at the broker side for simulating the messages pending in the queue.

But we still need a close success resp from broker side. How ?

@codelipenghui
Copy link
Contributor

@shoothzj You can create a method for handling the close producer response, then you can call it directly. If the code is untestable, we should make it testable.

@shoothzj
Copy link
Member Author

shoothzj commented Sep 7, 2021

@shoothzj You can create a method for handling the close producer response, then you can call it directly. If the code is untestable, we should make it testable.

Thanks a lot for your help, I found the way to add test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants