From 20336805f3669110d84498deddb76419d8153cce Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Thu, 29 Aug 2024 23:30:26 +0300 Subject: [PATCH] chore: enable experimental_new_io by default. (#3605) * chore: enable experimental_new_io by default. It has been running for weeks with the flag on, so enabled it also for community. --------- Signed-off-by: Roman Gershman Signed-off-by: Vladislav Oleshko Co-authored-by: Vladislav Oleshko --- .github/workflows/ci.yml | 2 +- src/facade/conn_context.cc | 9 ++++++--- src/facade/reply_builder.cc | 4 ++-- src/facade/reply_builder.h | 4 ++-- src/server/hset_family.cc | 5 ++--- tests/dragonfly/instance.py | 3 --- 6 files changed, 13 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0e2b04df..a1a3ed233 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -142,7 +142,7 @@ jobs: EOF gdb -ix ./init.gdb --batch -ex r --args ./dragonfly_test --force_epoll - DFLY_use_new_io=true FLAGS_force_epoll=true GLOG_vmodule=rdb_load=1,rdb_save=1,snapshot=1 timeout 20m ctest -V -L DFLY + FLAGS_force_epoll=true GLOG_vmodule=rdb_load=1,rdb_save=1,snapshot=1 timeout 20m ctest -V -L DFLY echo "Finished running tests with --force_epoll" diff --git a/src/facade/conn_context.cc b/src/facade/conn_context.cc index 6049231af..e55a51ed8 100644 --- a/src/facade/conn_context.cc +++ b/src/facade/conn_context.cc @@ -9,7 +9,9 @@ #include "facade/dragonfly_connection.h" #include "facade/reply_builder.h" -ABSL_FLAG(bool, use_new_io, false, "Use new IO by default"); +ABSL_FLAG(bool, experimental_new_io, true, + "Use new replying code - should " + "reduce latencies for pipelining"); namespace facade { @@ -21,8 +23,9 @@ ConnectionContext::ConnectionContext(::io::Sink* stream, Connection* owner) : ow if (stream) { switch (protocol_) { case Protocol::REDIS: { - RedisReplyBuilder* rb = absl::GetFlag(FLAGS_use_new_io) ? new RedisReplyBuilder2(stream) - : new RedisReplyBuilder(stream); + RedisReplyBuilder* rb = absl::GetFlag(FLAGS_experimental_new_io) + ? new RedisReplyBuilder2(stream) + : new RedisReplyBuilder(stream); rbuilder_.reset(rb); break; } diff --git a/src/facade/reply_builder.cc b/src/facade/reply_builder.cc index 884d33093..95b9ba549 100644 --- a/src/facade/reply_builder.cc +++ b/src/facade/reply_builder.cc @@ -881,7 +881,7 @@ void RedisReplyBuilder2Base::SendLong(long val) { } void RedisReplyBuilder2Base::SendDouble(double val) { - char buf[DoubleToStringConverter::kBase10MaximalLength + 1]; + char buf[DoubleToStringConverter::kBase10MaximalLength + 8]; // +8 to be on the safe side. static_assert(ABSL_ARRAYSIZE(buf) < kMaxInlineSize, "Write temporary string from buf inline"); string_view val_str = FormatDouble(val, buf, ABSL_ARRAYSIZE(buf)); @@ -983,7 +983,7 @@ void RedisReplyBuilder2::SendStored() { } void RedisReplyBuilder2::SendSetSkipped() { - SendSimpleString("SKIPPED"); + SendNull(); } void RedisReplyBuilder2::StartArray(unsigned len) { diff --git a/src/facade/reply_builder.h b/src/facade/reply_builder.h index 2afa51400..f65e72405 100644 --- a/src/facade/reply_builder.h +++ b/src/facade/reply_builder.h @@ -347,7 +347,7 @@ class RedisReplyBuilder : public SinkReplyBuilder { RedisReplyBuilder(::io::Sink* stream); virtual void SetResp3(bool is_resp3); - bool IsResp3() const { + virtual bool IsResp3() const { return is_resp3_; } @@ -418,7 +418,7 @@ class RedisReplyBuilder2Base : public SinkReplyBuilder2, public RedisReplyBuilde static char* FormatDouble(double d, char* dest, unsigned len); virtual void SendVerbatimString(std::string_view str, VerbatimFormat format = TXT) override; - bool IsResp3() const { + bool IsResp3() const override { return resp3_; } diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc index dd22870b4..0883d4f1c 100644 --- a/src/server/hset_family.cc +++ b/src/server/hset_family.cc @@ -1156,9 +1156,8 @@ void HSetFamily::HRandField(CmdArgList args, ConnectionContext* cntx) { if (result) { if ((result->size() == 1) && (args.size() == 1)) rb->SendBulkString(result->front()); - else { - rb->SendStringArr(*result, facade::RedisReplyBuilder::MAP); - } + else + rb->SendStringArr(*result, facade::RedisReplyBuilder::ARRAY); } else if (result.status() == OpStatus::KEY_NOTFOUND) { if (args.size() == 1) rb->SendNull(); diff --git a/tests/dragonfly/instance.py b/tests/dragonfly/instance.py index 4d316cff6..da1775937 100644 --- a/tests/dragonfly/instance.py +++ b/tests/dragonfly/instance.py @@ -349,9 +349,6 @@ class DflyInstanceFactory: # Add 1 byte limit for big values args.setdefault("serialization_max_chunk_size", 1) - if version > 1.21: - args.setdefault("use_new_io") - for k, v in args.items(): args[k] = v.format(**self.params.env) if isinstance(v, str) else v