From 389e1f73c49af478151d43067e368432618515bc Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 18 Jul 2023 14:15:49 +0800 Subject: [PATCH 1/2] Use LMOVE logic to handle RPOPLPUSH RPOPLPUSH is actually LMOVE RIGHT LEFT, we can use list_db.LMove to replace List::RPopLPush. In this way we can enjoy the multi locks guard and operate multi keys atomically. --- src/commands/cmd_list.cc | 2 +- src/types/redis_list.cc | 18 ------------------ src/types/redis_list.h | 1 - tests/cppunit/types/list_test.cc | 19 ------------------- 4 files changed, 1 insertion(+), 39 deletions(-) diff --git a/src/commands/cmd_list.cc b/src/commands/cmd_list.cc index fd025ce5fdb..1940e38b39a 100644 --- a/src/commands/cmd_list.cc +++ b/src/commands/cmd_list.cc @@ -508,7 +508,7 @@ class CommandRPopLPUSH : public Commander { Status Execute(Server *svr, Connection *conn, std::string *output) override { redis::List list_db(svr->storage, conn->GetNamespace()); std::string elem; - auto s = list_db.RPopLPush(args_[1], args_[2], &elem); + auto s = list_db.LMove(args_[1], args_[2], false, true, &elem); if (!s.ok() && !s.IsNotFound()) { return {Status::RedisExecErr, s.ToString()}; } diff --git a/src/types/redis_list.cc b/src/types/redis_list.cc index fe764c11d18..4ac6b6b9ed6 100644 --- a/src/types/redis_list.cc +++ b/src/types/redis_list.cc @@ -433,24 +433,6 @@ rocksdb::Status List::Set(const Slice &user_key, int index, Slice elem) { return storage_->Write(storage_->DefaultWriteOptions(), batch->GetWriteBatch()); } -rocksdb::Status List::RPopLPush(const Slice &src, const Slice &dst, std::string *elem) { - RedisType type = kRedisNone; - rocksdb::Status s = Type(dst, &type); - if (!s.ok()) return s; - if (type != kRedisNone && type != kRedisList) { - return rocksdb::Status::InvalidArgument(kErrMsgWrongType); - } - - s = Pop(src, false, elem); - if (!s.ok()) return s; - - uint64_t ret = 0; - std::vector elems; - elems.emplace_back(*elem); - s = Push(dst, elems, true, &ret); - return s; -} - rocksdb::Status List::LMove(const rocksdb::Slice &src, const rocksdb::Slice &dst, bool src_left, bool dst_left, std::string *elem) { if (src == dst) { diff --git a/src/types/redis_list.h b/src/types/redis_list.h index dbc86752cbe..2cea147da60 100644 --- a/src/types/redis_list.h +++ b/src/types/redis_list.h @@ -41,7 +41,6 @@ class List : public Database { rocksdb::Status PopMulti(const Slice &user_key, bool left, uint32_t count, std::vector *elems); rocksdb::Status Rem(const Slice &user_key, int count, const Slice &elem, uint64_t *removed_cnt); rocksdb::Status Index(const Slice &user_key, int index, std::string *elem); - rocksdb::Status RPopLPush(const Slice &src, const Slice &dst, std::string *elem); rocksdb::Status LMove(const Slice &src, const Slice &dst, bool src_left, bool dst_left, std::string *elem); rocksdb::Status Push(const Slice &user_key, const std::vector &elems, bool left, uint64_t *new_size); rocksdb::Status PushX(const Slice &user_key, const std::vector &elems, bool left, uint64_t *new_size); diff --git a/tests/cppunit/types/list_test.cc b/tests/cppunit/types/list_test.cc index 15c25201526..c723621c5fc 100644 --- a/tests/cppunit/types/list_test.cc +++ b/tests/cppunit/types/list_test.cc @@ -314,25 +314,6 @@ TEST_F(RedisListSpecificTest, Trim) { list_->Del(key_); } -TEST_F(RedisListTest, RPopLPush) { - uint64_t ret = 0; - list_->Push(key_, fields_, true, &ret); - EXPECT_EQ(fields_.size(), ret); - Slice dst("test-list-rpoplpush-key"); - for (auto &field : fields_) { - std::string elem; - list_->RPopLPush(key_, dst, &elem); - EXPECT_EQ(field.ToString(), elem); - } - for (auto &field : fields_) { - std::string elem; - list_->Pop(dst, false, &elem); - EXPECT_EQ(elem, field.ToString()); - } - list_->Del(key_); - list_->Del(dst); -} - TEST_F(RedisListLMoveTest, LMoveSrcNotExist) { std::string elem; auto s = list_->LMove(key_, dst_key_, true, true, &elem); From adae0942639267a218e8ccb4b082d8e643add029 Mon Sep 17 00:00:00 2001 From: hulk Date: Wed, 19 Jul 2023 12:16:49 +0800 Subject: [PATCH 2/2] Update src/commands/cmd_list.cc Co-authored-by: mwish <1506118561@qq.com> --- src/commands/cmd_list.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/cmd_list.cc b/src/commands/cmd_list.cc index 1940e38b39a..9f178765ef2 100644 --- a/src/commands/cmd_list.cc +++ b/src/commands/cmd_list.cc @@ -508,7 +508,7 @@ class CommandRPopLPUSH : public Commander { Status Execute(Server *svr, Connection *conn, std::string *output) override { redis::List list_db(svr->storage, conn->GetNamespace()); std::string elem; - auto s = list_db.LMove(args_[1], args_[2], false, true, &elem); + auto s = list_db.LMove(args_[1], args_[2], /*src_left=*/false, /*dst_left=*/true, &elem); if (!s.ok() && !s.IsNotFound()) { return {Status::RedisExecErr, s.ToString()}; }