mirrors/dragonfly
1
0
Fork 0
mirror of https://github.com/dragonflydb/dragonfly synced 2024-11-21 23:19:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: skip Send() in SinkReplyBuilder::Flush() when vec is empty (#4114)

Browse Source 
* skip Send() in SinkReplyBuilder::Flush() when vec is empty

Signed-off-by: kostas <kostas@dragonflydb.io>
This commit is contained in:
Kostas Kyrimis 2024-11-11 19:53:30 +02:00 committed by GitHub
parent 3ab244616a
commit 0facd6fd8f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 24 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/facade/reply_builder.cc
View File

@ -127,7 +127,8 @@ void SinkReplyBuilder::WriteRef(std::string_view str) {
}
void SinkReplyBuilder::Flush(size_t expected_buffer_cap) {
Send();
if (!vecs_.empty())
Send();
// Grow backing buffer if was at least half full and still below it's max size
if (buffer_.InputLen() * 2 > buffer_.Capacity() && buffer_.Capacity() * 2 <= kMaxBufferSize)
@ -144,6 +145,8 @@ void SinkReplyBuilder::Flush(size_t expected_buffer_cap) {
}
void SinkReplyBuilder::Send() {
DCHECK(sink_ != nullptr);
DCHECK(!vecs_.empty());
auto& reply_stats = tl_facade_stats->reply_stats;
send_active_ = true;

21
src/server/string_family.cc
View File

@ -1313,9 +1313,28 @@ void StringFamily::MGet(CmdArgList args, Transaction* tx, SinkReplyBuilder* buil
}
}
// The code below is safe in the context of squashing (uses CapturingReplyBuilder).
// Specifically:
// 1. For Memcache:
// builder != CapturingReplyBuilder here because this is only used in squashing
// and there are only two cases:
// * Squashing the pipeline something that is turned off when using MEMCACHE.
// * Squashing a multi/exec block. There exist no such command in MEMCACHE.
// Therefore this path is safe, and the DCHECK in the if statement below shall
// never trigger.
// 2. For Redis:
// * Call to StartArray() is safe because it calls RedisReplyBuilder::StartCollection which
// calls CapturingReplyBuilder::StartCollection
// * Calls to SendBulkString() and SendNull() find and if builder is CapturingReplyBuilder
// then the right member gets called.
//
// Finally, the ReplyScope will trigger a Flush() on scope's end. What that means is,
// for CapturingReplyBuilder the internal vec is empty and therefore we should skip the call
// to Send because sink_ is nullptr and there is no payload to Send since it was captured.
SinkReplyBuilder::ReplyScope scope(builder);
if (builder->GetProtocol() == Protocol::MEMCACHE) {
auto* rb = static_cast<MCReplyBuilder*>(builder);
DCHECK(dynamic_cast<CapturingReplyBuilder*>(builder) == nullptr);
for (const auto& entry : res) {
if (!entry)
continue;
@ -1588,7 +1607,7 @@ void StringFamily::Register(CommandRegistry* registry) {
<< CI{"SUBSTR", CO::READONLY, 4, 1, 1}.HFUNC(GetRange) // Alias for GetRange
<< CI{"SETRANGE", CO::WRITE | CO::DENYOOM, 4, 1, 1}.HFUNC(SetRange)
<< CI{"CL.THROTTLE", CO::WRITE | CO::DENYOOM | CO::FAST, -5, 1, 1, acl::THROTTLE}.HFUNC(
ClThrottle);
ClThrottle);
}
} // namespace dfly