Support memcache 'version' request, add memcache tests

And update documentation
ReemLotfy · Jul 3, 2021 · 84733fc · 84733fc
1 parent 85f9875
commit 84733fc
Show file tree

Hide file tree

Showing 9 changed files with 98 additions and 42 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -22,13 +22,15 @@ jobs:
 
     # See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#using-environment-variables-in-a-matrix
     strategy:
+      # Run to completion even if one redis version has failures
+      fail-fast: false
       matrix:
        include:
          - REDIS_VER: 3.0.7
          - REDIS_VER: 3.2.13
          - REDIS_VER: 4.0.14
          - REDIS_VER: 5.0.12
-         - REDIS_VER: 6.2.1
+         - REDIS_VER: 6.2.4
 
     # Steps represent a sequence of tasks that will be executed as part of the job
     steps:

diff --git a/ChangeLog b/ChangeLog
@@ -29,6 +29,8 @@
 	  redis commands (tyson)
 	  Optimization: Skip hashing and choosing server index when a pool has
 	  exactly one server (tyson)
+	  Support memcache 'version' requests by proxying the request to a single
+	  backend memcache server. (tyson)
 
  2015-22-06  Manju Rajashekhar  <[email protected]>
     * twemproxy: version 0.4.1 release

diff --git a/notes/debug.txt b/notes/debug.txt
@@ -109,9 +109,9 @@
   EPOLLET       = (1 << 31)
 
   /* opcodes */
-  EPOLL_CTL_ADD = 1 /* add a file decriptor to the interface */
-  EPOLL_CTL_DEL = 2 /* remove a file decriptor from the interface */
-  EPOLL_CTL_MOD = 3 /* change file decriptor epoll_event structure */
+  EPOLL_CTL_ADD = 1 /* add a file descriptor to the interface */
+  EPOLL_CTL_DEL = 2 /* remove a file descriptor from the interface */
+  EPOLL_CTL_MOD = 3 /* change file descriptor epoll_event structure */
 
 - kqueue (bsd)
 

diff --git a/notes/memcache.md b/notes/memcache.md
@@ -30,7 +30,7 @@
   * <data>    - uint8_t[]: data block
   * <cas>     - uint64_t
 
-#### Ascii Retrival Command
+#### Ascii Retrieval Command
 
     +-------------------+------------+--------------------------------------------------------------------------+
     |      Command      | Supported? | Format                                                                   |
@@ -68,11 +68,15 @@
     +-------------------+------------+--------------------------------------------------------------------------+
     |       touch       |    Yes     | touch <key> <expiry>[noreply]\r\n                                        |
     +-------------------+------------+--------------------------------------------------------------------------+
+    |        gat        |  Planned   | gat <expiry> <key>+\r\n                                                  |
+    +-------------------+------------+--------------------------------------------------------------------------+
+    |        gats       |  Planned   | gats <expiry> <key>+\r\n                                                 |
+    +-------------------+------------+--------------------------------------------------------------------------+
     |        quit       |    Yes     | quit\r\n                                                                 |
     +-------------------+------------+--------------------------------------------------------------------------+
     |      flush_all    |    No      | flush_all [<delay>] [noreply]\r\n                                        |
     +-------------------+------------+--------------------------------------------------------------------------+
-    |      version      |    No      | version\r\n                                                              |
+    |      version      |    Yes     | version\r\n                                                              |
     +-------------------+------------+--------------------------------------------------------------------------+
     |      verbosity    |    No      | verbosity <num> [noreply]\r\n                                            |
     +-------------------+------------+--------------------------------------------------------------------------+
@@ -112,7 +116,7 @@
     NOT_FOUND\r\n
     DELETED\r\n
 
-#### Retrival Responses
+#### Retrieval Responses
 
     END\r\n
     VALUE <key> <flags> <datalen> [<cas>]\r\n<data>\r\nEND\r\n
@@ -158,5 +162,7 @@
   - ascii protocol is easier to debug - think using strace or tcpdump to see
     protocol on the wire, Or using telnet or netcat or socat to build memcache
     requests and responses
-    http://stackoverflow.com/questions/2525188/are-binary-protocols-dead
-  - http://news.ycombinator.com/item?id=1712788
+    https://stackoverflow.com/questions/2525188/are-binary-protocols-dead
+  - nutcracker will support the more efficient meta-text protocol after the protocol
+    is marked as stable and memcached servers using it have had several releases.
+  - https://news.ycombinator.com/item?id=1712788
diff --git a/src/nc_message.c b/src/nc_message.c
@@ -892,3 +892,21 @@ msg_send(struct context *ctx, struct conn *conn)
 
     return NC_OK;
 }
