redis scan command support #678

lukemakeit · 2023-01-04T11:39:43Z

Problem

redis scan command not support.

Solution

Note:
server_idx is saved by the lowest 12 bits of cursor, so the number of servers of the twemproxy must be less than 4096.

Result

1.1.1.1:60100> scan 0
1) "1"
2) 1) "d"
   2) "b"
   3) "c"
   4) "a"
1.1.1.1:60100> scan 1
1) "0"
2) 1) "a"
   2) "b"

w4096 · 2023-01-04T16:37:26Z

src/nc_server.c

+        r->max_server_idx=(pool->server).nelem;
+    }else{
+        idx = server_pool_idx(pool, key, keylen);
+    }


Change the cursor of request in req_filter and save the server_index on msg in the mean time.

w4096 · 2023-01-04T16:46:40Z

src/proto/nc_redis.c

@@ -3053,6 +3069,46 @@ redis_post_coalesce_mget(struct msg *request)
    }
 }

+void redis_post_coalesce_scan(struct msg *request) {


move those code in rsp_filter

w4096 · 2023-01-04T16:50:57Z

src/proto/nc_redis.c

@@ -2963,6 +2976,9 @@ redis_fragment(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq)
    case MSG_REQ_REDIS_MSET:
        return redis_fragment_argx(r, nserver, frag_msgq, 2);

+    case MSG_REQ_REDIS_SCAN:
+        return redis_fragment_scan(r,frag_msgq);


The scan command doesn't need fragment, the thing you want do is change the cursor, you can do this in req_filter.
When response received you need change the cursor again, you can do this in rsp_filter.

w4096 · 2023-01-04T16:52:28Z

src/nc_server.h

@@ -137,7 +137,7 @@ void server_connected(struct context *ctx, struct conn *conn);
 void server_ok(struct context *ctx, struct conn *conn);

 uint32_t server_pool_idx(const struct server_pool *pool, const uint8_t *key, uint32_t keylen);
-struct conn *server_pool_conn(struct context *ctx, struct server_pool *pool, const uint8_t *key, uint32_t keylen);
+struct conn *server_pool_conn(struct context *ctx, struct server_pool *pool, const uint8_t *key, uint32_t keylen, struct msg *msg);


I think put msg in front of key and keylen is better.

w4096 · 2023-01-04T16:58:13Z

src/nc_server.c

+            nc_memcpy(key,arr,keylen);
+        }
+        r->scan_server_idx=idx;
+        r->max_server_idx=(pool->server).nelem;


I think max_server_idx is unnecessary, since you can always known the number of server by array_n(&pool->server) .

lukemakeit · 2023-01-09T07:06:11Z

Hi @wy-ei , I have re-committed according to your suggestion, thanks.

w4096 · 2023-01-13T14:18:56Z

src/nc_request.c

@@ -511,6 +520,31 @@ req_filter(struct conn *conn, struct msg *msg)
        msg->noforward = 1;
    }

+    if (msg->type == MSG_REQ_REDIS_SCAN) {
+        ASSERT(array_n(msg->keys) > 0);


move this into a function and call the function here like what you did in rsp_filter.

Good advice

w4096 · 2023-01-13T14:25:22Z

src/nc_message.h

@@ -300,6 +301,8 @@ struct msg {
    unsigned             fdone:1;         /* all fragments are done? */
    unsigned             swallow:1;       /* swallow response? */
    unsigned             redis:1;         /* redis? */
+
+    uint32_t             server_index; /* used for store the redis server index in server pool */
 };


this comment is not clear enough. how about this one:

the server index which the requstion should be forwarded.

this comment is not clear enough. how about this one:

the server index which the requstion should be forwarded.

ok

w4096 · 2023-01-13T14:40:17Z

@lukexwang

I have see the change you made. it's better. And I think the code can be optimized again. I have leave some more suggestions.

lukemakeit · 2023-01-14T09:30:08Z

Hi @wy-ei , thank you for your suggestion. I have re-committed.

lukemakeit · 2023-02-07T07:43:20Z

Hello @wy-ei , 空了辛苦看下哈 ^_^

yuerugou54 · 2023-02-07T07:44:24Z

马晓栋已经收到您的来件，谢谢!

redis script command support

…add-scan-support

lukemakeit added 2 commits January 4, 2023 15:29

redis script command support

fa1a7a7

redis scan command support

6e45af7

w4096 reviewed Jan 4, 2023

View reviewed changes

Apply suggestions from code review

4ed8a9e

w4096 reviewed Jan 13, 2023

View reviewed changes

Apply suggestions from code review

b6e9807

lukemakeit and others added 3 commits February 26, 2024 22:42

Merge pull request #1 from lukemakeit/add-script-support

3f904e2

redis script command support

Merge branch 'master' of https://github.com/lukexwang/twemproxy into …

6d3304f

…add-scan-support

fix a scan bug and update readme

758cb66

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

redis scan command support #678

redis scan command support #678

lukemakeit commented Jan 4, 2023

w4096 Jan 4, 2023

w4096 Jan 4, 2023

w4096 Jan 4, 2023

w4096 Jan 4, 2023

w4096 Jan 4, 2023

lukemakeit commented Jan 9, 2023

w4096 Jan 13, 2023

lukemakeit Jan 14, 2023

w4096 Jan 13, 2023

lukemakeit Jan 14, 2023

w4096 commented Jan 13, 2023 •

edited

Loading

lukemakeit commented Jan 14, 2023

lukemakeit commented Feb 7, 2023

yuerugou54 commented Feb 7, 2023 via email

redis scan command support #678

Are you sure you want to change the base?

redis scan command support #678

Conversation

lukemakeit commented Jan 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukemakeit commented Jan 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

w4096 commented Jan 13, 2023 • edited Loading

lukemakeit commented Jan 14, 2023

lukemakeit commented Feb 7, 2023

yuerugou54 commented Feb 7, 2023 via email

w4096 commented Jan 13, 2023 •

edited

Loading