Skip to content

Fix commandlog large-reply when using reply copy avoidance #2652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from
Open

Fix commandlog large-reply when using reply copy avoidance #2652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e292ab
Fix commandlog large-reply when using reply copy avoidance
enjoy-binbin Sep 26, 2025
bf86728
Fix test
enjoy-binbin Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/networking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,17 @@ static int tryAvoidBulkStrCopyToReply(client *c, robj *obj) {

/* Add an Object as a bulk reply */
void addReplyBulk(client *c, robj *obj) {
if (tryAvoidBulkStrCopyToReply(c, obj) == C_OK) return;
if (tryAvoidBulkStrCopyToReply(c, obj) == C_OK) {
/* If copy avoidance allowed, then we explicitly maintain net_output_bytes_curr_cmd. */
serverAssert(obj->encoding == OBJ_ENCODING_RAW);
size_t str_len = sdslen(obj->ptr);
uint32_t num_len = digits10(str_len);
/* RESP encodes bulk strings as $<length>\r\n<data>\r\n */
c->net_output_bytes_curr_cmd += (num_len + 3); /* $<length>\r\n */
c->net_output_bytes_curr_cmd += str_len; /* <data> */
c->net_output_bytes_curr_cmd += 2; /* \r\n */
return;
}
addReplyBulkLen(c, obj);
addReply(c, obj);
addReplyProto(c, "\r\n", 2);
Expand Down
25 changes: 19 additions & 6 deletions tests/unit/commandlog.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,11 @@ start_server {tags {"commandlog"} overrides {commandlog-execution-slower-than 10
assert_equal [lindex $e 3] {set testkey {AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA... (896 more bytes)}}
assert_equal {foobar} [lindex $e 5]

# for large-reply
set copy_avoid [lindex [r config get min-io-threads-avoid-copy-reply] 1]
r config set min-io-threads-avoid-copy-reply 0

# for large-reply - without reply copy avoidance
set copy_avoid [lindex [r config get min-string-size-avoid-copy-reply] 1]
if {!$::external} {
assert_morethan $copy_avoid 1024
}
r get testkey
set e [lindex [r commandlog get -1 large-reply] 0]
assert_equal [llength $e] 6
Expand All @@ -132,8 +133,20 @@ start_server {tags {"commandlog"} overrides {commandlog-execution-slower-than 10
assert_equal [lindex $e 3] {get testkey}
assert_equal {foobar} [lindex $e 5]

# Restore min-io-threads-avoid-copy-reply value
r config set min-io-threads-avoid-copy-reply $copy_avoid
# for large-reply - with reply copy avoidance
# set min-string-size-avoid-copy-reply to 1 so wo will use reply copy avoidance
r config set min-string-size-avoid-copy-reply 1
r get testkey
set e [lindex [r commandlog get -1 large-reply] 0]
assert_equal [llength $e] 6
if {!$::external} {
assert_equal [lindex $e 0] 118
}
assert_equal [expr {[lindex $e 2] > 1024}] 1
assert_equal [lindex $e 3] {get testkey}
assert_equal {foobar} [lindex $e 5]
# Restore min-string-size-avoid-copy-reply value
r config set min-string-size-avoid-copy-reply $copy_avoid
} {OK} {needs:debug}

test {COMMANDLOG slow - Certain commands are omitted that contain sensitive information} {
Expand Down
Loading