+
+/*
+ * Set a placeholder key for a command with no key that is forwarded to an
+ * arbitrary backend.
+ */
+bool msg_set_placeholder_key(struct msg *r)
+{
+    struct keypos *kpos;
+    ASSERT(array_n(r->keys) == 0);
+    kpos = array_push(r->keys);
+    if (kpos == NULL) {
+        return false;
+    }
+    kpos->start = (uint8_t *)"placeholder";
+    kpos->end = kpos->start + sizeof("placeholder") - 1;
+    return true;
+}
+
diff --git a/src/nc_message.h b/src/nc_message.h
@@ -50,6 +50,7 @@ typedef enum msg_parse_result {
     ACTION( REQ_MC_DECR )                                                                           \
     ACTION( REQ_MC_TOUCH )                     /* memcache touch request */                         \
     ACTION( REQ_MC_QUIT )                      /* memcache quit request */                          \
+    ACTION( REQ_MC_VERSION )                   /* memcache version request */                       \
     ACTION( RSP_MC_NUM )                       /* memcache arithmetic response */                   \
     ACTION( RSP_MC_STORED )                    /* memcache cas and storage response */              \
     ACTION( RSP_MC_NOT_STORED )                                                                     \
@@ -59,6 +60,7 @@ typedef enum msg_parse_result {
     ACTION( RSP_MC_VALUE )                                                                          \
     ACTION( RSP_MC_DELETED )                   /* memcache delete response */                       \
     ACTION( RSP_MC_TOUCHED )                   /* memcache touch response */                        \
+    ACTION( RSP_MC_VERSION )                   /* memcache version response */                      \
     ACTION( RSP_MC_ERROR )                     /* memcache error responses */                       \
     ACTION( RSP_MC_CLIENT_ERROR )                                                                   \
     ACTION( RSP_MC_SERVER_ERROR )                                                                   \
@@ -322,6 +324,7 @@ struct mbuf *msg_ensure_mbuf(struct msg *msg, size_t len);
 rstatus_t msg_append(struct msg *msg, const uint8_t *pos, size_t n);
 rstatus_t msg_prepend(struct msg *msg, const uint8_t *pos, size_t n);
 rstatus_t msg_prepend_format(struct msg *msg, const char *fmt, ...);
+bool msg_set_placeholder_key(struct msg *r);
 
 struct msg *req_get(struct conn *conn);
 void req_put(struct msg *msg);

diff --git a/src/proto/nc_memcache.c b/src/proto/nc_memcache.c
@@ -320,6 +320,14 @@ memcache_parse_req(struct msg *r)
                         break;
                     }
 
+                    if (str7cmp(m, 'v', 'e', 'r', 's', 'i', 'o', 'n')) {
+                        r->type = MSG_REQ_MC_VERSION;
+                        if (!msg_set_placeholder_key(r)) {
+                            goto enomem;
+                        }
+                        break;
+                    }
+
                     break;
                 }
 
@@ -342,6 +350,7 @@ memcache_parse_req(struct msg *r)
                     state = SW_SPACES_BEFORE_KEY;
                     break;
 
+                case MSG_REQ_MC_VERSION:
                 case MSG_REQ_MC_QUIT:
                     p = p - 1; /* go back by 1 byte */
                     state = SW_CRLF;
@@ -402,11 +411,10 @@ memcache_parse_req(struct msg *r)
                     state = SW_SPACES_BEFORE_FLAGS;
                 } else if (memcache_arithmetic(r) || memcache_touch(r) ) {
                     state = SW_SPACES_BEFORE_NUM;
-                } else if (memcache_delete(r)) {
-                    state = SW_RUNTO_CRLF;
                 } else if (memcache_retrieval(r)) {
                     state = SW_SPACES_BEFORE_KEYS;
                 } else {
+                    /* delete, etc. */
                     state = SW_RUNTO_CRLF;
                 }
 
@@ -917,6 +925,11 @@ memcache_parse_rsp(struct msg *r)
                         break;
                     }
 
+                    if (str7cmp(m, 'V', 'E', 'R', 'S', 'I', 'O', 'N')) {
+                        r->type = MSG_RSP_MC_VERSION;
+                        break;
+                    }
+
                     break;
 
                 case 9:
@@ -976,6 +989,7 @@ memcache_parse_rsp(struct msg *r)
 
                 case MSG_RSP_MC_CLIENT_ERROR:
                 case MSG_RSP_MC_SERVER_ERROR:
