PragmaTwice commented on code in PR #3318:
URL: https://github.com/apache/kvrocks/pull/3318#discussion_r2656094523


##########
src/types/redis_string.cc:
##########
@@ -393,46 +393,71 @@ rocksdb::Status String::IncrByFloat(engine::Context &ctx, 
const std::string &use
   return updateRawValue(ctx, ns_key, raw_value);
 }
 
-rocksdb::Status String::MSet(engine::Context &ctx, const 
std::vector<StringPair> &pairs, uint64_t expire_ms) {
+rocksdb::Status String::MSet(engine::Context &ctx, const 
std::vector<StringPair> &pairs, StringMSetArgs args,
+                             bool *flag) {
+  if (flag != nullptr) {
+    *flag = false;
+  }
+
+  if (args.type != StringSetType::NONE) {
+    int exists = 0;
+    int key_count = static_cast<int>(pairs.size());
+    std::vector<Slice> keys;
+    keys.reserve(pairs.size());
+    for (const auto &pair : pairs) {
+      keys.emplace_back(pair.key);
+    }
+    auto s = Exists(ctx, keys, &exists);
+    if (!s.ok()) return s;
+    if ((args.type == StringSetType::NX && exists > 0) || (args.type == 
StringSetType::XX && exists < key_count)) {
+      return rocksdb::Status::OK();
+    }
+  }
+
   auto batch = storage_->GetWriteBatchBase();
   WriteBatchLogData log_data(kRedisString);
-  auto s = batch->PutLogData(log_data.Encode());
+  rocksdb::Status s = batch->PutLogData(log_data.Encode());
   if (!s.ok()) return s;
+
   for (const auto &pair : pairs) {
     std::string bytes;
     Metadata metadata(kRedisString, false);
-    metadata.expire = expire_ms;
+    if (args.keep_ttl) {
+      uint64_t old_expire = 0;
+      rocksdb::Status s = GetExpireTime(ctx, pair.key, &old_expire);

Review Comment:
   Optimization: there are duplicated IO accesses in `Exists` and 
`GetExpireTime`, maybe we can unfiy them and only read a key once.



-- 
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]

Reply via email to