1. 04 Sep, 2024 1 commit
    • debing.sun's avatar
      Introduce reusable query buffer for client reads (#13488) · ea3e8b79
      debing.sun authored
      This PR is based on the commits from PR
      https://github.com/valkey-io/valkey/pull/258,
      https://github.com/valkey-io/valkey/pull/593,
      https://github.com/valkey-io/valkey/pull/639
      
      This PR optimizes client query buffer handling in Redis by introducing
      a reusable query buffer that is used by default for client reads. This
      reduces memory usage by ~20KB per client by avoiding allocations for
      most clients using short (<16KB) complete commands. For larger or
      partial commands, the client still gets its own private buffer.
      
      The primary changes are:
      
      * Adding a reusable query buffer `thread_shared_qb` that clients use by
      default.
      * Modifying client querybuf initialization and reset logic.
      * Freeing idle client query buffers when empty to allow reuse of the
      reusable query buffer.
      * Master client query buffers are kept private as their contents need to
      be preserved for replication stream.
      * When nested commands is executed, only the first user uses the reuse
      buffer, and subsequent users will still use the private buffer.
      
      In addition to the memory savings, this change shows a 3% improvement in
      latency and throughput when running with 1000 active clients.
      
      The memory reduction may also help reduce the need to evict clients when
      reaching max memory limit, as the query buffer is the main memory
      consumer per client.
      
      This PR is different from https://github.com/valkey-io/valkey/pull/258
      
      
      1. When a client is in the mid of requiring a reused buffer and
      returning it, regardless of whether the query buffer has changed
      (expanded), we do not update the reused query buffer in the middle, but
      return the reused query buffer (expanded or with data remaining) or
      reset it at the end.
      2. Adding a new thread variable `thread_shared_qb_used` to avoid
      multiple clients requiring the reusable query buffer at the same time.
      
      ---------
      Signed-off-by: default avatarUri Yagelnik <uriy@amazon.com>
      Signed-off-by: default avatarMadelyn Olson <matolson@amazon.com>
      Co-authored-by: default avatarUri Yagelnik <uriy@amazon.com>
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      Co-authored-by: default avataroranagra <oran@redislabs.com>
      ea3e8b79
  2. 04 May, 2023 1 commit
    • Binbin's avatar
      Pause cron to prevent premature shrinking in querybuf test (#12126) · e49c2a52
      Binbin authored
      Tests occasionally fail since #12000:
      ```
      *** [err]: query buffer resized correctly when not idle in tests/unit/querybuf.tcl
      Expected 0 > 32768 (context: type eval line 11 cmd {assert {$orig_test_client_qbuf > 32768}} proc ::test)
      
      *** [err]: query buffer resized correctly with fat argv in tests/unit/querybuf.tcl
      query buffer should not be resized when client idle time smaller than 2s
      ```
      
      The reason may be because we set hz to 100, querybuf shrinks before we count
      client_query_buffer. We avoid this problem by setting pause-cron to 1.
      e49c2a52
  3. 16 Apr, 2023 1 commit
    • judeng's avatar
      avoid incorrect shrinking of querybuf when client is reading a big argv (#12000) · e7f18432
      judeng authored
      this pr fix two wrongs:
      1. When client’s querybuf is pre-allocated for a fat argv, we need to update the
        querybuf_peak of the client immediately to completely avoid the unexpected
        shrinking of querybuf in the next clientCron (before data arrives to set the peak).
      2. the protocol's bulklen does not include `\r\n`, but the allocation and the data we
        read does. so in `clientsCronResizeQueryBuffer`, the `resize` or `querybuf_peak`
        should add these 2 bytes.
      
      the first bug is likely to hit us on large payloads over slow connections, in which case
      transferring the payload can take longer and a cron event will be triggered (specifically
      if there are not a lot of clients)
      e7f18432
  4. 06 Aug, 2021 1 commit
  5. 16 Jun, 2021 1 commit
    • sundb's avatar
      Fix querybuf test failure (#9091) · b586d5b5
      sundb authored
      Fix test failure which introduced by #9003.
      The following case will occur when querybuf expansion will allocate memory equal to (16*1024)k.
      1) make use ```CFLAGS=-DNO_MALLOC_USABLE_SIZE```.
      2) ```malloc``` will not allocate more under ```alpine```.
      b586d5b5
  6. 15 Jun, 2021 1 commit
    • sundb's avatar
      Fix the wrong reisze of querybuf (#9003) · e5d8a5eb
      sundb authored
      The initialize memory of `querybuf` is `PROTO_IOBUF_LEN(1024*16) * 2` (due to sdsMakeRoomFor being greedy), under `jemalloc`, the allocated memory will be 40k.
      This will most likely result in the `querybuf` being resized when call `clientsCronResizeQueryBuffer` unless the client requests it fast enough.
      
      Note that this bug existed even before #7875, since the condition for resizing includes the sds headers (32k+6).
      
      ## Changes
      1. Use non-greedy sdsMakeRoomFor when allocating the initial query buffer (of 16k).
      1. Also use non-greedy allocation when working with BIG_ARG (we won't use that extra space anyway)
      2. in case we did use a greedy allocation, read as much as we can into the buffer we got (including internal frag), to reduce system calls.
      3. introduce a dedicated constant for the shrinking (same value as before)
      3. Add test for querybuf.
      4. improve a maxmemory test by ignoring the effect of replica query buffers (can accumulate many ACKs on slow env)
      5. improve a maxmemory by disabling slowlog (it will cause slight memory growth on slow env).
      e5d8a5eb