Skip to content

Commit

Permalink
Fixes crash on large payloads for MGET/MSET/SCAN
Browse files Browse the repository at this point in the history
Updates python version to 3.9

Updates pip version

Update travis.sh

Adds apt update before package installation

Force pip upgrade
  • Loading branch information
akashdeepgoel committed Jul 31, 2022
1 parent b56cf6a commit bc78f1f
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
6 changes: 5 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions src/dyn_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/dyn_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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:
Expand Down
14 changes: 8 additions & 6 deletions src/proto/dyn_redis.c
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -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 */
Expand Down Expand Up @@ -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 |
* +------------|-|-|---+ +---------------------+ +---------------------+
* | | | ^ ^ ^
* | \ \ | | |
Expand Down Expand Up @@ -3506,21 +3508,21 @@ 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;
}
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;
Expand Down

0 comments on commit bc78f1f

Please sign in to comment.