Redis Redis monitor should support filtering on selected database

Command redis-cli -n 1 monitor breaks a lot of expectations by monitoring all databases, instead of the one specified in the -n switch :)

Comment From: arbaieffendi

Actually surprise this one doesn't get many attention until now. Does it mean having cluster of redis-server instance better than having 1 redis-server instance with n database ?

Comment From: madolson

Just for the context, Redis monitor doesn't actually support individual databases, it just dumps everything from all databases. That needs to be implemented first, then we could update the CLI :)

Comment From: enjoy-binbin

I took a quick look, thought of a simple way: we can set a new client flag, #define CLIENT_MONITOR_ALL something like that. To identify whether it is monitoring all dbs. Or a flag that identify the client only monitor the selected db For the selected db, we can simply compare monitor->db->id == dictid in replicationFeedMonitors Since for a monitor client, it would not be able to SELECT a new DB

void replicationFeedMonitors(client *c, list *monitors, int dictid, robj **argv, int argc) {
    ...

    listRewind(monitors,&li);
    while((ln = listNext(&li))) {
        client *monitor = ln->value;
        if ((monitor->flags & CLIENT_MONITOR_SELECTED) && monitor->db->id != dictid) {
            //
            continue;
        }
        addReply(monitor,cmdobj);
        updateClientMemUsage(c);
    }

So the syntax of the new MONITOR command will become something like: monitor db db_number

But obviously there will be such a problem, like i want to monitor a few dbs. If this is useful, i think there can a conclusion and then i can try to implement it @oranagra @madolson

Comment From: oranagra

we have big plans for MONITOR in the future, see https://github.com/redis/redis/projects/4#card-54055687 this is gonna affect both the arguments for the command, and also it's output (both user friendly and machine friendly). it needs a considerable design effort. for now, we don't want to change anything about this command before we get to handle it, since it may conflict with whatever future decisions we'd like to make and / or restrict us from backwards compatibility concerns.

Comment From: enjoy-binbin

ohh right, i have an impression of seeing it, but somehow i forgot it all

Comment From: w4096

@oranagra I haven't see any details on the project card you mentioned.

"this is gonna affect both the arguments for the command, and also it's output"

Are you planning change the output format of monitor command, or just add more arguments and change the code of redis-cli to handle the wrong arguments error?

I think add some filter arguments on monitor command is very useful. currently, I filter it by grep, but the output of monitor can be huge, and this way has a huge impact on the performance. Actually, using monitor in product environment is not possible.

I think it easy to add those arguments as filters:

monitor db 1                         # monitor the requests on db 1
monitor command GET                   # monitor GET requests
monitor addr 192.168.1.100           # monitor client which address is 192.168.1.100
monitor addr 192.168.1.100:40000        # monitor client which address is 192.168.1.100:40000
monitor addr 192.168.1                    # monitor client which address prefix matches with 192.168.1
monitor db 1 command GET                  # filters can be combined.

If we can throttle the output, use monitor on product environment can be possible:

monitor throttle 100                   # at most 100 commands per second

Comment From: oranagra

@wy-ei i'm sorry, we didn't get to handle this topic yet, and the card i previously linked doesn't exist anymore. we understand the importance of filtering MONITOR output at the source, and there are many different filters that could be added, so we don't wanna make any changes to the command syntax before we have a complete design of how we want it to look at the end. in addition to the filters you described above, some people rather monitor just errors (command that fail, or got rejected, rather than successful / executed ones), others may also want to see the reply redis sent to the client (not just the request). besides that, we also think it would be useful for monitor to have a machine readable format in addition to the human readable one we have now.

here are some pleaces in which it was discussed in the past: * https://github.com/redis/redis/pull/8831 * https://github.com/redis/redis/pull/8246 * https://github.com/redis/redis/pull/6832 * https://github.com/redis/redis/issues/150 * https://github.com/filipecosta90/redis/pull/1 * https://github.com/redis/redis/pull/11168 * #12280

for now, we don't have any immediate plan to handle it in the near future.