You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by hu...@apache.org on 2022/09/28 05:24:10 UTC

[incubator-kvrocks] branch unstable updated: Support EXAT/PEXAT option in the set command (#901)

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/incubator-kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new b94ea81  Support EXAT/PEXAT option in the set command (#901)
b94ea81 is described below

commit b94ea81a88dbc4c8dc066032856b769dbcbf3283
Author: CHENG PENG <im...@163.com>
AuthorDate: Wed Sep 28 13:24:04 2022 +0800

    Support EXAT/PEXAT option in the set command (#901)
---
 src/redis_cmd.cc                            | 43 ++++++++++++++++----
 src/slot_migrate.cc                         | 11 ++---
 tests/tcl/tests/integration/slotmigrate.tcl |  8 ++++
 tests/tcl/tests/unit/type/string.tcl        | 63 +++++++++++++++++++++++------
 4 files changed, 100 insertions(+), 25 deletions(-)

diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc
index 9e6d177..febe5e9 100644
--- a/src/redis_cmd.cc
+++ b/src/redis_cmd.cc
@@ -477,17 +477,31 @@ class CommandSet : public Commander {
       } else if (opt == "xx" && !nx_) {
         xx_ = true;
       } else if (opt == "ex" && !ttl_ && !last_arg) {
-        try {
-          std::string s = args_[++i];
-          std::string::size_type sz;
-          ttl_ = std::stoi(s, &sz);
-          if (sz != s.size()) {
-            return Status(Status::RedisParseErr, errValueNotInterger);
-          }
-        } catch (std::exception &e) {
+        auto parse_result = ParseInt<int>(args_[++i], 10);
+        if (!parse_result) {
           return Status(Status::RedisParseErr, errValueNotInterger);
         }
+        ttl_ = *parse_result;
         if (ttl_ <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
+      } else if (opt == "exat" && !ttl_ && !expire_ && !last_arg) {
+        auto parse_result = ParseInt<int64_t>(args_[++i], 10);
+        if (!parse_result) {
+          return Status(Status::RedisParseErr, errValueNotInterger);
+        }
+        expire_ = *parse_result;
+        if (expire_ <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
+      } else if (opt == "pxat" && !ttl_ && !expire_ && !last_arg) {
+        auto parse_result = ParseInt<uint64_t>(args[++i], 10);
+        if (!parse_result) {
+          return Status(Status::RedisParseErr, errValueNotInterger);
+        }
+        uint64_t expire_ms = *parse_result;
+        if (expire_ms <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
+        if (expire_ms < 1000) {
+          expire_ = 1;
+        } else {
+          expire_ = static_cast<int64_t>(expire_ms/1000);
+        }
       } else if (opt == "px" && !ttl_ && !last_arg) {
         int64_t ttl_ms = 0;
         std::string s = args_[++i];
@@ -512,6 +526,18 @@ class CommandSet : public Commander {
     int ret;
     Redis::String string_db(svr->storage_, conn->GetNamespace());
     rocksdb::Status s;
+
+    if (!ttl_ && expire_) {
+      int64_t now;
+      rocksdb::Env::Default()->GetCurrentTime(&now);
+      ttl_ = expire_ - now;
+      if (ttl_ <= 0) {
+        string_db.Del(args_[1]);
+        *output = Redis::SimpleString("OK");
+        return Status::OK();
+      }
+    }
+
     if (nx_) {
       s = string_db.SetNX(args_[1], args_[2], ttl_, &ret);
     } else if (xx_) {
@@ -534,6 +560,7 @@ class CommandSet : public Commander {
   bool xx_ = false;
   bool nx_ = false;
   int ttl_ = 0;
+  int64_t expire_ = 0;
 };
 
 class CommandSetEX : public Commander {
diff --git a/src/slot_migrate.cc b/src/slot_migrate.cc
index a65a3cb..c525710 100644
--- a/src/slot_migrate.cc
+++ b/src/slot_migrate.cc
@@ -554,6 +554,10 @@ Status SlotMigrate::MigrateOneKey(const rocksdb::Slice &key, const rocksdb::Slic
     return Status(Status::cOK, "empty");
   }
 
+  if (metadata.Expired()) {
+    return Status(Status::cOK, "expired");
+  }
+
   // Construct command according to type of the key
   switch (metadata.Type()) {
     case kRedisString: {
@@ -587,11 +591,8 @@ bool SlotMigrate::MigrateSimpleKey(const rocksdb::Slice &key, const Metadata &me
                                    const std::string &bytes, std::string *restore_cmds) {
   std::vector<std::string> command = {"set", key.ToString(), bytes.substr(5, bytes.size() - 5)};
   if (metadata.expire > 0) {
-    int64_t now;
-    rocksdb::Env::Default()->GetCurrentTime(&now);
-    int32_t ttl = metadata.expire - uint32_t(now);
-    command.emplace_back("EX");
-    command.emplace_back(std::to_string(ttl));
+    command.emplace_back("EXAT");
+    command.emplace_back(std::to_string(metadata.expire));
   }
   *restore_cmds += Redis::MultiBulkString(command, false);
   current_pipeline_size_++;
diff --git a/tests/tcl/tests/integration/slotmigrate.tcl b/tests/tcl/tests/integration/slotmigrate.tcl
index f9d805d..4d36fb6 100644
--- a/tests/tcl/tests/integration/slotmigrate.tcl
+++ b/tests/tcl/tests/integration/slotmigrate.tcl
@@ -147,6 +147,7 @@ start_server {tags {"Src migration server"} overrides {cluster-enabled yes}} {
             # Set keys
             set slot1_tag [lindex $::CRC16_SLOT_TABLE 1]
             set slot1_key_string string_{$slot1_tag}
+            set slot1_key_string2 string2_{$slot1_tag}
             set slot1_key_list list_{$slot1_tag}
             set slot1_key_hash hash_{$slot1_tag}
             set slot1_key_set set_{$slot1_tag}
@@ -168,6 +169,11 @@ start_server {tags {"Src migration server"} overrides {cluster-enabled yes}} {
             $r0 set $slot1_key_string $slot1_key_string
             $r0 expire  $slot1_key_string 10000
 
+            # Expired string key
+            $r0 set $slot1_key_string2 $slot1_key_string2 ex 1
+            after 3000
+            assert_equal [$r0 get $slot1_key_string2] {}
+
             # Type: list
             $r0 rpush $slot1_key_list 0 1 2 3 4 5
             $r0 lpush $slot1_key_list 9 3 7 3 5 4
@@ -242,6 +248,8 @@ start_server {tags {"Src migration server"} overrides {cluster-enabled yes}} {
             set expire_time [$r1 ttl $slot1_key_string]
             assert {$expire_time > 1000 && $expire_time <= 10000}
 
+            assert_equal [$r1 get $slot1_key_string2] {}
+
             # Check list and expired time
             assert {[$r1 lrange $slot1_key_list 0 -1] eq $lvalue}
             set expire_time [$r1 ttl $slot1_key_list]
diff --git a/tests/tcl/tests/unit/type/string.tcl b/tests/tcl/tests/unit/type/string.tcl
index 7db12af..4f51e0b 100644
--- a/tests/tcl/tests/unit/type/string.tcl
+++ b/tests/tcl/tests/unit/type/string.tcl
@@ -556,17 +556,41 @@ start_server {tags {"string"}} {
         assert {$ttl <= 10 && $ttl > 5}
     }
 
-    # test "Extended SET EXAT option" {
-    #     r del foo
-    #     r set foo bar exat [expr [clock seconds] + 10]
-    #     assert_range [r ttl foo] 5 10
-    # }
+    test {Extended SET EXAT option} {
+        r del foo
+        r set foo bar exat [expr [clock seconds] + 10]
+        assert_range [r ttl foo] 5 10
+    }
 
-    # test "Extended SET PXAT option" {
-    #     r del foo
-    #     r set foo bar pxat [expr [clock milliseconds] + 10000]
-    #     assert_range [r ttl foo] 5 10
-    # }
+    test {Extended SET EXAT option with expired timestamp} {
+        r del foo
+        assert_equal [r set foo bar exat 1] OK
+        assert_equal [r get foo] {}
+
+        r set foo bar
+        assert_equal [r get foo] bar
+
+        assert_equal [r set foo bar exat [expr [clock seconds] - 5]] OK
+        assert_equal [r get foo] {}
+    }
+
+    test {Extended SET PXAT option} {
+        r del foo
+        r set foo bar pxat [expr [clock milliseconds] + 10000]
+        assert_range [r ttl foo] 5 10
+    }
+
+    test {Extended SET PXAT option with expired timestamp} {
+        r del foo
+        assert_equal [r set foo bar pxat 1] OK
+        assert_equal [r get foo] {}
+
+        r set foo bar
+        assert_equal [r get foo] bar
+
+        assert_equal [r set foo bar pxat [expr [clock milliseconds] - 5000]] OK
+        assert_equal [r get foo] {}
+    }
 
     test {Extended SET with incorrect use of multi options should result in syntax err} {
       catch {r set foo bar ex 10 px 10000} err1
@@ -576,8 +600,23 @@ start_server {tags {"string"}} {
 
     test {Extended SET with incorrect expire value} {
         catch {r set foo bar ex 1234xyz} e
-        set e
-    } {*not an integer*}
+        assert_match {*not an integer*} $e
+
+        catch {r set foo bar ex 0} e
+        assert_match {*invalid expire time*} $e
+
+        catch {r set foo bar exat 1234xyz} e
+        assert_match {*not an integer*} $e
+
+        catch {r set foo bar exat 0} e
+        assert_match {*invalid expire time*} $e
+
+        catch {r set foo bar pxat 1234xyz} e
+        assert_match {*not an integer*} $e
+
+        catch {r set foo bar pxat 0} e
+        assert_match {*invalid expire time*} $e
+    }
 
     test {Extended SET using multiple options at once} {
         r set foo val