Adding Missing filters to CLIENT LIST and Dedup Parsing #1401

sarthakaggarwal97 · 2024-12-06T16:25:59Z

Adds filter options to CLIENT LIST:

* USER <username>
  Return clients authenticated by <username>.
* ADDR <ip:port>
  Return clients connected from the specified address.
* LADDR <ip:port>
  Return clients connected to the specified local address.
* SKIPME (YES|NO)
  Exclude the current client from the list (default: no).
* MAXAGE <maxage>
  Only list connections older than the specified age.

Modifies the ID filter to CLIENT KILL to allow multiple IDs

* ID <client-id> [<client-id>...]
  Kill connections by client ids.

This makes CLIENT LIST and CLIENT KILL accept the same options.

For backward compatibility, the default value for SKIPME is NO for CLIENT LIST and YES for CLIENT KILL.

The MAXAGE comes from CLIENT KILL, where it keeps clients with the given max age and kills the older ones. This logic becomes weird for CLIENT LIST, but is kept for similary with CLIENT KILL, for the use case of first testing manually using CLIENT LIST, and then running CLIENT KILL with the same filters.

The ID client-id [client-id ...] no longer needs to be the last filter. The parsing logic determines if an argument is an ID or not based on whether it can be parsed as an integer or not.

Partly addresses: #668

codecov · 2024-12-06T16:41:33Z

Codecov Report

Attention: Patch coverage is 91.83673% with 28 lines in your changes missing coverage. Please review.

Project coverage is 70.93%. Comparing base (d13aad4) to head (110c7d3).
Report is 5 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/networking.c	91.83%	28 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1401      +/-   ##
============================================
- Coverage     70.94%   70.93%   -0.01%     
============================================
  Files           120      120              
  Lines         65044    65078      +34     
============================================
+ Hits          46143    46161      +18     
- Misses        18901    18917      +16

Files with missing lines	Coverage Δ
src/commands.def	`100.00% <ø> (ø)`
src/server.h	`100.00% <ø> (ø)`
src/networking.c	`88.76% <91.83%> (+0.28%)`	⬆️

... and 14 files with indirect coverage changes

zuiderkwast

Thanks.

I just took a quick look and I can see some things that appear to be broken:

CLIENT LIST with multiple IDs, for example CLIENT LIST ID 3 4 5. (It means id is 3 or 4 or 5.) We a test case for this.
The default for "skipme" for CLIENT KILL should be YES by default (when filters are used), but for CLIENT LIST, skipme should be NO by default. Otherwise, we have a breaking change. Please add some tests without SKIPME but with other filters matching the calling client to cover the default skipme value for LIST and KILL.

We need some test cases combining multiple filter rules at the same time.

src/server.h

src/networking.c

sarthakaggarwal97 · 2024-12-09T17:29:41Z

Thanks @zuiderkwast for the comments! I am working on them

src/networking.c

sarthakaggarwal97 · 2024-12-11T09:28:12Z

@zuiderkwast thank you for your comments. Adressed most of them. Working on adding tests meanwhile. Trying to see how can I have tests with multiple clients.

Sharing some responses from local testing

Client Kill Command

127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=11 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=53 tot-net-out=209291 tot-cmds=1
id=4 addr=127.0.0.1:63141 laddr=127.0.0.1:6379 fd=9 name= age=8 idle=8 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
id=5 addr=127.0.0.1:63142 laddr=127.0.0.1:6379 fd=10 name= age=5 idle=5 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client info
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=16 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|info user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=79 tot-net-out=210285 tot-cmds=2
127.0.0.1:6379> client kill id 4
(integer) 1
127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=39 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=146 tot-net-out=210626 tot-cmds=4
id=5 addr=127.0.0.1:63142 laddr=127.0.0.1:6379 fd=10 name= age=33 idle=33 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=50 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=172 tot-net-out=211295 tot-cmds=5
id=5 addr=127.0.0.1:63142 laddr=127.0.0.1:6379 fd=10 name= age=44 idle=44 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
id=7 addr=127.0.0.1:63168 laddr=127.0.0.1:6379 fd=9 name= age=3 idle=3 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client kill id 5 7 
(integer) 2
127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=72 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=246 tot-net-out=212296 tot-cmds=7
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=7 idle=7 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
id=9 addr=127.0.0.1:63184 laddr=127.0.0.1:6379 fd=10 name= age=4 idle=4 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client kill id 3 9 addr 127.0.0.1:63183
(integer) 0
127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=97 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=352 tot-net-out=213295 tot-cmds=9
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=32 idle=32 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
id=9 addr=127.0.0.1:63184 laddr=127.0.0.1:6379 fd=10 name= age=29 idle=29 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client kill id 3 9 addr 127.0.0.1:63184
(integer) 1
127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=107 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=458 tot-net-out=214298 tot-cmds=11
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=42 idle=42 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1

