[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1311: refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3)

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

empiredan commented on code in PR #1311:
URL: https://github.com/apache/incubator-pegasus/pull/1311#discussion_r1070850938


##########
src/server/pegasus_server_impl.cpp:
##########
@@ -1828,8 +1823,8 @@ ::dsn::error_code pegasus_server_impl::stop(bool clear_state)
         _pfc_rdb_memtable_mem_usage->set(0);
     }
 
-    LOG_INFO(
-        "%s: close app succeed, clear_state = %s", replica_name(), clear_state ? "true" : "false");
+    LOG_INFO_F(
+        "{}: close app succeed, clear_state = {}", replica_name(), clear_state ? "true" : "false");

Review Comment:
   ```suggestion
       LOG_INFO_PREFIX(
           "close app succeed, clear_state = {}", clear_state ? "true" : "false");
   ```



##########
src/test/kill_test/data_verifier.cpp:
##########
@@ -119,33 +119,33 @@ void do_set(int thread_id)
             client->set(hash_key, sort_key, value, set_and_get_timeout_milliseconds, 0, &info);
         if (ret == PERR_OK) {
             long cur_time = get_time();
-            LOG_INFO("SetThread[%d]: set succeed: id=%lld, try=%d, time=%ld (gpid=%d.%d, "
-                     "decree=%lld, server=%s)",
-                     thread_id,
-                     id,
-                     try_count,
-                     (cur_time - last_time),
-                     info.app_id,
-                     info.partition_index,
-                     info.decree,
-                     info.server.c_str());
+            LOG_INFO_F("SetThread[{}]: set succeed: id={}, try={}, time={} (gpid={}.{}, decree={}, "
+                       "server={})",
+                       thread_id,
+                       id,
+                       try_count,
+                       (cur_time - last_time),

Review Comment:
   ```suggestion
                          cur_time - last_time,
   ```



##########
src/server/pegasus_server_impl.cpp:
##########
@@ -3139,10 +3123,8 @@ bool pegasus_server_impl::set_options(
     }
     rocksdb::Status status = _db->SetOptions(_data_cf, new_options);
     if (status == rocksdb::Status::OK()) {
-        LOG_INFO("%s: rocksdb set options returns %s: {%s}",
-                 replica_name(),
-                 status.ToString().c_str(),
-                 oss.str().c_str());
+        LOG_INFO_F(
+            "{}: rocksdb set options returns {}: {}", replica_name(), status.ToString(), oss.str());

Review Comment:
   ```suggestion
           LOG_INFO_PREFIX(
               "rocksdb set options returns {}: {}", status.ToString(), oss.str());
   ```



##########
src/server/pegasus_server_impl.cpp:
##########
@@ -2563,27 +2554,22 @@ pegasus_server_impl::get_restore_dir_from_env(const std::map<std::string, std::s
 
     auto it = env_kvs.find(ROCKSDB_ENV_RESTORE_FORCE_RESTORE);
     if (it != env_kvs.end()) {
-        LOG_INFO("%s: found %s in envs", replica_name(), ROCKSDB_ENV_RESTORE_FORCE_RESTORE.c_str());
+        LOG_INFO_PREFIX("found {} in envs", ROCKSDB_ENV_RESTORE_FORCE_RESTORE);
         res.second = true;
     }
 
     it = env_kvs.find(ROCKSDB_ENV_RESTORE_POLICY_NAME);
     if (it != env_kvs.end()) {
-        LOG_INFO("%s: found %s in envs: %s",
-                 replica_name(),
-                 ROCKSDB_ENV_RESTORE_POLICY_NAME.c_str(),
-                 it->second.c_str());
+        LOG_INFO_PREFIX("found {} in envs: {}", ROCKSDB_ENV_RESTORE_POLICY_NAME, it->second);
         os << it->second << ".";
     } else {
         return res;
     }
 
     it = env_kvs.find(ROCKSDB_ENV_RESTORE_BACKUP_ID);
     if (it != env_kvs.end()) {
-        LOG_INFO("%s: found %s in envs: %s",
-                 replica_name(),
-                 ROCKSDB_ENV_RESTORE_BACKUP_ID.c_str(),
-                 it->second.c_str());
+        LOG_INFO_F(
+            "{}: found {} in envs: {}", replica_name(), ROCKSDB_ENV_RESTORE_BACKUP_ID, it->second);

Review Comment:
   ```suggestion
           LOG_INFO_PREFIX(
               "found {} in envs: {}", ROCKSDB_ENV_RESTORE_BACKUP_ID, it->second);
   ```



##########
src/replica/backup/cold_backup_context.cpp:
##########
@@ -567,31 +573,29 @@ void cold_backup_context::on_upload_chkpt_dir()
     }
 
     if (checkpoint_files.size() <= 0) {
-        LOG_INFO("%s: checkpoint dir is empty, so upload is complete and just start write "
-                 "backup_metadata",
-                 name);
+        LOG_INFO_F("{}: checkpoint dir is empty, so upload is complete and just start write", name);

Review Comment:
   ```suggestion
           LOG_INFO_F("{}: checkpoint dir is empty, so upload is complete and just start to write backup_metadata", name);
   ```



-- 
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: dev-unsubscribe@pegasus.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org