Open JyotinderSingh opened 2 weeks ago
please assign this to me
@JyotinderSingh can you unassign this issue https://github.com/DiceDB/dice/issues/1129 and assign this issue to me i want to try mid-level issue
please assign this to me
Assigned.
@JyotinderSingh can you unassign this issue #1129 and assign this issue to me i want to try mid-level issue
Hey @swarajrb7, assigning this to @kakdeykaushik since they commented first. Will be adding more mid level issues and will tag you there.
Hey, it may be good to discuss the design before you begin working on the implementation. We need to ensure this does not introduce any performance regressions on the critical path. Feel free to let us know on discord if you'd like to discuss this in a larger group.
I have been doing some research on how redis behaves and implements this command, redis's client management impl and what all syscall and socket options we can use to get required information.
Also implemented other simpler CLIENT command CLIENT ID
, CLIENT GETNAME
& CLIENT SETNAME
to get some understanding.
We don't store all the connected clients to dicedb server. Except Async server which is exclusive for clients conncted to it(connectedClients @ server.go#L36). For HTTP server we just create a client but never store it. For WS server post #1090 we will be creating client object but only if they have triggered QWATCH command. We would need to create object for all the clients. Again we never store it.
A global list of all the connected clients which would be modified only when client connects/disconnects and read only when CLIENT LIST
command is fired.
Extra - If we implement autoincrement client id we can optimize read/write ops of this global list. Implementation - #1157.
Implementation details and impact(for critical path) for each fields are mentioned. I have checkboxed the ones whose Redis behaviour I'm aware of and our approach that I arrived at. Descoped won't be implemented in dicedb as we don't support relevant functionalities. (please correct me if I have missed something for example last time i checked we were not going to impl transactions but i still see them around.) For unchecked fields i have to spend more time.
[x] id: a unique 64-bit client ID
[x] addr: address/port of the client
[x] laddr: address/port of local address client connected to (bind address)
[x] fd: file descriptor corresponding to the socket
f, _ := conn.NetConn().(*net.TCPConn).File()
fd := f.Fd()
[x] name: the name set by the client with CLIENT SETNAME
[x] age: total duration of the connection in seconds
[x] idle: idle time of the connection in seconds
SO_TIMESTAMP
for every FD - with this kernel will keep track of timestamp for each packet that goes through (we can do socket level monitoring with this, but overkill as of now i feel and throughput/latency would be impacted negatively)[ ] flags: client flags (see below)
[-] db: current database ID
[-] sub: number of channel subscriptions
[-] psub: number of pattern matching subscriptions
[-] ssub: number of shard channel subscriptions. Added in Redis 7.0.3
[-] multi: number of commands in a MULTI/EXEC context
[-] watch: number of keys this client is currently watching. Added in Redis 7.4
[ ] qbuf: query buffer length (0 means no query pending)
[ ] qbuf-free: free space of the query buffer (0 means the buffer is full)
[ ] argv-mem: incomplete arguments for the next command (already extracted from query buffer)
[-] multi-mem: memory is used up by buffered multi commands. Added in Redis 7.0
[ ] obl: output buffer length
[ ] oll: output list length (replies are queued in this list when the buffer is full)
[ ] omem: output buffer memory usage
[ ] tot-mem: total memory consumed by this client in its various buffers
[ ] events: file descriptor events (see below)
[x] cmd: last command played
[x] user: the authenticated username of the client
[ ] redir: client id of current client tracking redirection
[ ] resp: client RESP protocol version. Added in Redis 7.0
Values qbuf
, qbuf-free
, argv-mem
, mutli-mem
, obl
, oll
, omem
, tot-mem
are derived from querybuf
, buf
and relevent fields from Redis's client
struct.
Primary purpose of these fields is to keep track of size of query / response to keep memory consumption in check and even terminate the client connection when consumption is too high.
Implementing these functionalities in DiceDB may impact performance since for each client we would have this buffer overhead and operating them for every query. On the plus side having these functionalities can help terminate clients which are faulty (for lack of better word).
argv-mem
- args length. Example - CLIENT LIST
= 10 (6+4).
multi-mem
- same as argv-mem but for MULTI command.
For these fields we should have discussion, for rest of the fields I'll start the implementing them.
Add support for the
CLIENT LIST
command in DiceDB similar to theCLIENT LIST
command in Redis. Please refer to the following commit in Redis to understand the implementation specifics - source.Write unit and integration tests for the command referring to the tests written in the Redis codebase 7.2.5. For integration tests, you can refer to the
tests
folder. Note: they have usedTCL
for the test suite, and we need to port that to our way of writing integration tests using the relevant helper methods. Please refer to our tests directory.For the command, benchmark the code and measure the time taken and memory allocs using
benchmem
and try to keep them to the bare minimum.