Client List Command

127.0.0.1:6379> client list
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=1361 idle=8 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=1920 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=484 tot-net-out=215973 tot-cmds=13
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=1296 idle=1296 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
id=10 addr=127.0.0.1:55765 laddr=127.0.0.1:6379 fd=10 name= age=10 idle=0 flags=N db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=10 multi-mem=0 rbs=1024 rbp=0 obl=0 oll=0 omem=0 tot-mem=1946 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=53 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client list id 3 8
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=1368 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=1920 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=484 tot-net-out=215973 tot-cmds=13
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=1303 idle=1303 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client list id 3 8 age 1300
ERR syntax error
127.0.0.1:6379> client list id 3 8 maxage 1300
id=3 addr=127.0.0.1:63139 laddr=127.0.0.1:6379 fd=8 name= age=1387 idle=34 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=1920 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=484 tot-net-out=215973 tot-cmds=13
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=1322 idle=1322 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1
127.0.0.1:6379> client list id 3 8 maxage 1500
127.0.0.1:6379> client list id 3 8 addr 127.0.0.1:63183
id=8 addr=127.0.0.1:63183 laddr=127.0.0.1:6379 fd=9 name= age=1349 idle=1349 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=1920 events=r cmd=command|docs user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=27 tot-net-out=209291 tot-cmds=1

zuiderkwast

Trying to see how can I have tests with multiple clients.

You can create multiple clients.

        set c1 [valkey_client]
        set c2 [valkey_client]
        set c3 [valkey_client]
        ...
        $c1 close
        $c2 close
        $c3 close

src/networking.c

zuiderkwast

The refactoring looks good. I have just a few small comments and there is some merge conflict.

src/networking.c

zuiderkwast

LGTM. Just two nits.

src/networking.c

zuiderkwast · 2024-12-20T18:10:07Z

@valkey-io/core-team Please approve the extended syntax for CLIENT LIST as described in the top comment.

The MAXAGE argument for CLIENT LIST is a little strange, because it's the max age of clients to exclude, for symmetry with CLIENT KILL. An alternative is to forbid MAXAGE for CLIENT LIST.

src/networking.c

Signed-off-by: Sarthak Aggarwal <[email protected]>

madolson · 2025-01-13T16:23:21Z

Core team meeting. Can we run a quick performance test to validate that it's not significantly regressing. Let's run valkey-benchmark with 10 and 10000 clients connected, and see how much more time it takes to return. Also include cmdstats so that we can compare IO vs server side compute time.

hpatro · 2025-01-13T20:27:44Z

Core team meeting. Can we run a quick performance test to validate that it's not significantly regressing. Let's run valkey-benchmark with 10 and 10000 clients connected, and see how much more time it takes to return. Also include cmdstats so that we can compare IO vs server side compute time.

Current suggestion:

Write a small script to generate idle ‘N’ connections or use the benchmark to generate active clients via valkey-benchmark -l -c 10 -t PING
Use the benchmark on separate terminal to gauge the performance of CLIENT LIST (possibly with filters) command. valkey-benchmark -n 100000 -c 1 CLIENT LIST

Let’s capture data with modifying client parameter for both steps.

If there are any other suggestion @hwware / @zuiderkwast, feel free to add. We'll capture that.

zuiderkwast · 2025-01-13T20:29:02Z

