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.

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

Jump to bottom

feat(banned): filter out banned messages from mqueue #9893

Merged
merged 2 commits into from Mar 13, 2023
Merged

feat(banned): filter out banned messages from mqueue #9893

merged 2 commits into from Mar 13, 2023

Conversation

lafirest
Copy link
Member

@lafirest lafirest commented Feb 2, 2023 •
edited by id

@lafirest lafirest marked this pull request as ready for review February 2, 2023 14:49
@lafirest lafirest requested review from a team and thalesmg as code owners February 2, 2023 14:49
@qzhuyan
Copy link
Contributor

qzhuyan commented Feb 14, 2023

if the message is in the mqueue, that means the messages weren't banned while routing/forwarding? The ban of the client could be issued afterward?

If so, I am not sure this is a general solution for all.
Is this behavior approved by the product?

@lafirest
Copy link
Member Author

This PR is from a discussion, and I'm not sure about this modification either.
Now, when a client was banned, we will kick it out but won't clear its messages or other data and also won't check if the client was banned when delivering(routing/forwarding) a message.

@id
Copy link
Collaborator

id commented Mar 10, 2023

@lafirest @qzhuyan confirmed with Ben and Stone, we can merge this.
@qzhuyan do you think it may lead to performance issues?

@lafirest lafirest merged commit e8cf604 into emqx:master Mar 13, 2023
@lafirest lafirest deleted the feat/filiter_banned branch March 13, 2023 01:58
@zmstone zmstone mentioned this pull request Mar 17, 2023
Merged
@Porkepix Porkepix mentioned this pull request Mar 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qzhuyan qzhuyan qzhuyan approved these changes

@thalesmg thalesmg Awaiting requested review from thalesmg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lafirest @qzhuyan @id