redis scan command support by lukemakeit · Pull Request #678 · twitter/twemproxy

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

    }
 }

+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

        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


 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

    }

+    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

    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

Conversation

lukemakeit commented Jan 4, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukemakeit commented Jan 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w4096 commented Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukemakeit commented Jan 14, 2023

Uh oh!

lukemakeit commented Feb 7, 2023

Uh oh!

yuerugou54 commented Feb 7, 2023 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

w4096 commented Jan 13, 2023 •

edited

Loading