+                case MSG_RSP_MC_VERSION:
                     state = SW_RUNTO_CRLF;
                     break;
 

diff --git a/src/proto/nc_redis.c b/src/proto/nc_redis.c
@@ -397,24 +397,6 @@ redis_error(const struct msg *r)
     return false;
 }
 
-/*
- * Set a placeholder key for a command with no key that is forwarded to an
- * arbitrary backend.
- */
-static bool
-set_placeholder_key(struct msg *r)
-{
-    struct keypos *kpos;
-    ASSERT(array_n(r->keys) == 0);
-    kpos = array_push(r->keys);
-    if (kpos == NULL) {
-        return false;
-    }
-    kpos->start = (uint8_t *)"placeholder";
-    kpos->end = kpos->start + sizeof("placeholder") - 1;
-    return true;
-}
-
 /*
  * Reference: http://redis.io/topics/protocol
  *
@@ -1021,7 +1003,7 @@ redis_parse_req(struct msg *r)
 
                 if (str6icmp(m, 'l', 'o', 'l', 'w', 'u', 't')) {
                     r->type = MSG_REQ_REDIS_LOLWUT;
-                    if (!set_placeholder_key(r)) {
+                    if (!msg_set_placeholder_key(r)) {
                         goto enomem;
                     }
                     break;
@@ -1117,7 +1099,7 @@ redis_parse_req(struct msg *r)
 
                 if (str7icmp(m, 'c', 'o', 'm', 'm', 'a', 'n', 'd')) {
                     r->type = MSG_REQ_REDIS_COMMAND;
-                    if (!set_placeholder_key(r)) {
+                    if (!msg_set_placeholder_key(r)) {
                         goto enomem;
                     }
                     break;

diff --git a/src/test_all.c b/src/test_all.c
@@ -178,6 +178,7 @@ static void test_memcache_parse_rsp_success_case(const char* data, int expected)
     struct conn fake_client = {0};
     struct mbuf *m = mbuf_get();
     const int SW_START = 0;  /* Same as SW_START in memcache_parse_rsp */
+    const int original_failures = failures;
 
     struct msg *rsp = msg_get(&fake_client, 0, 0);
     rsp->state = SW_START;
@@ -200,21 +201,31 @@ static void test_memcache_parse_rsp_success_case(const char* data, int expected)
 
     msg_put(rsp);
     /* mbuf_put(m); */
