Perf: Track net bytes only if commandlog-request-larger-than != -1 by dvkashapov · Pull Request #3086 · valkey-io/valkey

dvkashapov · 2026-01-21T10:09:44Z

Currently after #2652 in copy avoidance path we unconditionally track c->net_output_bytes_curr_cmd even when commandlog-request-larger-than -1. This PR provides ability to skip that accounting in copy avoidance path based on config value. If commandlog-request-larger-than -1 then performance is same as before #2652

Also added tracking of c->net_output_bytes_curr_cmd in IO thread if it is not tracked in main thread based on decision made in main thread.

Read Performance (write performance is the same):

With this change:
Summary:
  throughput summary: 2191732.75 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
        1.720     0.072     1.743     1.919     2.647    23.983

Unstable: 
Summary:
  throughput summary: 1658197.25 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
        2.299     0.120     2.343     2.503     3.319     4.791

Config:
commandlog-request-larger-than -1
^^ without this performance is just like on unstable branch
databases 1
save ""
appendonly no
rdbcompression no
activedefrag no
maxclients 1000
io-threads 9
protected-mode no
hz 10
maxmemory 2gb

Closes #2926.

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

codecov · 2026-01-21T10:28:28Z

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.34%. Comparing base (951d942) to head (e31cee9).
⚠️ Report is 13 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/networking.c	75.00%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3086      +/-   ##
============================================
- Coverage     74.36%   74.34%   -0.02%     
============================================
  Files           129      129              
  Lines         71012    71021       +9     
============================================
- Hits          52807    52801       -6     
- Misses        18205    18220      +15

Files with missing lines	Coverage Δ
src/networking.c	`89.61% <75.00%> (-0.07%)`	⬇️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

zuiderkwast

WIP review

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

zuiderkwast

Very good, thank you!

zuiderkwast · 2026-01-22T10:31:01Z

@roshkhatri @madolson @enjoy-binbin FYI: This fix allows users to mitigate to the performance regression added in 9.0.1 by #2652 by disabling the large-reply log. This fix adds a note about it in valkey.conf.

We should include this fix in 9.0.2. Later, we can consider disabling the large-reply log by default (in 10.0) if we haven't found another solution by then.

enjoy-binbin

Thanks, so if we are enabling commandlog-request-larger-than, we will track it (request and slot-stats) in the main thread. And if we disblae commandlog-request-larger-than, there is no need to do it and we can just track slot-stats in the io-threads? Smart move!

roshkhatri

Great, Thank you so much!!
can we run perf tests on the the PR?

zuiderkwast · 2026-01-22T18:29:01Z

can we run perf tests on the the PR?

You mean the automated one? If it disables command-log-large-reply, it can be useful. Does it? Otherwise, it will show just the same as unstable.

roshkhatri · 2026-01-22T19:18:54Z

The test doesn't disable it by default. Can we disable it by default for 9.1 and keep working on a solution?
Because for 9.1 we would have to add a condition to disable it or users would see regression against 9.0.

zuiderkwast · 2026-01-22T22:32:50Z

Can we disable it by default for 9.1 and keep working on a solution?

We normally don't change defaults in minor versions, because that's not considered backward compatible.

If we really want to change the default, we could change our release plans and bump the next version directly to 10.0 sooner, skipping 9.1.

Because for 9.1 we would have to add a condition to disable it or users would see regression against 9.0.

They will see a regression compared to 9.0.0, but that's because 9.0.0 was buggy and just skipped the large-reply logging for io-threads > 1. If they compare to 9.0.1 or 8.x.y, they shouldn't see a regression.

We're adding a note in valkey.conf. Daniil suggested the we can log a warning on startup if io-threads + large-reply log are enabled. We can mention it in release notes... I hope this is enough, but we can discuss other options too.

roshkhatri · 2026-01-27T18:04:30Z

We're adding a note in valkey.conf. Daniil suggested the we can log a warning on startup if io-threads + large-reply log are enabled. We can mention it in release notes... I hope this is enough, but we can discuss other options too.

I think this should be sufficient notice for the users

This change amends #3086 in which `commandlog-request-larger-than` was mixed up with `commandlog-reply-larger-than` while checking for config values and in tests. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

…alkey-io#3086) Currently after valkey-io#2652 in copy avoidance path we unconditionally track `c->net_output_bytes_curr_cmd` even when `commandlog-request-larger-than -1`. This PR provides ability to skip that accounting in copy avoidance path based on config value. If `commandlog-request-larger-than -1` then performance is same as before valkey-io#2652. Also added tracking of `c->net_output_bytes_curr_cmd` in IO thread if it is not tracked in main thread based on decision made in main thread. Read Performance (write performance is the same): ``` With this change: Summary: throughput summary: 2191732.75 requests per second latency summary (msec): avg min p50 p95 p99 max 1.720 0.072 1.743 1.919 2.647 23.983 Unstable: Summary: throughput summary: 1658197.25 requests per second latency summary (msec): avg min p50 p95 p99 max 2.299 0.120 2.343 2.503 3.319 4.791 Config: commandlog-request-larger-than -1 ^^ without this performance is just like on unstable branch databases 1 save "" appendonly no rdbcompression no activedefrag no maxclients 1000 io-threads 9 protected-mode no hz 10 maxmemory 2gb ``` --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

