[kvrocks] branch unstable updated: Use LMOVE logic to handle RPOPLPUSH (#1596)

[hulk]

This is an automated email from the ASF dual-hosted git repository.

hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new f48c02f2 Use LMOVE logic to handle RPOPLPUSH (#1596)
f48c02f2 is described below

commit f48c02f292fe3c4fc9f505ec507dbdb6e62b95ff
Author: Binbin <binloveplay1...@qq.com>
AuthorDate: Wed Jul 19 22:36:05 2023 +0800

    Use LMOVE logic to handle RPOPLPUSH (#1596)
    
    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 fd025ce5..9f178765 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], /*src_left=*/false, 
/*dst_left=*/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 fe764c11..4ac6b6b9 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<Slice> 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 dbc86752..2cea147d 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<std::string> *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<Slice> &elems, 
bool left, uint64_t *new_size);
   rocksdb::Status PushX(const Slice &user_key, const std::vector<Slice> 
&elems, bool left, uint64_t *new_size);
diff --git a/tests/cppunit/types/list_test.cc b/tests/cppunit/types/list_test.cc
index 15c25201..c723621c 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);