enjoy-binbin commented on code in PR #1684:
URL: https://github.com/apache/kvrocks/pull/1684#discussion_r1306610060
##########
src/commands/cmd_server.cc:
##########
@@ -978,6 +980,70 @@ static uint64_t GenerateConfigFlag(const
std::vector<std::string> &args) {
return 0;
}
+class CommandRestore : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 4);
+ ttl_ms_ = GET_OR_RET(ParseInt<uint64_t>(args[2], 10));
+ while (parser.Good()) {
+ if (parser.EatEqICase("replace")) {
+ replace_ = true;
+ } else if (parser.EatEqICase("absttl")) {
+ auto now = util::GetTimeStampMS();
+ if (ttl_ms_ <= static_cast<uint64_t>(now)) {
+ return {Status::RedisExecErr, "Invalid expire time"};
+ }
+ ttl_ms_ -= now;
+ } else if (parser.EatEqICase("idletime")) {
+ // idle time is not supported in Kvrocks, so just skip it
+ auto idle_time = GET_OR_RET(parser.TakeInt());
+ if (idle_time < 0) {
+ return {Status::RedisParseErr, "IDLETIME can't be negative"};
+ }
+ } else if (parser.EatEqICase("freq")) {
+ // freq is not supported in Kvrocks, so just skip it
+ auto freq = GET_OR_RET(parser.TakeInt());
+ if (freq < 0 || freq > 255) {
+ return {Status::RedisParseErr, "FREQ must be >= 0 and <= 255"};
+ }
+ } else {
+ return {Status::RedisParseErr, errInvalidSyntax};
+ }
+ }
+ return Status::OK();
+ }
+
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ rocksdb::Status db_status;
+ redis::Database redis(svr->storage, conn->GetNamespace());
+ if (!replace_) {
+ int count = 0;
+ db_status = redis.Exists({args_[1]}, &count);
+ if (!db_status.ok()) {
+ return {Status::RedisExecErr, db_status.ToString()};
+ }
+ if (count > 0) {
+ return {Status::RedisExecErr, "target key name already exists."};
Review Comment:
redis will throw a `BUSYKEY` error, ie. is actually a BUSYKEY error code. We
will only return ERR here. but i guess it is ok, we need to find time to clean
up and align some error codes
```
127.0.0.1:6379> restore busy_key 1111
"\x00\x01b\x0b\x00\x7f\xfb\x05\xe2[|\xfb\n" absttl
(error) BUSYKEY Target key name already exists.
```
##########
src/commands/cmd_server.cc:
##########
@@ -978,6 +980,70 @@ static uint64_t GenerateConfigFlag(const
std::vector<std::string> &args) {
return 0;
}
+class CommandRestore : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 4);
+ ttl_ms_ = GET_OR_RET(ParseInt<uint64_t>(args[2], 10));
+ while (parser.Good()) {
+ if (parser.EatEqICase("replace")) {
+ replace_ = true;
+ } else if (parser.EatEqICase("absttl")) {
+ auto now = util::GetTimeStampMS();
+ if (ttl_ms_ <= static_cast<uint64_t>(now)) {
+ return {Status::RedisExecErr, "Invalid expire time"};
+ }
+ ttl_ms_ -= now;
+ } else if (parser.EatEqICase("idletime")) {
+ // idle time is not supported in Kvrocks, so just skip it
+ auto idle_time = GET_OR_RET(parser.TakeInt());
+ if (idle_time < 0) {
+ return {Status::RedisParseErr, "IDLETIME can't be negative"};
+ }
+ } else if (parser.EatEqICase("freq")) {
+ // freq is not supported in Kvrocks, so just skip it
+ auto freq = GET_OR_RET(parser.TakeInt());
+ if (freq < 0 || freq > 255) {
+ return {Status::RedisParseErr, "FREQ must be >= 0 and <= 255"};
+ }
+ } else {
+ return {Status::RedisParseErr, errInvalidSyntax};
+ }
+ }
+ return Status::OK();
+ }
+
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ rocksdb::Status db_status;
+ redis::Database redis(svr->storage, conn->GetNamespace());
+ if (!replace_) {
+ int count = 0;
+ db_status = redis.Exists({args_[1]}, &count);
+ if (!db_status.ok()) {
+ return {Status::RedisExecErr, db_status.ToString()};
+ }
+ if (count > 0) {
+ return {Status::RedisExecErr, "target key name already exists."};
+ }
+ } else {
+ db_status = redis.Del(args_[1]);
+ if (!db_status.ok() && !db_status.IsNotFound()) {
+ return {Status::RedisExecErr, db_status.ToString()};
+ }
Review Comment:
if key is not found, i think we should not return an error?
```
127.0.0.1:6379> del busy_key
(integer) 0
127.0.0.1:6379> restore busy_key 1111
"\x00\x01b\x0b\x00\x7f\xfb\x05\xe2[|\xfb\n" absttl replace
OK
```
##########
src/commands/cmd_server.cc:
##########
@@ -978,6 +980,70 @@ static uint64_t GenerateConfigFlag(const
std::vector<std::string> &args) {
return 0;
}
+class CommandRestore : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 4);
+ ttl_ms_ = GET_OR_RET(ParseInt<uint64_t>(args[2], 10));
+ while (parser.Good()) {
+ if (parser.EatEqICase("replace")) {
+ replace_ = true;
+ } else if (parser.EatEqICase("absttl")) {
+ auto now = util::GetTimeStampMS();
+ if (ttl_ms_ <= static_cast<uint64_t>(now)) {
+ return {Status::RedisExecErr, "Invalid expire time"};
+ }
+ ttl_ms_ -= now;
Review Comment:
in this case, redis will implicitly expire keys instead of throwing an error.
```
127.0.0.1:6379> restore busy_key 1111
"\x00\x01b\x0b\x00\x7f\xfb\x05\xe2[|\xfb\n" absttl
OK
```
and if we want to throw the error, here i think we should use RedisParseErr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
