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

add user-client mapping. #405

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from

Conversation

ztxiuyuan
Copy link

As you can see in redis/redis#12245 ,user-client mapping has been added to the server to facilitate statistics on the usage of each user connection.

client list user return specified user connection info

127.0.0.1:6379> CLIENT list user doug
id=5 addr=127.0.0.1:59469 laddr=127.0.0.1:6379 fd=9 name= age=33 idle=28 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=0 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1824 events=r cmd=auth user=doug redir=-1 resp=2 lib-name= lib-ver=
id=6 addr=127.0.0.1:59490 laddr=127.0.0.1:6379 fd=10 name= age=19 idle=15 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=0 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1824 events=r cmd=auth user=doug redir=-1 resp=2 lib-name= lib-ver=

client count return specified user connetcion count

127.0.0.1:6379> CLIENT COUNT doug
"2"

Copy link

codecov bot commented Apr 30, 2024

Codecov Report

Attention: Patch coverage is 50.63291% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 68.50%. Comparing base (93f8a19) to head (689db89).
Report is 8 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #405      +/-   ##
============================================
+ Coverage     68.43%   68.50%   +0.06%     
============================================
  Files           109      109              
  Lines         61681    61760      +79     
============================================
+ Hits          42214    42309      +95     
+ Misses        19467    19451      -16     
Files Coverage Δ
src/acl.c 89.02% <100.00%> (+0.04%) ⬆️
src/commands.def 100.00% <ø> (ø)
src/networking.c 83.84% <46.57%> (-1.24%) ⬇️

... and 10 files with indirect coverage changes

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Hi! Thanks!

I'm just looking at the commands and arguments now, not the code yet. This PR adds two different things.

  • CLIENT LIST: Adding a filter argument USER username is a good idea. But ID id-1 id-2 ... must be the last argument because it accepts multiple arguments.

  • CLIENT COUNT: It is too specific to only user names. It needs to be able to use the same filters as CLIENT LIST, for example

    CLIENT COUNT TYPE normal USER doug ID 5 7 8 9
    

    The return value should be an integer, not a string.

  • CLIENT KILL has the same arguments as CLIENT LIST. It needs to support USER doug too.

Please read this proposal: redis/redis#12311 -- we don't need to implement all of it but we should follow the logic explained there.

Signed-off-by: 邠风 <zhaoteng.zhao@alibaba-inc.com>
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.

None yet

2 participants