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

[dvkashapov]

Change default of commandlog-reply-larger-than config to -1 because enabling tracking noticeably reduces performance with copy avoidance feature enabled. This is a breaking change and should be included only in major version.
Related Issue/PRs with more details about reasoning and benchmarks:
#2926 #2652 #3086

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.67%. Comparing base (1d7224f) to head (c7cf914).
⚠️ Report is 1 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3646      +/-   ##
============================================
+ Coverage     76.66%   76.67%   +0.01%     
============================================
  Files           162      162              
  Lines         80644    80647       +3     
============================================
+ Hits          61823    61835      +12     
+ Misses        18821    18812       -9     

Files with missing lines	Coverage Δ
src/config.c	78.09% <ø> (ø)

... and 20 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.

[sarthakaggarwal97]

Yes, this is a good idea to do it in 10!

[soloestoy]

I think this PR is more of a workaround than a root fix.

The fundamental issue lies in the copy avoidance implementation itself — it enqueues robj* pointers into the iovec without preserving the reply size as metadata.At enqueue time, the main thread already holds a reference to the robj. The SDS length is readily available in the SDS header (sdslen() is just a field read from the header). If the copy avoidance path recorded the reply size into a per-command accumulator (or as metadata alongside the iovec entry) at the moment it enqueues the pointer, then commandlog-reply-larger-than tracking could simply consume that already-cached value — no post-hoc dereference, no extra cache miss on the hot path.

In other words, the conflict is not inherent between copy avoidance and commandlog tracking. It was introduced by #2652's choice to retroactively compute reply size after the fact, instead of having the copy avoidance path supply that information as part of its own enqueue contract. Disabling tracking by default sidesteps the symptom but leaves the architectural mismatch in place: any future feature that needs reply size on the fast path (slot stats, client memory accounting in #3306, latency tracking, etc.) will keep running into the same trade-off.

Since this change is already scoped to the next major version (Valkey 10), there is plenty of runway to address the root cause rather than ship a workaround. I'd suggest using this window to refactor the copy avoidance path so it carries reply size metadata at enqueue time. That way both features can coexist without performance trade-off, observability stays on by default, and downstream consumers of reply size won't need their own ad-hoc accounting.

[dvkashapov]

In my initial investigation of mitigating #2652 I've tried including sdslen in bulkStrRef, looks like exactly what you're describing, but it was not beneficial on x86 and did not provide any performance benefits, I suppose because of frequent cache misses on it and that's why I suggested just changing observability by default.

/* To avoid copy of whole string in reply buffer
 * we store pointers to object and string itself */
typedef struct __attribute__((__packed__)) bulkStrRef {
    robj *obj; /* pointer to object used for reference count management */
    sds str;   /* pointer to string to optimize memory access by I/O thread */
} bulkStrRef;