Can we run a quick performance test to validate that it's not significantly regressing. Let's run valkey-benchmark with 10 and 10000 clients connected, and see how much more time it takes to return. Also include cmdstats so that we can compare IO vs server side compute time.

For the benchmark, we should run CLIENT LISTS with some filter, right? CLIENT LIST TYPE NORMAL?

hwware · 2025-01-13T21:25:39Z

Core team meeting. Can we run a quick performance test to validate that it's not significantly regressing. Let's run valkey-benchmark with 10 and 10000 clients connected, and see how much more time it takes to return. Also include cmdstats so that we can compare IO vs server side compute time.

Current suggestion:

Write a small script to generate idle ‘N’ connections or use the benchmark to generate active clients via valkey-benchmark -l -c 10 -t PING

Use the benchmark on separate terminal to gauge the performance of CLIENT LIST (possibly with filters) command. valkey-benchmark -n 100000 -c 1 CLIENT LIST

Let’s capture data with modifying client parameter for both steps.

If there are any other suggestion @hwware / @zuiderkwast, feel free to add. We'll capture that.

I would like to suggest as following:

Run valkey-benchmark as 10000 client read/write request (datasize could be 256 bytes or several kb)
Run valkey-benchmark as 10 client with CLIENT List "filter", the filter can be some combinations as the following:
- USER
  Return clients authenticated by .
- ADDR ip:port
  Return clients connected from the specified address.
- LADDR ip:port
  Return clients connected to the specified local address.
- MAXAGE
  Only list connections older than the specified age.

sarthakaggarwal97 · 2025-01-13T21:30:06Z

@hwware @zuiderkwast @hpatro Thank you for the suggestions, really helpful.

I had just one query, since some of the filters were not supported in Client List originally, how would we compare the performance through benchmark? Or do we want to compare the performance with just Client List?
We can definitely compare with id/type filters which were supported before.

sarthakaggarwal97 · 2025-01-14T02:55:26Z

Hi folks, sharing some data around the performance. The results do not look very deviated and are possibly within margin. I have tried my best to keep the setup as similar as possible. I was not able to get the data for 10000 clients, as both of my machines kinda couldn't process it (on both branches). If required, I can try it on a beefier machine and get back.

sarthakaggarwal97 · 2025-01-14T06:15:58Z

I was able to extract the numbers for 10000 clients as well. Seeing negligible variance across the latency / throughput parameters for with and without filters.

Sharing Command Info Stats for 10000 Clients. Please review.

Unstable:

Command: `client list`

cmdstat_client|list:calls=100000,usec=1828098028,usec_per_call=18280.98,rejected_calls=0,failed_calls=0

Command: `client list type normal`

cmdstat_client|list:calls=100000,usec=1746052452,usec_per_call=17460.53,rejected_calls=0,failed_calls=0

PR:

Command: `client list`

cmdstat_client|list:calls=100000,usec=1736309756,usec_per_call=17363.10,rejected_calls=0,failed_calls=0

Command: `client list type normal`

cmdstat_client|list:calls=100000,usec=1835707578,usec_per_call=18357.08,rejected_calls=0,failed_calls=0

Notes:

Setup:

Machine: c5.12xlarge
Running: valkey-benchmark -l -c 10 -t PING
Running benchmarks for client list (with and without filters)
Resetting stats using config resetstat after every run.

hwware · 2025-01-14T18:59:01Z

@hwware @zuiderkwast @hpatro Thank you for the suggestions, really helpful.

I had just one query, since some of the filters were not supported in Client List originally, how would we compare the performance through benchmark? Or do we want to compare the performance with just Client List? We can definitely compare with id/type filters which were supported before.

My idea is sending read/write operations through 10000 or more clients, and at the same time, simulating operator runs CLIENT LIST filter in another valkey-bench, and you can watch if there are qps drop in the read/write side.

Because the client list with filter will execute a slow operation, the server qps could drop. If there is no significant drop, we can merge this PR, thanks

zuiderkwast · 2025-01-14T21:05:08Z

