From 7445b205480a037cad58720db480db551043bb6b Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Thu, 24 Feb 2022 17:14:03 +0200 Subject: [PATCH] More work on memcached API 1. Fix memcached ttl semantics. 2. For incr/decr commands if a key does not exist - return NOT_FOUND. --- server/dragonfly_connection.cc | 2 +- server/main_service.cc | 9 +++++++++ server/reply_builder.cc | 4 ++++ server/reply_builder.h | 1 + server/string_family.cc | 11 +++++++++-- server/string_family.h | 5 ++++- 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/server/dragonfly_connection.cc b/server/dragonfly_connection.cc index eb2ceed..ddcb3c1 100644 --- a/server/dragonfly_connection.cc +++ b/server/dragonfly_connection.cc @@ -82,7 +82,7 @@ struct Connection::Shutdown { struct Connection::Request { absl::FixedArray args; - // I do not use m_heap_t explicitly but mi_stl_allocator at the end does the same job + // I do not use mi_heap_t explicitly but mi_stl_allocator at the end does the same job // of using the thread's heap. absl::FixedArray> storage; diff --git a/server/main_service.cc b/server/main_service.cc index 7416999..7d5f825 100644 --- a/server/main_service.cc +++ b/server/main_service.cc @@ -471,9 +471,12 @@ void Service::DispatchMC(const MemcacheParser::Command& cmd, std::string_view va ConnectionContext* cntx) { absl::InlinedVector args; char cmd_name[16]; + char ttl[16]; char store_opt[32] = {0}; + char ttl_op[] = "EX"; MCReplyBuilder* mc_builder = static_cast(cntx->reply_builder()); + switch (cmd.type) { case MemcacheParser::REPLACE: strcpy(cmd_name, "SET"); @@ -529,6 +532,12 @@ void Service::DispatchMC(const MemcacheParser::Command& cmd, std::string_view va if (store_opt[0]) { args.emplace_back(store_opt, strlen(store_opt)); } + + if (cmd.expire_ts && memcmp(cmd_name, "SET", 3) == 0) { + char* next = absl::numbers_internal::FastIntToBuffer(cmd.expire_ts, ttl); + args.emplace_back(ttl_op, 2); + args.emplace_back(ttl, next - ttl); + } cntx->conn_state.memcache_flag = cmd.flags; } else if (cmd.type < MemcacheParser::QUIT) { // read commands for (auto s : cmd.keys_ext) { diff --git a/server/reply_builder.cc b/server/reply_builder.cc index 093fa59..6479b9d 100644 --- a/server/reply_builder.cc +++ b/server/reply_builder.cc @@ -125,6 +125,10 @@ void MCReplyBuilder::SendSetSkipped() { SendDirect("NOT_STORED\r\n"); } +void MCReplyBuilder::SendNotFound() { + SendDirect("NOT_FOUND\r\n"); +} + RedisReplyBuilder::RedisReplyBuilder(::io::Sink* sink) : SinkReplyBuilder(sink) { } diff --git a/server/reply_builder.h b/server/reply_builder.h index 146aeca..f1999fe 100644 --- a/server/reply_builder.h +++ b/server/reply_builder.h @@ -99,6 +99,7 @@ class MCReplyBuilder : public SinkReplyBuilder { void SendSetSkipped() final; void SendClientError(std::string_view str); + void SendNotFound(); }; class RedisReplyBuilder : public SinkReplyBuilder { diff --git a/server/string_family.cc b/server/string_family.cc index 44cafc6..3e085a6 100644 --- a/server/string_family.cc +++ b/server/string_family.cc @@ -263,8 +263,10 @@ void StringFamily::Prepend(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::IncrByGeneric(std::string_view key, int64_t val, ConnectionContext* cntx) { + bool skip_on_missing = cntx->protocol() == Protocol::MEMCACHE; + auto cb = [&](Transaction* t, EngineShard* shard) { - OpResult res = OpIncrBy(OpArgs{shard, t->db_index()}, key, val); + OpResult res = OpIncrBy(OpArgs{shard, t->db_index()}, key, val, skip_on_missing); return res; }; @@ -281,6 +283,8 @@ void StringFamily::IncrByGeneric(std::string_view key, int64_t val, ConnectionCo return builder->SendError("increment or decrement would overflow"); case OpStatus::WRONG_TYPE: return builder->SendError(kWrongTypeErr); + case OpStatus::KEY_NOTFOUND: // Relevant only for MC + return reinterpret_cast(builder)->SendNotFound(); default:; } __builtin_unreachable(); @@ -432,11 +436,14 @@ OpStatus StringFamily::OpMSet(const Transaction* t, EngineShard* es) { } OpResult StringFamily::OpIncrBy(const OpArgs& op_args, std::string_view key, - int64_t incr) { + int64_t incr, bool skip_on_missing) { auto& db_slice = op_args.shard->db_slice(); auto [it, expire_it] = db_slice.FindExt(op_args.db_ind, key); if (!IsValid(it)) { + if (skip_on_missing) + return OpStatus::KEY_NOTFOUND; + CompactObj cobj; cobj.SetInt(incr); diff --git a/server/string_family.h b/server/string_family.h index 11426c1..0b381b6 100644 --- a/server/string_family.h +++ b/server/string_family.h @@ -73,7 +73,10 @@ class StringFamily { EngineShard* shard); static OpStatus OpMSet(const Transaction* t, EngineShard* es); - static OpResult OpIncrBy(const OpArgs& op_args, std::string_view key, int64_t val); + + // if skip_on_missing - returns KEY_NOTFOUND. + static OpResult OpIncrBy(const OpArgs& op_args, std::string_view key, int64_t val, + bool skip_on_missing); // Returns the length of the extended string. if prepend is false - appends the val. static OpResult ExtendOrSet(const OpArgs& op_args, std::string_view key,