+    if (original_failures != failures) {
+        printf("Saw test failures for test_memcache_parse_rsp_success_case (%s)\n",
+               data);
+    }
 }
 
 static void test_memcache_parse_rsp_success(void) {
-    test_memcache_parse_rsp_success_case("END\r\n", MSG_RSP_MC_END);
+    test_memcache_parse_rsp_success_case("0\r\n", MSG_RSP_MC_NUM);
+    /* The number returned by the server may be space-padded at the end */
+    test_memcache_parse_rsp_success_case("0  \r\n", MSG_RSP_MC_NUM);
+    test_memcache_parse_rsp_success_case("9223372036854775807\r\n", MSG_RSP_MC_NUM);
     test_memcache_parse_rsp_success_case("DELETED\r\n", MSG_RSP_MC_DELETED);
-    test_memcache_parse_rsp_success_case("TOUCHED\r\n", MSG_RSP_MC_TOUCHED);
-    test_memcache_parse_rsp_success_case("STORED\r\n", MSG_RSP_MC_STORED);
-    test_memcache_parse_rsp_success_case("EXISTS\r\n", MSG_RSP_MC_EXISTS);
+    test_memcache_parse_rsp_success_case("END\r\n", MSG_RSP_MC_END);
     test_memcache_parse_rsp_success_case("ERROR\r\n", MSG_RSP_MC_ERROR);
+    test_memcache_parse_rsp_success_case("EXISTS\r\n", MSG_RSP_MC_EXISTS);
     test_memcache_parse_rsp_success_case("NOT_FOUND\r\n", MSG_RSP_MC_NOT_FOUND);
+    test_memcache_parse_rsp_success_case("STORED\r\n", MSG_RSP_MC_STORED);
+    test_memcache_parse_rsp_success_case("TOUCHED\r\n", MSG_RSP_MC_TOUCHED);
     test_memcache_parse_rsp_success_case("VALUE key 0 2\r\nab\r\nEND\r\n", MSG_RSP_MC_END);
     test_memcache_parse_rsp_success_case("VALUE key 0 2\r\nab\r\nVALUE key2 0 2\r\ncd\r\nEND\r\n", MSG_RSP_MC_END);
+    test_memcache_parse_rsp_success_case("VERSION 1.5.22\r\n", MSG_RSP_MC_VERSION);
 }
 
 static void test_memcache_parse_req_success_case(const char* data, int expected) {
+    const int original_failures = failures;
     struct conn fake_client = {0};
     struct mbuf *m = mbuf_get();
     const int SW_START = 0;  /* Same as SW_START in memcache_parse_req */
@@ -240,23 +251,41 @@ static void test_memcache_parse_req_success_case(const char* data, int expected)
 
     msg_put(req);
     /* mbuf_put(m); */
+    if (original_failures != failures) {
+        printf("Saw test failures for test_memcache_parse_req_success_case (%s)\n",
+               data);
+    }
 }
 
 static void test_memcache_parse_req_success(void) {
-    test_memcache_parse_req_success_case("get key\r\n", MSG_REQ_MC_GET);
-    test_memcache_parse_req_success_case("gets u\r\n", MSG_REQ_MC_GETS);
-    test_memcache_parse_req_success_case("get a b xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\r\n", MSG_REQ_MC_GET);
-    test_memcache_parse_req_success_case("set key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_SET);
-    test_memcache_parse_req_success_case("set key 0 5 10 noreply\r\nvalue12345\r\n", MSG_REQ_MC_SET);
     test_memcache_parse_req_success_case("add key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_ADD);
-    test_memcache_parse_req_success_case("replace key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_REPLACE);
+    /* Can add any binary data such as '\n' */
+    test_memcache_parse_req_success_case("add key 0 0 1 noreply\r\n\n\r\n", MSG_REQ_MC_ADD);
     test_memcache_parse_req_success_case("append key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_APPEND);
-    test_memcache_parse_req_success_case("prepend key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_PREPEND);
+    test_memcache_parse_req_success_case("append key 0 1 0 noreply\r\n\r\n", MSG_REQ_MC_APPEND);
     test_memcache_parse_req_success_case("cas key 0 600 5 123456\r\nvalue\r\n", MSG_REQ_MC_CAS);
+    test_memcache_parse_req_success_case("cas key 0 1 1 1 noreply\r\nx\r\n", MSG_REQ_MC_CAS);
+    test_memcache_parse_req_success_case("decr key 0\r\n", MSG_REQ_MC_DECR);
+    test_memcache_parse_req_success_case("decr key 0 noreply\r\n", MSG_REQ_MC_DECR);
+    test_memcache_parse_req_success_case("delete a noreply\r\n", MSG_REQ_MC_DELETE);
     test_memcache_parse_req_success_case("delete key\r\n", MSG_REQ_MC_DELETE);
+    /* TODO https://github.com/twitter/twemproxy/issues/631 gat/gats */
+    /* test_memcache_parse_req_success_case("gat 3600 key\r\n", MSG_REQ_MC_GAT); */
+    test_memcache_parse_req_success_case("get a b xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\r\n", MSG_REQ_MC_GET);
+    test_memcache_parse_req_success_case("get key\r\n", MSG_REQ_MC_GET);
+    test_memcache_parse_req_success_case("gets u\r\n", MSG_REQ_MC_GETS);
     test_memcache_parse_req_success_case("incr key 1\r\n", MSG_REQ_MC_INCR);
+    test_memcache_parse_req_success_case("incr key 9223372036854775807 noreply\r\n", MSG_REQ_MC_INCR);
+    test_memcache_parse_req_success_case("prepend key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_PREPEND);
+    test_memcache_parse_req_success_case("prepend key 0 600 0 noreply\r\n\r\n", MSG_REQ_MC_PREPEND);
     test_memcache_parse_req_success_case("quit\r\n", MSG_REQ_MC_QUIT);
+    test_memcache_parse_req_success_case("replace key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_REPLACE);
+    test_memcache_parse_req_success_case("replace key 0 9 0 noreply\r\n\r\n", MSG_REQ_MC_REPLACE);
+    test_memcache_parse_req_success_case("set key 0 5 10 noreply\r\nvalue12345\r\n", MSG_REQ_MC_SET);
+    test_memcache_parse_req_success_case("set key 0 600 5\r\nvalue\r\n", MSG_REQ_MC_SET);
     test_memcache_parse_req_success_case("touch key 12345\r\n", MSG_REQ_MC_TOUCH);
+    test_memcache_parse_req_success_case("touch key 12345 noreply\r\n", MSG_REQ_MC_TOUCH);
+    test_memcache_parse_req_success_case("version\r\n", MSG_REQ_MC_VERSION);
 }
 
 int main(int argc, char **argv) {