I can see in the benchmarks that this PR actually makes CLIENT LIST faster than before with 1000 clients connected. It seems that there is some initial cost to set up the filter struct (so slightly dropped latency when there are very few clients connected) but once the filter struct is set up, it is faster to filter each client.

Because of this, I would assume that QPS for the read/write clients will be affected less by CLIENT LIST with this PR compared to unstable. Testing is of course better, but it looks pretty safe to me.

sarthakaggarwal97 · 2025-01-14T23:12:17Z

Thanks folks for the suggestions and inputs.

Based on @hwware suggestions, I performed the SET/GET operations while running client list <filter> <filter-value> for 10000 clients.
I do not see any major deviation in performance from the current unstable command.

Sharing the results:

Notes:

Latency Unit: msec
RPS: requests per second

hpatro · 2025-01-14T23:19:06Z

@sarthakaggarwal97 Thanks for sharing the results and is inline with our expectation. I'm good with the stats. The above benchmarking is performed with one client running the CLIENT LIST (filters) and 10k clients running SET/GET operations. The setup is quite similar to a real world scenario where the no. of application clients >>> no. of administrator clients.

hwware · 2025-01-15T16:09:26Z

It looks good to me. Approved

hpatro · 2025-01-15T18:33:11Z

I see atleast 4 core-team member have approved this either through voting or PR approval. @valkey-io/core-team Do you want to review further or is it good to be merged?

zuiderkwast · 2025-01-15T19:43:17Z

Let's merge it.

