Skip to content

Commit

Permalink
fix: fix errors in format string for logging and macro (#1588)
Browse files Browse the repository at this point in the history
#1587

Fix some wrong format string in logging and macro that are:

- still using c-style format string, which would not print the correct messages;
- calling `to_string()` method for the classes which have implemented `operator<<`.
  • Loading branch information
empiredan authored Aug 28, 2023
1 parent c5a01c1 commit be9119e
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 40 deletions.
13 changes: 3 additions & 10 deletions src/meta/meta_backup_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1036,10 +1036,7 @@ void policy_context::sync_remove_backup_info(const backup_info &info, dsn::task_
0,
_backup_service->backup_option().meta_retry_delay_ms);
} else {
CHECK(false,
"{}: we can't handle this right now, error({})",
_policy.policy_name,
err.to_string());
CHECK(false, "{}: we can't handle this right now, error({})", _policy.policy_name, err);
}
};

Expand Down Expand Up @@ -1373,9 +1370,7 @@ void backup_service::do_add_policy(dsn::message_ex *req,
_opt.meta_retry_delay_ms);
return;
} else {
CHECK(false,
"we can't handle this when create backup policy, err({})",
err.to_string());
CHECK(false, "we can't handle this when create backup policy, err({})", err);
}
},
value);
Expand Down Expand Up @@ -1411,9 +1406,7 @@ void backup_service::do_update_policy_to_remote_storage(
0,
_opt.meta_retry_delay_ms);
} else {
CHECK(false,
"we can't handle this when create backup policy, err({})",
err.to_string());
CHECK(false, "we can't handle this when create backup policy, err({})", err);
}
});
}
Expand Down
12 changes: 4 additions & 8 deletions src/meta/meta_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,7 @@ bool construct_replica(meta_view view, const gpid &pid, int max_replica_count)
// we put max_replica_count-1 recent replicas to last_drops, in case of the DDD-state when the
// only primary dead
// when add node to pc.last_drops, we don't remove it from our cc.drop_list
CHECK(pc.last_drops.empty(),
"last_drops of partition({}.{}) must be empty",
pid.get_app_id(),
pid.get_partition_index());
CHECK(pc.last_drops.empty(), "last_drops of partition({}) must be empty", pid);
for (auto iter = drop_list.rbegin(); iter != drop_list.rend(); ++iter) {
if (pc.last_drops.size() + 1 >= max_replica_count)
break;
Expand Down Expand Up @@ -402,10 +399,9 @@ int config_context::collect_drop_replica(const rpc_address &node, const replica_
iter = find_from_dropped(node);
if (iter == dropped.end()) {
CHECK(!in_dropped,
"adjust position of existing node({}) failed, this is a bug, partition({}.{})",
node.to_string(),
config_owner->pid.get_app_id(),
config_owner->pid.get_partition_index());
"adjust position of existing node({}) failed, this is a bug, partition({})",
node,
config_owner->pid);
return -1;
}
return in_dropped ? 1 : 0;
Expand Down
14 changes: 5 additions & 9 deletions src/meta/partition_guardian.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ void partition_guardian::reconfig(meta_view view, const configuration_update_req
if (!cc->lb_actions.empty()) {
const configuration_proposal_action *current = cc->lb_actions.front();
CHECK(current != nullptr && current->type != config_type::CT_INVALID,
"invalid proposal for gpid({}.{})",
gpid.get_app_id(),
gpid.get_partition_index());
"invalid proposal for gpid({})",
gpid);
// if the valid proposal is from cure
if (!cc->lb_actions.is_from_balancer()) {
finish_cure_proposal(view, gpid, *current);
Expand Down Expand Up @@ -132,7 +131,7 @@ void partition_guardian::reconfig(meta_view view, const configuration_update_req

CHECK(cc->record_drop_history(request.node),
"node({}) has been in the dropped",
request.node.to_string());
request.node);
}
});
}
Expand Down Expand Up @@ -241,8 +240,7 @@ pc_status partition_guardian::on_missing_primary(meta_view &view, const dsn::gpi

for (int i = 0; i < pc.secondaries.size(); ++i) {
node_state *ns = get_node_state(*(view.nodes), pc.secondaries[i], false);
CHECK_NOTNULL(
ns, "invalid secondary address, address = {}", pc.secondaries[i].to_string());
CHECK_NOTNULL(ns, "invalid secondary address, address = {}", pc.secondaries[i]);
if (!ns->alive())
continue;

Expand Down Expand Up @@ -607,9 +605,7 @@ pc_status partition_guardian::on_missing_secondary(meta_view &view, const dsn::g
// if not emergency, only try to recover last dropped server
const dropped_replica &server = cc.dropped.back();
if (is_node_alive(*view.nodes, server.node)) {
CHECK(!server.node.is_invalid(),
"invalid server address, address = {}",
server.node.to_string());
CHECK(!server.node.is_invalid(), "invalid server address, address = {}", server.node);
action.node = server.node;
}

Expand Down
1 change: 0 additions & 1 deletion src/meta/test/backup_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <set>
#include <string>
#include <thread>
#include <type_traits>
#include <utility>
#include <vector>

Expand Down
6 changes: 2 additions & 4 deletions src/replica/replica.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@

#include <fmt/core.h>
#include <fmt/ostream.h>
#include <inttypes.h>
#include <rocksdb/status.h>
#include <algorithm>
#include <functional>
#include <iosfwd>
#include <rocksdb/status.h>
#include <set>

#include "backup/replica_backup_manager.h"
Expand Down Expand Up @@ -285,8 +284,7 @@ void replica::on_client_read(dsn::message_ex *request, bool ignore_throttling)

// a small window where the state is not the latest yet
if (last_committed_decree() < _primary_states.last_prepare_decree_on_new_primary) {
LOG_ERROR_PREFIX("last_committed_decree(%" PRId64
") < last_prepare_decree_on_new_primary(%" PRId64 ")",
LOG_ERROR_PREFIX("last_committed_decree({}) < last_prepare_decree_on_new_primary({})",
last_committed_decree(),
_primary_states.last_prepare_decree_on_new_primary);
response_client_read(request, ERR_INVALID_STATE);
Expand Down
6 changes: 1 addition & 5 deletions src/replica/replica_2pc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,7 @@ void replica::init_prepare(mutation_ptr &mu, bool reconciliation, bool pop_all_c
}

mu->_tracer->set_name(fmt::format("mutation[{}]", mu->name()));
dlog(level,
"%s: mutation %s init_prepare, mutation_tid=%" PRIu64,
name(),
mu->name(),
mu->tid());
dlog_f(level, "{}: mutation {} init_prepare, mutation_tid={}", name(), mu->name(), mu->tid());

// child should prepare mutation synchronously
mu->set_is_sync_to_child(_primary_states.sync_send_write_request);
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/task/task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ task::task(dsn::task_code code, int hash, service_node *node)
} else {
auto p = get_current_node();
CHECK_NOTNULL(p,
"tasks without explicit service node "
"can only be created inside threads which is attached to specific node");
"tasks without explicit service node can only be created "
"inside threads which is attached to specific node");
_node = p;
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/pegasus_server_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2308,7 +2308,7 @@ bool pegasus_server_impl::validate_filter(::dsn::apps::filter_type::type filter_
}
}
default:
CHECK(false, "unsupported filter type: %d", filter_type);
CHECK(false, "unsupported filter type: {}", filter_type);
}
return false;
}
Expand Down

0 comments on commit be9119e

Please sign in to comment.