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

refactor(limiter): refactor and simplify the limiter #14475

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lafirest
Copy link
Member

@lafirest lafirest commented Dec 27, 2024

Fixes EMQX-13124

Release version: v/e5.9.0

Summary

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • Added property-based tests for code which performs user input validation
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
  • Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

  • If changed package build workflow, pass this action (manual trigger)
  • Change log has been added to changes/ dir for user-facing artifacts update

@lafirest lafirest force-pushed the refactor/limiter branch 8 times, most recently from 6afd0a9 to f0c46c3 Compare January 2, 2025 11:17
@lafirest lafirest marked this pull request as ready for review January 2, 2025 13:54
@lafirest lafirest requested a review from a team as a code owner January 2, 2025 13:54
apps/emqx/src/emqx_connection.erl Outdated Show resolved Hide resolved
apps/emqx/src/emqx_limiter/src/emqx_limiter.erl Outdated Show resolved Hide resolved
Comment on lines +51 to +55
-callback check(non_neg_integer(), limiter()) -> {boolean(), limiter()}.
-callback restore(non_neg_integer(), limiter()) -> limiter().
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe take_out and pub_back ?

apps/emqx/src/emqx_limiter/src/emqx_limiter.erl Outdated Show resolved Hide resolved
apps/emqx/src/emqx_limiter/src/emqx_limiter.erl Outdated Show resolved Hide resolved
apps/emqx/src/emqx_limiter/src/emqx_limiter_allocator.erl Outdated Show resolved Hide resolved
apps/emqx/src/emqx_limiter/src/emqx_limiter_allocator.erl Outdated Show resolved Hide resolved
bucket_ref()
) -> boolean().
insert_bucket(Id, Type, Bucket) ->
insert_bucket(Zone, Name, Bucket) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

store in persistent term for hot config update.

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