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/19 07:44:33 UTC
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1320: refactor(log): use LOG_ERROR_F instead of LOG_ERROR (2/2)
acelyc111 commented on code in PR #1320:
URL: https://github.com/apache/incubator-pegasus/pull/1320#discussion_r1080883044
##########
src/failure_detector/failure_detector.cpp:
##########
@@ -227,13 +227,13 @@ void failure_detector::check_all_records()
is_time_greater_than(now, record.last_send_time_for_beacon_with_ack) &&
now + _check_interval_milliseconds - record.last_send_time_for_beacon_with_ack >
_lease_milliseconds) {
- LOG_ERROR("master %s disconnected, now=%" PRId64 ", last_send_time=%" PRId64
- ", now+check_interval-last_send_time=%" PRId64,
- record.node.to_string(),
- now,
- record.last_send_time_for_beacon_with_ack,
- now + _check_interval_milliseconds -
- record.last_send_time_for_beacon_with_ack);
+ LOG_ERROR_F("master {} disconnected, now={:#018x}, last_send_time={:#018x}, "
Review Comment:
is it necessary to use `:#018x`?
##########
src/client/partition_resolver_simple.cpp:
##########
@@ -311,28 +311,28 @@ void partition_resolver_simple::query_config_reply(error_code err,
}
}
} else if (resp.err == ERR_OBJECT_NOT_FOUND) {
- LOG_ERROR("%s.client: query config reply, gpid = %d.%d, err = %s",
- _app_name.c_str(),
- _app_id,
- partition_index,
- resp.err.to_string());
+ LOG_ERROR_F("{}.client: query config reply, gpid = {}.{}, err = {}",
Review Comment:
You can use `LOG_ERROR_PREFIX` macro instead.
##########
src/meta/meta_backup_service.cpp:
##########
@@ -625,10 +625,10 @@ void policy_context::sync_backup_to_remote_storage_unlocked(const backup_info &b
LOG_WARNING_F("{}: empty callback", _policy.policy_name);
}
} else if (ERR_TIMEOUT == err) {
- LOG_ERROR("%s: sync backup info(" PRId64
- ") to remote storage got timeout, retry it later",
- _policy.policy_name.c_str(),
- b_info.backup_id);
+ LOG_ERROR_F(
+ "{}: sync backup info({:#018x}) to remote storage got timeout, retry it later",
Review Comment:
why use `:#018x` ?
##########
src/runtime/rpc/asio_net_provider.cpp:
##########
@@ -148,7 +148,7 @@ void asio_network_provider::do_accept()
if (!ec) {
auto remote = socket->remote_endpoint(ec);
if (ec) {
- LOG_ERROR("failed to get the remote endpoint: %s", ec.message().data());
+ LOG_ERROR_F("failed to get the remote endpoint: {}", ec.message().data());
Review Comment:
ec.message() is ok?
##########
src/nfs/nfs_server_impl.cpp:
##########
@@ -208,9 +207,9 @@ void nfs_service_impl::on_get_file_size(
struct stat st;
if (0 != ::stat(file_path.c_str(), &st)) {
- LOG_ERROR("{nfs_service} get stat of file %s failed, err = %s",
- file_path.c_str(),
- strerror(errno));
+ LOG_ERROR_F("[nfs_service] get stat of file {} failed, err = {}",
+ file_path,
+ strerror(errno));
Review Comment:
it's recommend to use `dsn::utils::safe_strerror` instead.
--
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