From bc78f1f954b4871b63624c616f62cf0b8dfd13a4 Mon Sep 17 00:00:00 2001 From: Akashdeep Goel Date: Tue, 19 Jul 2022 13:03:22 -0700 Subject: [PATCH 1/2] Fixes crash on large payloads for MGET/MSET/SCAN Updates python version to 3.9 Updates pip version Update travis.sh Adds apt update before package installation Force pip upgrade --- .travis.yml | 6 +++++- src/dyn_client.c | 6 ++++++ src/dyn_message.h | 4 ++++ src/proto/dyn_redis.c | 14 ++++++++------ 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 46f387a36..4889d36af 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,12 @@ language: c script: bash ./travis.sh +before_install: + - sudo pip3 install --upgrade pip + addons: apt: + update: true packages: - - python3 + - python3.9 - python3-pip diff --git a/src/dyn_client.c b/src/dyn_client.c index 3b67e2ce7..e1671f773 100644 --- a/src/dyn_client.c +++ b/src/dyn_client.c @@ -1099,6 +1099,12 @@ void req_recv_done(struct context *ctx, struct conn *conn, struct msg *req, if (status != DN_OK) goto error; status = fragment_query_if_necessary(req, conn, &frag_msgq); + if (status == DYNOMITE_PAYLOAD_TOO_LARGE) { + dictAdd(conn->outstanding_msgs_dict, &req->id, req); + req->nfrag = 0; // Since we failed to fragment the payload, we don't have any fragments for this request + goto error; + } + if (status != DN_OK) goto error; status = rewrite_query_with_timestamp_md(&req, ctx); diff --git a/src/dyn_message.h b/src/dyn_message.h index 19c08f08a..87a883155 100644 --- a/src/dyn_message.h +++ b/src/dyn_message.h @@ -294,6 +294,7 @@ typedef enum dyn_error { BAD_FORMAT, DYNOMITE_NO_QUORUM_ACHIEVED, DYNOMITE_SCRIPT_SPANS_NODES, + DYNOMITE_PAYLOAD_TOO_LARGE, } dyn_error_t; static inline char *dn_strerror(dyn_error_t err) { @@ -318,6 +319,8 @@ static inline char *dn_strerror(dyn_error_t err) { return "Failed to achieve Quorum"; case DYNOMITE_SCRIPT_SPANS_NODES: return "Keys in the script cannot span multiple nodes"; + case DYNOMITE_PAYLOAD_TOO_LARGE: + return "MSET/MGET/SCAN payload too large"; default: return strerror(err); } @@ -329,6 +332,7 @@ static inline char *dyn_error_source(dyn_error_t err) { case DYNOMITE_INVALID_STATE: case DYNOMITE_NO_QUORUM_ACHIEVED: case DYNOMITE_SCRIPT_SPANS_NODES: + case DYNOMITE_PAYLOAD_TOO_LARGE: return "Dynomite:"; case PEER_CONNECTION_REFUSE: case PEER_HOST_DOWN: diff --git a/src/proto/dyn_redis.c b/src/proto/dyn_redis.c index ca026b034..14b6f8678 100644 --- a/src/proto/dyn_redis.c +++ b/src/proto/dyn_redis.c @@ -3030,7 +3030,9 @@ static rstatus_t redis_copy_bulk(struct msg *dst, struct msg *src, bool log) { } p = mbuf->pos; - ASSERT(*p == '$'); + if (*p != '$') { + return DYNOMITE_PAYLOAD_TOO_LARGE; + } p++; if (p[0] == '-' && p[1] == '1') { @@ -3044,7 +3046,7 @@ static rstatus_t redis_copy_bulk(struct msg *dst, struct msg *src, bool log) { } len += CRLF_LEN * 2; len += (p - mbuf->pos); - } + } bytes = len; /* copy len bytes to dst */ @@ -3378,7 +3380,7 @@ static rstatus_t redis_append_key(struct msg *r, struct keypos *kpos_src) { * +--------------------+ +---------------------+ +----+----------------+ * | frag_id = 10 | | frag_id = 10 | | frag_id = 10 | * | nfrag = 3 | | nfrag = 0 | | nfrag = 0 | - * | frag_seq = x x x | | key1, key3 | | key2 | + * | frag_seq = x x x | | key1, key3 | | key2 | * +------------|-|-|---+ +---------------------+ +---------------------+ * | | | ^ ^ ^ * | \ \ | | | @@ -3506,7 +3508,7 @@ static rstatus_t redis_fragment_argx(struct msg *r, struct server_pool *pool, TAILQ_INSERT_TAIL(frag_msgq, sub_msg, m_tqe); } - status = redis_append_key(sub_msg, kpos); + status = redis_append_key(sub_msg, kpos); // Adds key to the sub_msg if (status != DN_OK) { dn_free(sub_msgs); return status; @@ -3514,13 +3516,13 @@ static rstatus_t redis_fragment_argx(struct msg *r, struct server_pool *pool, if (key_step == 1) { // mget,del continue; } else { // mset - status = redis_copy_bulk(NULL, r, false); // eat key + status = redis_copy_bulk(NULL, r, false); // Consumes key portion of the payload if (status != DN_OK) { dn_free(sub_msgs); return status; } - status = redis_copy_bulk(sub_msg, r, false); + status = redis_copy_bulk(sub_msg, r, false); // Consumes and copies value to the sub_msg fragment if (status != DN_OK) { dn_free(sub_msgs); return status; From 9042cde63957b202a4b3dcaa2c0e2444fdd5fc84 Mon Sep 17 00:00:00 2001 From: Akashdeep Goel Date: Sun, 31 Jul 2022 10:46:29 -0700 Subject: [PATCH 2/2] Fix pip version --- .travis.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4889d36af..0bd236184 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,11 +2,13 @@ language: c script: bash ./travis.sh before_install: - - sudo pip3 install --upgrade pip + - sudo apt remove --purge python3-pip + - curl -O https://bootstrap.pypa.io/pip/3.5/get-pip.py + - sudo -E python3 get-pip.py + - sudo -E python3 -m pip install --upgrade "pip < 22.3" addons: apt: update: true packages: - python3.9 - - python3-pip