This change amends valkey-io#3086 in which `commandlog-request-larger-than` was mixed up with `commandlog-reply-larger-than` while checking for config values and in tests. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

…alkey-io#3086) Currently after valkey-io#2652 in copy avoidance path we unconditionally track `c->net_output_bytes_curr_cmd` even when `commandlog-request-larger-than -1`. This PR provides ability to skip that accounting in copy avoidance path based on config value. If `commandlog-request-larger-than -1` then performance is same as before valkey-io#2652. Also added tracking of `c->net_output_bytes_curr_cmd` in IO thread if it is not tracked in main thread based on decision made in main thread. Read Performance (write performance is the same): ``` With this change: Summary: throughput summary: 2191732.75 requests per second latency summary (msec): avg min p50 p95 p99 max 1.720 0.072 1.743 1.919 2.647 23.983 Unstable: Summary: throughput summary: 1658197.25 requests per second latency summary (msec): avg min p50 p95 p99 max 2.299 0.120 2.343 2.503 3.319 4.791 Config: commandlog-request-larger-than -1 ^^ without this performance is just like on unstable branch databases 1 save "" appendonly no rdbcompression no activedefrag no maxclients 1000 io-threads 9 protected-mode no hz 10 maxmemory 2gb ``` --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

This change amends valkey-io#3086 in which `commandlog-request-larger-than` was mixed up with `commandlog-reply-larger-than` while checking for config values and in tests. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

…3086) Currently after #2652 in copy avoidance path we unconditionally track `c->net_output_bytes_curr_cmd` even when `commandlog-request-larger-than -1`. This PR provides ability to skip that accounting in copy avoidance path based on config value. If `commandlog-request-larger-than -1` then performance is same as before #2652. Also added tracking of `c->net_output_bytes_curr_cmd` in IO thread if it is not tracked in main thread based on decision made in main thread. Read Performance (write performance is the same): ``` With this change: Summary: throughput summary: 2191732.75 requests per second latency summary (msec): avg min p50 p95 p99 max 1.720 0.072 1.743 1.919 2.647 23.983 Unstable: Summary: throughput summary: 1658197.25 requests per second latency summary (msec): avg min p50 p95 p99 max 2.299 0.120 2.343 2.503 3.319 4.791 Config: commandlog-request-larger-than -1 ^^ without this performance is just like on unstable branch databases 1 save "" appendonly no rdbcompression no activedefrag no maxclients 1000 io-threads 9 protected-mode no hz 10 maxmemory 2gb ``` --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

This change amends #3086 in which `commandlog-request-larger-than` was mixed up with `commandlog-reply-larger-than` while checking for config values and in tests. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

…alkey-io#3086) Currently after valkey-io#2652 in copy avoidance path we unconditionally track `c->net_output_bytes_curr_cmd` even when `commandlog-request-larger-than -1`. This PR provides ability to skip that accounting in copy avoidance path based on config value. If `commandlog-request-larger-than -1` then performance is same as before valkey-io#2652. Also added tracking of `c->net_output_bytes_curr_cmd` in IO thread if it is not tracked in main thread based on decision made in main thread. Read Performance (write performance is the same): ``` With this change: Summary: throughput summary: 2191732.75 requests per second latency summary (msec): avg min p50 p95 p99 max 1.720 0.072 1.743 1.919 2.647 23.983 Unstable: Summary: throughput summary: 1658197.25 requests per second latency summary (msec): avg min p50 p95 p99 max 2.299 0.120 2.343 2.503 3.319 4.791 Config: commandlog-request-larger-than -1 ^^ without this performance is just like on unstable branch databases 1 save "" appendonly no rdbcompression no activedefrag no maxclients 1000 io-threads 9 protected-mode no hz 10 maxmemory 2gb ``` --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

This change amends valkey-io#3086 in which `commandlog-request-larger-than` was mixed up with `commandlog-reply-larger-than` while checking for config values and in tests. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

