You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2023/01/13 18:03:00 UTC
[GitHub] [incubator-pegasus] acelyc111 opened a new pull request, #1311: refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3)
acelyc111 opened a new pull request, #1311:
URL: https://github.com/apache/incubator-pegasus/pull/1311
https://github.com/apache/incubator-pegasus/issues/1305
This patch aim to use LOG_INFO_F instead of LOG_INFO for all the remain parts:
- Use `LOG_INFO_F` instead of `LOG_INFO`
- Use `LOG_INFO_PREFIX` instead of `LOG_INFO` for class `proxy_session`, `pegasus_server_impl `
- Use upper case for commands in class `redis_parser`
- Remove some commentted `LOG_INFO` code
TODO:
1. print dsn_primary_address in log is useless, since it must be itself in whose log, e.g. in `src/common/fs_manager.cpp`, `src/replica/replica_stub.cpp`
2. try to use `LOG_*_PREFIX` for class `policy_context`, `cold_backup_context`
--
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
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1311: refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3)
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on code in PR #1311:
URL: https://github.com/apache/incubator-pegasus/pull/1311#discussion_r1070881204
##########
src/replica/backup/cold_backup_context.cpp:
##########
@@ -567,31 +573,31 @@ 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 "
+ "backup_metadata",
+ name);
Review Comment:
"backup_metadata" is not an independent parameter, but a part of the first string parameter, so it should not be removed.
--
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
[GitHub] [incubator-pegasus] Smityz commented on a diff in pull request #1311: refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3)
Posted by GitBox <gi...@apache.org>.
Smityz commented on code in PR #1311:
URL: https://github.com/apache/incubator-pegasus/pull/1311#discussion_r1070259025
##########
src/replica/backup/cold_backup_context.cpp:
##########
@@ -567,31 +573,31 @@ 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 "
+ "backup_metadata",
+ name);
Review Comment:
```suggestion
LOG_INFO_F("{}: checkpoint dir is empty, so upload is complete and just start write", 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
[GitHub] [incubator-pegasus] acelyc111 merged pull request #1311: refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3)
Posted by GitBox <gi...@apache.org>.
acelyc111 merged PR #1311:
URL: https://github.com/apache/incubator-pegasus/pull/1311
--
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
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1311: refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3)
Posted by 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