@sarthakaggarwal97 do you want to update the docs too? The commands/*.md files in the valkey-doc repo. We can have one doc PR for all these updates if you want.

sarthakaggarwal97 · 2025-01-15T20:00:03Z

Thank you everyone for helping in this one.

@zuiderkwast let me raise the doc PR as well. Thank you!

Adds filter options to CLIENT LIST: * USER <username> Return clients authenticated by <username>. * ADDR <ip:port> Return clients connected from the specified address. * LADDR <ip:port> Return clients connected to the specified local address. * SKIPME (YES|NO) Exclude the current client from the list (default: no). * MAXAGE <maxage> Only list connections older than the specified age. Modifies the ID filter to CLIENT KILL to allow multiple IDs * ID <client-id> [<client-id>...] Kill connections by client ids. This makes CLIENT LIST and CLIENT KILL accept the same options. For backward compatibility, the default value for SKIPME is NO for CLIENT LIST and YES for CLIENT KILL. The MAXAGE comes from CLIENT KILL, where it *keeps* clients with the given max age and kills the older ones. This logic becomes weird for CLIENT LIST, but is kept for similary with CLIENT KILL, for the use case of first testing manually using CLIENT LIST, and then running CLIENT KILL with the same filters. The `ID client-id [client-id ...]` no longer needs to be the last filter. The parsing logic determines if an argument is an ID or not based on whether it can be parsed as an integer or not. Partly addresses: valkey-io#668 --------- Signed-off-by: Sarthak Aggarwal <[email protected]> Signed-off-by: proost <[email protected]>

Documentation for valkey-io/valkey#1401 --------- Signed-off-by: Sarthak Aggarwal <[email protected]>

Adds filter options to CLIENT LIST: * USER <username> Return clients authenticated by <username>. * ADDR <ip:port> Return clients connected from the specified address. * LADDR <ip:port> Return clients connected to the specified local address. * SKIPME (YES|NO) Exclude the current client from the list (default: no). * MAXAGE <maxage> Only list connections older than the specified age. Modifies the ID filter to CLIENT KILL to allow multiple IDs * ID <client-id> [<client-id>...] Kill connections by client ids. This makes CLIENT LIST and CLIENT KILL accept the same options. For backward compatibility, the default value for SKIPME is NO for CLIENT LIST and YES for CLIENT KILL. The MAXAGE comes from CLIENT KILL, where it *keeps* clients with the given max age and kills the older ones. This logic becomes weird for CLIENT LIST, but is kept for similary with CLIENT KILL, for the use case of first testing manually using CLIENT LIST, and then running CLIENT KILL with the same filters. The `ID client-id [client-id ...]` no longer needs to be the last filter. The parsing logic determines if an argument is an ID or not based on whether it can be parsed as an integer or not. Partly addresses: valkey-io#668 --------- Signed-off-by: Sarthak Aggarwal <[email protected]>

sarthakaggarwal97 force-pushed the client-command-revamp branch from 54adfa6 to 7630eba Compare December 9, 2024 00:58

sarthakaggarwal97 marked this pull request as ready for review December 9, 2024 00:59

sarthakaggarwal97 requested review from hpatro and zuiderkwast December 9, 2024 00:59

zuiderkwast reviewed Dec 9, 2024

View reviewed changes

src/server.h Outdated Show resolved Hide resolved

src/networking.c Outdated Show resolved Hide resolved

src/networking.c Outdated Show resolved Hide resolved

sarthakaggarwal97 mentioned this pull request Dec 10, 2024

Extended Valkey client introspection functionality #668

Open

zuiderkwast reviewed Dec 10, 2024

View reviewed changes

src/networking.c Outdated Show resolved Hide resolved

src/networking.c Outdated Show resolved Hide resolved

sarthakaggarwal97 force-pushed the client-command-revamp branch 2 times, most recently from 049f2d4 to e3c1c5a Compare December 11, 2024 09:23

zuiderkwast reviewed Dec 11, 2024

View reviewed changes

sarthakaggarwal97 force-pushed the client-command-revamp branch 3 times, most recently from fd743fe to 6681e47 Compare December 16, 2024 21:00

zuiderkwast reviewed Dec 16, 2024

View reviewed changes

src/networking.c Outdated Show resolved Hide resolved

src/networking.c Outdated Show resolved Hide resolved

src/networking.c Outdated Show resolved Hide resolved

sarthakaggarwal97 force-pushed the client-command-revamp branch from 6681e47 to 52fd611 Compare December 16, 2024 22:41

sarthakaggarwal97 mentioned this pull request Dec 20, 2024

[Client Introspection] Client Commands Extended Filtering #1466

Open

sarthakaggarwal97 force-pushed the client-command-revamp branch from 52fd611 to ac3f5e5 Compare December 20, 2024 17:16

zuiderkwast reviewed Dec 20, 2024

View reviewed changes

src/networking.c Outdated Show resolved Hide resolved

src/networking.c Outdated Show resolved Hide resolved

zuiderkwast added release-notes This issue should get a line item in the release notes major-decision-pending Major decision pending by TSC team needs-doc-pr This change needs to update a documentation page. Remove label once doc PR is open. labels Dec 20, 2024

zuiderkwast reviewed Dec 20, 2024

View reviewed changes

src/networking.c Outdated Show resolved Hide resolved

hpatro reviewed Dec 20, 2024

View reviewed changes

sarthakaggarwal97 force-pushed the client-command-revamp branch from 3b789c2 to a944e0e Compare December 20, 2024 19:26

sarthakaggarwal97 mentioned this pull request Dec 20, 2024

[Client Introspection] Client Count Command #1467

Open

sarthakaggarwal97 force-pushed the client-command-revamp branch from 2120e61 to 301993f Compare December 20, 2024 23:43

refactoring client subcommand function names

110c7d3

Signed-off-by: Sarthak Aggarwal <[email protected]>

sarthakaggarwal97 force-pushed the client-command-revamp branch from 3f171c8 to 110c7d3 Compare January 13, 2025 15:30

madolson self-requested a review January 13, 2025 16:31

madolson added major-decision-approved Major decision approved by TSC team major-decision-pending Major decision pending by TSC team and removed major-decision-pending Major decision pending by TSC team major-decision-approved Major decision approved by TSC team labels Jan 13, 2025

hwware approved these changes Jan 15, 2025

View reviewed changes

zuiderkwast added major-decision-approved Major decision approved by TSC team and removed major-decision-pending Major decision pending by TSC team labels Jan 15, 2025

zuiderkwast merged commit 6a8f068 into valkey-io:unstable Jan 15, 2025
51 checks passed

sarthakaggarwal97 mentioned this pull request Jan 16, 2025

Updated Documentation for CLIENT KILL and CLIENT LIST valkey-io/valkey-doc#218

Merged

zuiderkwast pushed a commit to valkey-io/valkey-doc that referenced this pull request Jan 17, 2025

Updated Documentation for CLIENT KILL and CLIENT LIST (#218)

af26ec2

Documentation for valkey-io/valkey#1401 --------- Signed-off-by: Sarthak Aggarwal <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Missing filters to CLIENT LIST and Dedup Parsing #1401

Adding Missing filters to CLIENT LIST and Dedup Parsing #1401

sarthakaggarwal97 commented Dec 6, 2024 •

edited by zuiderkwast

Loading

codecov bot commented Dec 6, 2024 •

edited

Loading

zuiderkwast left a comment

sarthakaggarwal97 commented Dec 9, 2024

sarthakaggarwal97 commented Dec 11, 2024 •

edited

Loading

zuiderkwast left a comment

zuiderkwast left a comment

zuiderkwast left a comment •

edited

Loading

zuiderkwast commented Dec 20, 2024

madolson commented Jan 13, 2025

hpatro commented Jan 13, 2025

zuiderkwast commented Jan 13, 2025

hwware commented Jan 13, 2025 •

edited

Loading

sarthakaggarwal97 commented Jan 13, 2025

sarthakaggarwal97 commented Jan 14, 2025

sarthakaggarwal97 commented Jan 14, 2025 •

edited

Loading

hwware commented Jan 14, 2025 •

edited

Loading

zuiderkwast commented Jan 14, 2025

sarthakaggarwal97 commented Jan 14, 2025 •

edited

Loading

hpatro commented Jan 14, 2025

hwware commented Jan 15, 2025

hpatro commented Jan 15, 2025

zuiderkwast commented Jan 15, 2025

sarthakaggarwal97 commented Jan 15, 2025

Adding Missing filters to CLIENT LIST and Dedup Parsing #1401

Adding Missing filters to CLIENT LIST and Dedup Parsing #1401

Conversation

sarthakaggarwal97 commented Dec 6, 2024 • edited by zuiderkwast Loading

codecov bot commented Dec 6, 2024 • edited Loading

Codecov Report

zuiderkwast left a comment

Choose a reason for hiding this comment

sarthakaggarwal97 commented Dec 9, 2024

sarthakaggarwal97 commented Dec 11, 2024 • edited Loading

zuiderkwast left a comment

Choose a reason for hiding this comment

zuiderkwast left a comment

Choose a reason for hiding this comment

zuiderkwast left a comment • edited Loading

Choose a reason for hiding this comment

zuiderkwast commented Dec 20, 2024

madolson commented Jan 13, 2025

hpatro commented Jan 13, 2025

zuiderkwast commented Jan 13, 2025

hwware commented Jan 13, 2025 • edited Loading

sarthakaggarwal97 commented Jan 13, 2025

sarthakaggarwal97 commented Jan 14, 2025

sarthakaggarwal97 commented Jan 14, 2025 • edited Loading

Unstable:

Command: client list

Command: client list type normal

PR:

Command: client list

Command: client list type normal

Notes:

hwware commented Jan 14, 2025 • edited Loading

zuiderkwast commented Jan 14, 2025

sarthakaggarwal97 commented Jan 14, 2025 • edited Loading

hpatro commented Jan 14, 2025

hwware commented Jan 15, 2025

hpatro commented Jan 15, 2025

zuiderkwast commented Jan 15, 2025

sarthakaggarwal97 commented Jan 15, 2025

sarthakaggarwal97 commented Dec 6, 2024 •

edited by zuiderkwast

Loading

codecov bot commented Dec 6, 2024 •

edited

Loading

sarthakaggarwal97 commented Dec 11, 2024 •

edited

Loading

zuiderkwast left a comment •

edited

Loading

hwware commented Jan 13, 2025 •

edited

Loading

sarthakaggarwal97 commented Jan 14, 2025 •

edited

Loading

Command: `client list`

Command: `client list type normal`

Command: `client list`

Command: `client list type normal`

hwware commented Jan 14, 2025 •

edited

Loading

sarthakaggarwal97 commented Jan 14, 2025 •

edited

Loading