…alkey-io#3086) Currently after valkey-io#2652 in copy avoidance path we unconditionally track `c->net_output_bytes_curr_cmd` even when `commandlog-request-larger-than -1`. This PR provides ability to skip that accounting in copy avoidance path based on config value. If `commandlog-request-larger-than -1` then performance is same as before valkey-io#2652. Also added tracking of `c->net_output_bytes_curr_cmd` in IO thread if it is not tracked in main thread based on decision made in main thread. Read Performance (write performance is the same): ``` With this change: Summary: throughput summary: 2191732.75 requests per second latency summary (msec): avg min p50 p95 p99 max 1.720 0.072 1.743 1.919 2.647 23.983 Unstable: Summary: throughput summary: 1658197.25 requests per second latency summary (msec): avg min p50 p95 p99 max 2.299 0.120 2.343 2.503 3.319 4.791 Config: commandlog-request-larger-than -1 ^^ without this performance is just like on unstable branch databases 1 save "" appendonly no rdbcompression no activedefrag no maxclients 1000 io-threads 9 protected-mode no hz 10 maxmemory 2gb ``` --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com> Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>

This change amends valkey-io#3086 in which `commandlog-request-larger-than` was mixed up with `commandlog-reply-larger-than` while checking for config values and in tests. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com> Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [docker.io/valkey/valkey](https://github.com/valkey-io/valkey) | image | patch | `9.0.1` → `9.0.4` | --- ### Release Notes <details> <summary>valkey-io/valkey (docker.io/valkey/valkey)</summary> ### [`v9.0.4`](https://github.com/valkey-io/valkey/releases/tag/9.0.4) [Compare Source](valkey-io/valkey@9.0.3...9.0.4) Upgrade urgency SECURITY: This release includes security fixes we recommend you apply as soon as possible. ##### Security fixes - (CVE-2026-23479) Use-After-Free in unblock client flow - (CVE-2026-25243) Invalid Memory Access in RESTORE command - (CVE-2026-23631) Use-after-free when full sync occurs during a yielding Lua/function execution ### [`v9.0.3`](https://github.com/valkey-io/valkey/releases/tag/9.0.3) [Compare Source](valkey-io/valkey@9.0.2...9.0.3) ##### Valkey 9.0.3 Upgrade urgency SECURITY: This release includes security fixes we recommend you apply as soon as possible. ##### Security fixes - (CVE-2025-67733) RESP Protocol Injection via Lua error\_reply - (CVE-2026-21863) Remote DoS with malformed Valkey Cluster bus message - (CVE-2026-27623) Reset request type after handling empty requests ##### Bug fixes - Avoids crash during MODULE UNLOAD when ACL rules reference a module command and subcommand ([#3160](valkey-io/valkey#3160)) - Fix server assert on ACL LOAD when current user loses permission to channels ([#3182](valkey-io/valkey#3182)) - Fix bug causing no response flush sometimes when IO threads are busy ([#3205](valkey-io/valkey#3205)) ### [`v9.0.2`](https://github.com/valkey-io/valkey/releases/tag/9.0.2) [Compare Source](valkey-io/valkey@9.0.1...9.0.2) Upgrade urgency HIGH: There are critical bugs that may affect a subset of users. #### Bug fixes - Avoid memory leak of new argv when HEXPIRE commands target only non-exiting fields ([#2973](valkey-io/valkey#2973)) - Fix HINCRBY and HINCRBYFLOAT to update volatile key tracking ([#2974](valkey-io/valkey#2974)) - Avoid empty hash object when HSETEX added no fields ([#2998](valkey-io/valkey#2998)) - Fix case-sensitive check for the FNX and FXX arguments in HSETEX ([#3000](valkey-io/valkey#3000)) - Prevent assertion in active expiration job after a hash with volatile fields is overwritten ([#3003](valkey-io/valkey#3003), [#3007](valkey-io/valkey#3007)) - Fix HRANDFIELD to return null response when no field could be found ([#3022](valkey-io/valkey#3022)) - Fix HEXPIRE to not delete items when validation rules fail and expiration is in the past ([#3023](valkey-io/valkey#3023), [#3048](valkey-io/valkey#3048)) - Fix how hash is handling overriding of expired fields overwrite ([#3060](valkey-io/valkey#3060)) - HSETEX - Always issue keyspace notifications after validation ([#3001](valkey-io/valkey#3001)) - Make zero a valid TTL for hash fields during import mode and data loading ([#3006](valkey-io/valkey#3006)) - Trigger prepareCommand on argc change in module command filters ([#2945](valkey-io/valkey#2945)) - Restrict TTL from being negative and avoid crash in import-mode ([#2944](valkey-io/valkey#2944)) - Fix chained replica crash when doing dual channel replication ([#2983](valkey-io/valkey#2983)) - Skip slot cache optimization for AOF client to prevent key duplication and data corruption ([#3004](valkey-io/valkey#3004)) - Fix used\_memory\_dataset underflow due to miscalculated used\_memory\_overhead ([#3005](valkey-io/valkey#3005)) - Avoid duplicate calculations of network-bytes-out in slot stats with copy-avoidance ([#3046](valkey-io/valkey#3046)) - Fix XREAD returning error on empty stream with + ID ([#2742](valkey-io/valkey#2742)) #### Performance/Efficiency Improvements - Track reply bytes in I/O threads if commandlog-reply-larger-than is -1 ([#3086](valkey-io/valkey#3086), [#3126](valkey-io/valkey#3126)). This makes it possible to mitigate a performance regression in 9.0.1 caused by the bug fix [#2652](valkey-io/valkey#2652). **Full Changelog**: <valkey-io/valkey@9.0.1...9.0.2> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - "before 6am" - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).

