diff --git a/src/networking.c b/src/networking.c index b4c63b47e..ff14a5a32 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2212,24 +2212,6 @@ void readQueryFromClient(connection *conn) { processInputBuffer(c); } -void getClientsMaxBuffers(unsigned long *longest_output_list, - unsigned long *biggest_input_buffer) { - client *c; - listNode *ln; - listIter li; - unsigned long lol = 0, bib = 0; - - listRewind(server.clients,&li); - while ((ln = listNext(&li)) != NULL) { - c = listNodeValue(ln); - - if (listLength(c->reply) > lol) lol = listLength(c->reply); - if (sdslen(c->querybuf) > bib) bib = sdslen(c->querybuf); - } - *longest_output_list = lol; - *biggest_input_buffer = bib; -} - /* A Redis "Address String" is a colon separated ip:port pair. * For IPv4 it's in the form x.y.z.k:port, example: "127.0.0.1:1234". * For IPv6 addresses we use [] around the IP part, like in "[::1]:1234". diff --git a/src/server.c b/src/server.c index 8d232b53b..0ae43d148 100644 --- a/src/server.c +++ b/src/server.c @@ -1673,17 +1673,19 @@ int clientsCronResizeQueryBuffer(client *c) { size_t querybuf_size = sdsalloc(c->querybuf); time_t idletime = server.unixtime - c->lastinteraction; - /* Only resize the query buffer if the buffer is bigger than - * PROTO_RESIZE_THRESHOLD, and it is actually wasting at least a few kbytes. */ - if (querybuf_size > PROTO_RESIZE_THRESHOLD && sdsavail(c->querybuf) > 1024*4) { + /* Only resize the query buffer if the buffer is actually wasting at least a + * few kbytes */ + if (sdsavail(c->querybuf) > 1024*4) { /* There are two conditions to resize the query buffer: */ if (idletime > 2) { /* 1) Query is idle for a long time. */ c->querybuf = sdsRemoveFreeSpace(c->querybuf); - } else if (querybuf_size/2 > c->querybuf_peak) { - /* 2) Query buffer is too big for latest peak. trim excess space but - * only up to a limit, not below the recent peak and current - * c->querybuf (which will be soon get used). */ + } else if (querybuf_size > PROTO_RESIZE_THRESHOLD && querybuf_size/2 > c->querybuf_peak) { + /* 2) Query buffer is too big for latest peak and is larger than + * resize threshold. Trim excess space but only up to a limit, + * not below the recent peak and current c->querybuf (which will + * be soon get used). If we're in the middle of a bulk then make + * sure not to resize to less than the bulk length. */ size_t resize = sdslen(c->querybuf); if (resize < c->querybuf_peak) resize = c->querybuf_peak; if (c->bulklen != -1 && resize < (size_t)c->bulklen) resize = c->bulklen; diff --git a/src/server.h b/src/server.h index e5228bb5e..7e76c0a0f 100644 --- a/src/server.h +++ b/src/server.h @@ -1911,8 +1911,6 @@ size_t sdsZmallocSize(sds s); size_t getStringObjectSdsUsedMemory(robj *o); void freeClientReplyValue(void *o); void *dupClientReplyValue(void *o); -void getClientsMaxBuffers(unsigned long *longest_output_list, - unsigned long *biggest_input_buffer); char *getClientPeerId(client *client); char *getClientSockName(client *client); sds catClientInfoString(sds s, client *client); diff --git a/tests/unit/querybuf.tcl b/tests/unit/querybuf.tcl index 458f8e844..bbbea12f4 100644 --- a/tests/unit/querybuf.tcl +++ b/tests/unit/querybuf.tcl @@ -21,28 +21,46 @@ start_server {tags {"querybuf slow"}} { # The test will run at least 2s to check if client query # buffer will be resized when client idle 2s. test "query buffer resized correctly" { - # Memory will increase by more than 32k due to client query buffer. - set rd [redis_deferring_client] + set rd [redis_client] $rd client setname test_client set orig_test_client_qbuf [client_query_buffer test_client] + # Make sure query buff has less than the peak resize threshold (PROTO_RESIZE_THRESHOLD) 32k + # but at least the basic IO reading buffer size (PROTO_IOBUF_LEN) 16k assert {$orig_test_client_qbuf >= 16384 && $orig_test_client_qbuf < 32768} - # Check that the initial query buffer is not resized if it is idle for more than 2s + # Check that the initial query buffer is resized after 2 sec wait_for_condition 1000 10 { - [client_idle_sec test_client] > 3 && [client_query_buffer test_client] == $orig_test_client_qbuf + [client_idle_sec test_client] >= 3 && [client_query_buffer test_client] == 0 } else { - fail "query buffer was resized" + fail "query buffer was not resized" } + $rd close + } - # Fill query buffer to more than 32k - $rd set bigstring v ;# create bigstring in advance to avoid adding extra memory - $rd set bigstring [string repeat A 32768] nx + test "query buffer resized correctly when not idle" { + # Memory will increase by more than 32k due to client query buffer. + set rd [redis_client] + $rd client setname test_client - # Wait for query buffer to be resized to 0. - wait_for_condition 1000 10 { - [client_query_buffer test_client] == 0 - } else { - fail "querybuf expected to be resized" + # Create a large query buffer (more than PROTO_RESIZE_THRESHOLD - 32k) + $rd set x [string repeat A 400000] + + # Make sure query buff is larger than the peak resize threshold (PROTO_RESIZE_THRESHOLD) 32k + set orig_test_client_qbuf [client_query_buffer test_client] + assert {$orig_test_client_qbuf > 32768} + + # Wait for qbuf to shrink due to lower peak + set t [clock milliseconds] + while true { + # Write something smaller, so query buf peak can shrink + $rd set x [string repeat A 100] + set new_test_client_qbuf [client_query_buffer test_client] + if {$new_test_client_qbuf < $orig_test_client_qbuf} { break } + if {[expr [clock milliseconds] - $t] > 1000} { break } + after 10 } + # Validate qbuf shrunk but isn't 0 since we maintain room based on latest peak + assert {[client_query_buffer test_client] > 0 && [client_query_buffer test_client] < $orig_test_client_qbuf} + $rd close } }