Skip to content

feat: add redis and redis-cluster in limit-conn#10866

Merged
shreemaan-abhishek merged 43 commits into
apache:masterfrom
theweakgod:feat/add_redis_in_limit_conn
Feb 7, 2024
Merged

feat: add redis and redis-cluster in limit-conn#10866
shreemaan-abhishek merged 43 commits into
apache:masterfrom
theweakgod:feat/add_redis_in_limit_conn

Conversation

@theweakgod

@theweakgod theweakgod commented Jan 24, 2024

Copy link
Copy Markdown
Contributor

Description

Fixes #10501 (#10501)

Add options for redis and redis-cluster in the limit-conn plugin, and use redis as the counter for the leaky bucket algorithm.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@theweakgod
theweakgod marked this pull request as draft January 24, 2024 12:06
@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Let's bring support for limit-conn first then you can raise another PR for limit-req it's easier to review.

@theweakgod

Copy link
Copy Markdown
Contributor Author

Let's bring support for limit-conn first then you can raise another PR for limit-req it's easier to review.

I'll split it into two pull requests. I encountered an issue during development related to the management of Redis keys. I noticed that APISIX generates a significant number of Redis keys. I'm considering consolidating these keys into a hash for better management. What are your thoughts on this?

@theweakgod

Copy link
Copy Markdown
Contributor Author

@shreemaan-abhishek @monkeyDluffy6017 I have a problem now. I need to write a test case to test the plugin, but what address should I fill in for the redis cluster? How to fill in the cluster with password and the cluster without password? How to fill in the address of the redis singleton with password and without password?

@theweakgod theweakgod changed the title feat: add redis and redis-cluster in limit-conn and limit-req feat: add redis and redis-cluster in limit-conn Jan 25, 2024
@theweakgod
theweakgod marked this pull request as ready for review January 25, 2024 12:36
@theweakgod
theweakgod marked this pull request as draft January 25, 2024 12:37
@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

@theweakgod you will find redis servers (singleton and cluster) defined as docker compose applications in ci directory. You can use them or create new ones. You can also take inspiration from existing test cases for redis policy in limit count plugin.

@theweakgod

Copy link
Copy Markdown
Contributor Author

@theweakgod you will find redis servers (singleton and cluster) defined as docker compose applications in ci directory. You can use them or create new ones. You can also take inspiration from existing test cases for redis policy in limit count plugin.

Ok.I see, thank u.

@theweakgod
theweakgod marked this pull request as ready for review January 26, 2024 09:54
@theweakgod

Copy link
Copy Markdown
Contributor Author

@shreemaan-abhishek @monkeyDluffy6017 I'm ready to review

Comment thread t/plugin/limit-conn-redis.t Outdated
Comment thread t/plugin/limit-conn-redis.t Outdated
Comment thread t/plugin/limit-conn-redis.t
Comment thread t/plugin/limit-conn-redis.t
Comment thread apisix/plugins/limit-conn/util.lua Outdated
Comment thread apisix/plugins/limit-conn/util.lua Outdated
Comment thread docs/en/latest/plugins/limit-conn.md Outdated
Comment thread docs/zh/latest/plugins/limit-conn.md Outdated
@kayx23

kayx23 commented Feb 2, 2024

Copy link
Copy Markdown
Member

@shreemaan-abhishek I checked the doc and no more comments from me.

Comment thread apisix/utils/redis.lua Outdated
Comment thread apisix/cli/ngx_tpl.lua
Comment thread apisix/utils/redis.lua
Comment thread apisix/plugins/utils/redis-schema.lua
@theweakgod
theweakgod force-pushed the feat/add_redis_in_limit_conn branch from 8c142f1 to f375fca Compare February 5, 2024 07:10
@shreemaan-abhishek
shreemaan-abhishek merged commit 7e907a5 into apache:master Feb 7, 2024
illidan33 pushed a commit to illidan33/apisix that referenced this pull request Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

feat: centralized counter for limit conn and limit req

4 participants