Track net bytes only if commandlog-request-larger-than != -1

a9d2c07

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

dvkashapov requested a review from zuiderkwast January 21, 2026 10:09

github-actions Bot assigned dvkashapov Jan 21, 2026

dvkashapov requested a review from enjoy-binbin January 21, 2026 10:18

dvkashapov added this to Valkey 9.0 and Valkey 9.1 Jan 21, 2026

dvkashapov moved this to To be backported in Valkey 9.0 Jan 21, 2026

dvkashapov moved this to In Progress in Valkey 9.1 Jan 21, 2026

Handle slot stats in releaseBufReferences

2591fed

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

zuiderkwast reviewed Jan 21, 2026

View reviewed changes

Comment thread src/server.h

Comment thread src/config.c Outdated

Per reply tracking

28aa85e

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

dvkashapov mentioned this pull request Jan 22, 2026

Add static assert for client flags #3096

Closed

dvkashapov force-pushed the dont-compute-bytes-without-commandlog branch from e7d7b11 to 28aa85e Compare January 22, 2026 07:38

zuiderkwast reviewed Jan 22, 2026

View reviewed changes

Comment thread src/networking.c Outdated

dvkashapov added 3 commits January 22, 2026 14:30

Pack bits

8363425

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

Format long lines

1f8b661

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

Add config comment

e31cee9

Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>

zuiderkwast approved these changes Jan 22, 2026

View reviewed changes

zuiderkwast added the release-notes This issue should get a line item in the release notes label Jan 22, 2026

enjoy-binbin approved these changes Jan 22, 2026

View reviewed changes

roshkhatri approved these changes Jan 22, 2026

View reviewed changes

zuiderkwast merged commit 985cf83 into valkey-io:unstable Jan 26, 2026
57 checks passed

github-project-automation Bot moved this from In Progress to Done in Valkey 9.1 Jan 26, 2026

dvkashapov deleted the dont-compute-bytes-without-commandlog branch January 27, 2026 11:00

dvkashapov mentioned this pull request Jan 29, 2026

Fix mixup of reply-larger-than and request-larger-than #3126

Merged

zuiderkwast moved this from To be backported to 9.0.2 WIP in Valkey 9.0 Jan 30, 2026

enjoy-binbin mentioned this pull request Apr 15, 2026

[Performance] Potential regression of 25% from commandlog large-reply when using reply copy avoidance #2926

Open

dvkashapov mentioned this pull request May 8, 2026

Change default of commandlog-reply-larger-than to disable tracking #3646

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Perf: Track net bytes only if commandlog-request-larger-than != -1#3086

Perf: Track net bytes only if commandlog-request-larger-than != -1#3086
zuiderkwast merged 6 commits into
valkey-io:unstablefrom
dvkashapov:dont-compute-bytes-without-commandlog

dvkashapov commented Jan 21, 2026 •

edited by enjoy-binbin

Loading

Uh oh!

codecov Bot commented Jan 21, 2026 •

edited

Loading

Uh oh!

zuiderkwast left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuiderkwast left a comment

Uh oh!

zuiderkwast commented Jan 22, 2026

Uh oh!

enjoy-binbin left a comment

Uh oh!

roshkhatri left a comment •

edited

Loading

Uh oh!

zuiderkwast commented Jan 22, 2026

Uh oh!

roshkhatri commented Jan 22, 2026

Uh oh!

zuiderkwast commented Jan 22, 2026

Uh oh!

Uh oh!

roshkhatri commented Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

dvkashapov commented Jan 21, 2026 • edited by enjoy-binbin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

zuiderkwast commented Jan 22, 2026

Uh oh!

enjoy-binbin left a comment

Choose a reason for hiding this comment

Uh oh!

roshkhatri left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuiderkwast commented Jan 22, 2026

Uh oh!

roshkhatri commented Jan 22, 2026

Uh oh!

zuiderkwast commented Jan 22, 2026

Uh oh!

Uh oh!

roshkhatri commented Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dvkashapov commented Jan 21, 2026 •

edited by enjoy-binbin

Loading

codecov Bot commented Jan 21, 2026 •

edited

Loading

roshkhatri left a comment •

edited

Loading