[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #906: Avoid using Get interface to obtain metadata when iterating data during migration

[GitBox <gi...@apache.org>]

PragmaTwice commented on code in PR #906:
URL: https://github.com/apache/incubator-kvrocks/pull/906#discussion_r977411598


##########
src/slot_migrate.cc:
##########
@@ -543,29 +543,15 @@ bool SlotMigrate::CheckResponseWithCounts(int sock_fd, int total) {
   return true;  // Can't reach here
 }
 
-bool SlotMigrate::GetSlotKeyMetadata(const rocksdb::Slice &prefix_key, std::string *bytes) {
-  rocksdb::ReadOptions read_options;
-  read_options.snapshot = slot_snapshot_;
-  auto s = storage_->GetDB()->Get(read_options, storage_->GetCFHandle("metadata"), prefix_key, bytes);
-  if (!s.ok()) {
-    LOG(ERROR) << "[migrate] Failed to get metadata of key: " << prefix_key.ToString()
-      << ", Err: " << s.ToString();
-    return false;
-  }
-  return true;
-}
-
-Status SlotMigrate::MigrateOneKey(rocksdb::Slice key, std::string *restore_cmds) {
+Status SlotMigrate::MigrateOneKey(const rocksdb::Slice &key, const rocksdb::Slice &value, std::string *restore_cmds) {
   std::string prefix_key;
   AppendNamespacePrefix(key, &prefix_key);
-  std::string bytes;
-  bool st = GetSlotKeyMetadata(prefix_key, &bytes);
-  if (!st) return Status(Status::NotOK, "[migrate] Failed to get key's metadata");
+  std::string bytes = value.ToString();

Review Comment:
   This `std::move` is useless, and it will block the (N)RVO ([return value optimization](https://en.cppreference.com/w/cpp/language/copy_elision)).
   
   ###  1. this `std::move` is useless
   Signature of `ToString`:
   ```
   std::string Slice::ToString(...);
   ```
   So the return value `x.ToString()` is already an rvalue (or concretely, a [prvalue](https://en.cppreference.com/w/cpp/language/value_category#prvalue)).
   Move an rvalue is useless, since it will essentially dispatch to the rvalue reference overload.
   
   ### 2. it will block the (N)RVO
   



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org