From 4758a3689cf43951bfe8c458ca1dd4a756743ccf Mon Sep 17 00:00:00 2001 From: Yingchun Lai Date: Sun, 6 Nov 2022 15:15:58 +0800 Subject: [PATCH] refactor(macro): use DCHECK* to replace dbg_dassert and self defined ASSERT_* --- .../balancer_simulator/balancer_simulator.cpp | 25 +--- src/meta/test/balancer_validator.cpp | 105 ++++++-------- src/meta/test/misc/misc.cpp | 129 +++++++++--------- src/runtime/rpc/network.cpp | 10 +- src/runtime/rpc/rpc_engine.cpp | 34 +++-- src/runtime/rpc/rpc_message.cpp | 1 - src/runtime/rpc/rpc_stream.h | 21 +-- src/runtime/rpc/rpc_task.cpp | 18 +-- src/runtime/task/task_queue.cpp | 2 +- src/utils/fmt_logging.h | 64 ++++++++- 10 files changed, 213 insertions(+), 196 deletions(-) diff --git a/src/meta/test/balancer_simulator/balancer_simulator.cpp b/src/meta/test/balancer_simulator/balancer_simulator.cpp index 2fa7b1b7b5..2efd6b2aab 100644 --- a/src/meta/test/balancer_simulator/balancer_simulator.cpp +++ b/src/meta/test/balancer_simulator/balancer_simulator.cpp @@ -25,30 +25,17 @@ */ #include + #include +#include "meta/greedy_load_balancer.h" #include "meta/meta_data.h" #include "meta/server_load_balancer.h" -#include "meta/greedy_load_balancer.h" #include "meta/test/misc/misc.h" +#include "utils/fmt_logging.h" using namespace dsn::replication; -#ifdef ASSERT_EQ -#undef ASSERT_EQ -#endif -#define ASSERT_EQ(left, right) CHECK((left) == (right), "") - -#ifdef ASSERT_TRUE -#undef ASSERT_TRUE -#endif -#define ASSERT_TRUE(exp) CHECK((exp), "") - -#ifdef ASSERT_FALSE -#undef ASSERT_FALSE -#endif -#define ASSERT_FALSE(exp) CHECK(!(exp), "") - class simple_priority_queue { public: @@ -143,8 +130,8 @@ void generate_balanced_apps(/*out*/ app_mapper &apps, apps.emplace(the_app->app_id, the_app); - ASSERT_TRUE(pri_max - pri_min <= 1); - ASSERT_TRUE(part_max - part_min <= 1); + CHECK_LE(pri_max - pri_min, 1); + CHECK_LE(part_max - part_min, 1); } void random_move_primary(app_mapper &apps, node_mapper &nodes, int primary_move_ratio) @@ -184,7 +171,7 @@ void greedy_balancer_perfect_move_primary() for (const auto &kv : ml) { const std::shared_ptr &req = kv.second; for (const configuration_proposal_action &act : req->action_list) { - ASSERT_TRUE(act.type != config_type::CT_ADD_SECONDARY_FOR_LB); + CHECK_NE(act.type, config_type::CT_ADD_SECONDARY_FOR_LB); } } glb.check({&apps, &nodes}, ml); diff --git a/src/meta/test/balancer_validator.cpp b/src/meta/test/balancer_validator.cpp index 60f5146c4f..ffb117789f 100644 --- a/src/meta/test/balancer_validator.cpp +++ b/src/meta/test/balancer_validator.cpp @@ -24,66 +24,42 @@ * THE SOFTWARE. */ +#include + #include #include "common/api_common.h" -#include "runtime/api_task.h" +#include "common/gpid.h" +#include "backup_types.h" +#include "bulk_load_types.h" +#include "common/serialization_helper/dsn.layer2_types.h" +#include "consensus_types.h" +#include "duplication_types.h" +#include "meta_admin_types.h" +#include "meta_service_test_app.h" +#include "meta/greedy_load_balancer.h" +#include "meta/meta_data.h" +#include "meta/server_load_balancer.h" +#include "meta/test/misc/misc.h" +#include "partition_split_types.h" +#include "replica_admin_types.h" #include "runtime/api_layer1.h" -#include "runtime/app_model.h" -#include "utils/api_utilities.h" -#include "common/api_common.h" #include "runtime/api_task.h" -#include "runtime/api_layer1.h" #include "runtime/app_model.h" -#include "utils/api_utilities.h" -#include "utils/error_code.h" -#include "utils/threadpool_code.h" -#include "runtime/task/task_code.h" -#include "common/gpid.h" -#include "runtime/rpc/serialization.h" #include "runtime/rpc/rpc_stream.h" +#include "runtime/rpc/serialization.h" #include "runtime/serverlet.h" #include "runtime/service_app.h" +#include "runtime/task/task_code.h" +#include "utils/api_utilities.h" +#include "utils/error_code.h" +#include "utils/fmt_logging.h" #include "utils/rpc_address.h" -#include "meta_admin_types.h" -#include "partition_split_types.h" -#include "duplication_types.h" -#include "bulk_load_types.h" -#include "backup_types.h" -#include "consensus_types.h" -#include "replica_admin_types.h" -#include "common/serialization_helper/dsn.layer2_types.h" - -#include - -#include "meta/meta_data.h" -#include "meta/server_load_balancer.h" -#include "meta/greedy_load_balancer.h" - -#include "meta/test/misc/misc.h" - -#include "meta_service_test_app.h" +#include "utils/threadpool_code.h" namespace dsn { namespace replication { -#ifdef ASSERT_EQ -#undef ASSERT_EQ -#endif - -#define ASSERT_EQ(left, right) CHECK((left) == (right), "") - -#ifdef ASSERT_TRUE -#undef ASSERT_TRUE -#endif - -#define ASSERT_TRUE(exp) CHECK((exp), "") - -#ifdef ASSERT_FALSE -#undef ASSERT_FALSE -#endif -#define ASSERT_FALSE(exp) CHECK(!(exp), "") - static void check_cure(app_mapper &apps, node_mapper &nodes, ::dsn::partition_configuration &pc) { meta_service svc; @@ -98,48 +74,49 @@ static void check_cure(app_mapper &apps, node_mapper &nodes, ::dsn::partition_co break; switch (act.type) { case config_type::CT_ASSIGN_PRIMARY: - ASSERT_TRUE(pc.primary.is_invalid() && pc.secondaries.size() == 0); - ASSERT_EQ(act.node, act.target); - ASSERT_TRUE(nodes.find(act.node) != nodes.end()); + CHECK(pc.primary.is_invalid(), ""); + CHECK(pc.secondaries.empty(), ""); + CHECK_EQ(act.node, act.target); + CHECK(nodes.find(act.node) != nodes.end(), ""); - ASSERT_EQ(nodes[act.node].served_as(pc.pid), partition_status::PS_INACTIVE); + CHECK_EQ(nodes[act.node].served_as(pc.pid), partition_status::PS_INACTIVE); nodes[act.node].put_partition(pc.pid, true); pc.primary = act.node; break; case config_type::CT_ADD_SECONDARY: - ASSERT_FALSE(is_member(pc, act.node)); - ASSERT_EQ(pc.primary, act.target); - ASSERT_TRUE(nodes.find(act.node) != nodes.end()); + CHECK(!is_member(pc, act.node), ""); + CHECK_EQ(pc.primary, act.target); + CHECK(nodes.find(act.node) != nodes.end(), ""); pc.secondaries.push_back(act.node); ns = &nodes[act.node]; - ASSERT_EQ(ns->served_as(pc.pid), partition_status::PS_INACTIVE); + CHECK_EQ(ns->served_as(pc.pid), partition_status::PS_INACTIVE); ns->put_partition(pc.pid, false); break; default: - ASSERT_TRUE(false); + CHECK(false, ""); break; } } // test upgrade to primary - ASSERT_EQ(nodes[pc.primary].served_as(pc.pid), partition_status::PS_PRIMARY); + CHECK_EQ(nodes[pc.primary].served_as(pc.pid), partition_status::PS_PRIMARY); nodes[pc.primary].remove_partition(pc.pid, true); pc.primary.set_invalid(); ps = guardian.cure({&apps, &nodes}, pc.pid, act); - ASSERT_EQ(act.type, config_type::CT_UPGRADE_TO_PRIMARY); - ASSERT_TRUE(pc.primary.is_invalid()); - ASSERT_EQ(act.node, act.target); - ASSERT_TRUE(is_secondary(pc, act.node)); - ASSERT_TRUE(nodes.find(act.node) != nodes.end()); + CHECK_EQ(act.type, config_type::CT_UPGRADE_TO_PRIMARY); + CHECK(pc.primary.is_invalid(), ""); + CHECK_EQ(act.node, act.target); + CHECK(is_secondary(pc, act.node), ""); + CHECK(nodes.find(act.node) != nodes.end(), ""); ns = &nodes[act.node]; pc.primary = act.node; std::remove(pc.secondaries.begin(), pc.secondaries.end(), pc.primary); - ASSERT_EQ(ns->served_as(pc.pid), partition_status::PS_SECONDARY); + CHECK_EQ(ns->served_as(pc.pid), partition_status::PS_SECONDARY); ns->put_partition(pc.pid, true); } @@ -286,8 +263,8 @@ void meta_service_test_app::balancer_validator() std::shared_ptr &the_app = apps[1]; for (::dsn::partition_configuration &pc : the_app->partitions) { - ASSERT_FALSE(pc.primary.is_invalid()); - ASSERT_TRUE(pc.secondaries.size() >= pc.max_replica_count - 1); + CHECK(!pc.primary.is_invalid(), ""); + CHECK_GE(pc.secondaries.size(), pc.max_replica_count - 1); } // now test the cure diff --git a/src/meta/test/misc/misc.cpp b/src/meta/test/misc/misc.cpp index 147b0432d6..8b3a383744 100644 --- a/src/meta/test/misc/misc.cpp +++ b/src/meta/test/misc/misc.cpp @@ -24,20 +24,19 @@ * THE SOFTWARE. */ +#include "misc.h" + #include #include + #include -#include "utils/rand.h" #include "common/replication_common.h" -#include "misc.h" +#include "utils/fmt_logging.h" +#include "utils/rand.h" using namespace dsn::replication; -#define ASSERT_EQ(left, right) CHECK((left) == (right), "") -#define ASSERT_TRUE(exp) CHECK((exp), "") -#define ASSERT_FALSE(exp) CHECK(!(exp), "") - uint32_t random32(uint32_t min, uint32_t max) { uint32_t res = (uint32_t)(rand() % (max - min + 1)); @@ -88,7 +87,7 @@ void generate_node_mapper( ns->put_partition(pc.pid, true); } for (const dsn::rpc_address &sec : pc.secondaries) { - ASSERT_FALSE(sec.is_invalid()); + CHECK(!sec.is_invalid(), ""); ns = get_node_state(output_nodes, sec, true); ns->put_partition(pc.pid, false); } @@ -113,10 +112,10 @@ void generate_app(/*out*/ std::shared_ptr &app, if (i != p) pc.secondaries.push_back(node_list[indices[i]]); - ASSERT_FALSE(pc.primary.is_invalid()); - ASSERT_FALSE(is_secondary(pc, pc.primary)); - ASSERT_EQ(pc.secondaries.size(), 2); - ASSERT_TRUE(pc.secondaries[0] != pc.secondaries[1]); + CHECK(!pc.primary.is_invalid(), ""); + CHECK(!is_secondary(pc, pc.primary), ""); + CHECK_EQ(pc.secondaries.size(), 2); + CHECK_NE(pc.secondaries[0], pc.secondaries[1]); } } @@ -216,26 +215,26 @@ void track_disk_info_check_and_apply(const dsn::replication::configuration_propo /*in-out*/ nodes_fs_manager &manager) { config_context *cc = get_config_context(apps, pid); - ASSERT_TRUE(cc != nullptr); + CHECK_NOTNULL(cc, ""); fs_manager *target_manager = get_fs_manager(manager, act.target); - ASSERT_TRUE(target_manager != nullptr); + CHECK_NOTNULL(target_manager, ""); fs_manager *node_manager = get_fs_manager(manager, act.node); - ASSERT_TRUE(node_manager != nullptr); + CHECK_NOTNULL(node_manager, ""); std::string dir; replica_info ri; switch (act.type) { case config_type::CT_ASSIGN_PRIMARY: target_manager->allocate_dir(pid, "test", dir); - ASSERT_EQ(dsn::ERR_OK, target_manager->get_disk_tag(dir, ri.disk_tag)); + CHECK_EQ(dsn::ERR_OK, target_manager->get_disk_tag(dir, ri.disk_tag)); cc->collect_serving_replica(act.target, ri); break; case config_type::CT_ADD_SECONDARY: case config_type::CT_ADD_SECONDARY_FOR_LB: node_manager->allocate_dir(pid, "test", dir); - ASSERT_EQ(dsn::ERR_OK, node_manager->get_disk_tag(dir, ri.disk_tag)); + CHECK_EQ(dsn::ERR_OK, node_manager->get_disk_tag(dir, ri.disk_tag)); cc->collect_serving_replica(act.node, ri); break; @@ -250,7 +249,7 @@ void track_disk_info_check_and_apply(const dsn::replication::configuration_propo break; default: - ASSERT_TRUE(false); + CHECK(false, ""); break; } } @@ -265,9 +264,9 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act, node_state *ns; ++pc.ballot; - ASSERT_TRUE(act.type != config_type::CT_INVALID); - ASSERT_FALSE(act.target.is_invalid()); - ASSERT_FALSE(act.node.is_invalid()); + CHECK_NE(act.type, config_type::CT_INVALID); + CHECK(!act.target.is_invalid(), ""); + CHECK(!act.node.is_invalid(), ""); if (manager) { track_disk_info_check_and_apply(act, pid, apps, nodes, *manager); @@ -275,76 +274,76 @@ void proposal_action_check_and_apply(const configuration_proposal_action &act, switch (act.type) { case config_type::CT_ASSIGN_PRIMARY: - ASSERT_EQ(act.node, act.target); - ASSERT_TRUE(pc.primary.is_invalid()); - ASSERT_TRUE(pc.secondaries.empty()); + CHECK_EQ(act.node, act.target); + CHECK(pc.primary.is_invalid(), ""); + CHECK(pc.secondaries.empty(), ""); pc.primary = act.node; ns = &nodes[act.node]; - ASSERT_EQ(ns->served_as(pc.pid), partition_status::PS_INACTIVE); + CHECK_EQ(ns->served_as(pc.pid), partition_status::PS_INACTIVE); ns->put_partition(pc.pid, true); break; case config_type::CT_ADD_SECONDARY: - ASSERT_EQ(act.target, pc.primary); - ASSERT_FALSE(is_member(pc, act.node)); + CHECK_EQ(act.target, pc.primary); + CHECK(!is_member(pc, act.node), ""); pc.secondaries.push_back(act.node); ns = &nodes[act.node]; - ASSERT_EQ(ns->served_as(pc.pid), partition_status::PS_INACTIVE); + CHECK_EQ(ns->served_as(pc.pid), partition_status::PS_INACTIVE); ns->put_partition(pc.pid, false); break; case config_type::CT_DOWNGRADE_TO_SECONDARY: - ASSERT_EQ(act.node, act.target); - ASSERT_EQ(act.node, pc.primary); - ASSERT_TRUE(nodes.find(act.node) != nodes.end()); - ASSERT_FALSE(is_secondary(pc, pc.primary)); + CHECK_EQ(act.node, act.target); + CHECK_EQ(act.node, pc.primary); + CHECK(nodes.find(act.node) != nodes.end(), ""); + CHECK(!is_secondary(pc, pc.primary), ""); nodes[act.node].remove_partition(pc.pid, true); pc.secondaries.push_back(pc.primary); pc.primary.set_invalid(); break; case config_type::CT_UPGRADE_TO_PRIMARY: - ASSERT_TRUE(pc.primary.is_invalid()); - ASSERT_EQ(act.node, act.target); - ASSERT_TRUE(is_secondary(pc, act.node)); - ASSERT_TRUE(nodes.find(act.node) != nodes.end()); + CHECK(pc.primary.is_invalid(), ""); + CHECK_EQ(act.node, act.target); + CHECK(is_secondary(pc, act.node), ""); + CHECK(nodes.find(act.node) != nodes.end(), ""); ns = &nodes[act.node]; pc.primary = act.node; - ASSERT_TRUE(replica_helper::remove_node(act.node, pc.secondaries)); + CHECK(replica_helper::remove_node(act.node, pc.secondaries), ""); ns->put_partition(pc.pid, true); break; case config_type::CT_ADD_SECONDARY_FOR_LB: - ASSERT_EQ(act.target, pc.primary); - ASSERT_FALSE(is_member(pc, act.node)); - ASSERT_FALSE(act.node.is_invalid()); + CHECK_EQ(act.target, pc.primary); + CHECK(!is_member(pc, act.node), ""); + CHECK(!act.node.is_invalid(), ""); pc.secondaries.push_back(act.node); ns = &nodes[act.node]; ns->put_partition(pc.pid, false); - ASSERT_EQ(ns->served_as(pc.pid), partition_status::PS_SECONDARY); + CHECK_EQ(ns->served_as(pc.pid), partition_status::PS_SECONDARY); break; // in balancer, remove primary is not allowed case config_type::CT_REMOVE: case config_type::CT_DOWNGRADE_TO_INACTIVE: - ASSERT_FALSE(pc.primary.is_invalid()); - ASSERT_EQ(pc.primary, act.target); - ASSERT_TRUE(is_secondary(pc, act.node)); - ASSERT_TRUE(nodes.find(act.node) != nodes.end()); - ASSERT_TRUE(replica_helper::remove_node(act.node, pc.secondaries)); + CHECK(!pc.primary.is_invalid(), ""); + CHECK_EQ(pc.primary, act.target); + CHECK(is_secondary(pc, act.node), ""); + CHECK(nodes.find(act.node) != nodes.end(), ""); + CHECK(replica_helper::remove_node(act.node, pc.secondaries), ""); ns = &nodes[act.node]; - ASSERT_EQ(ns->served_as(pc.pid), partition_status::PS_SECONDARY); + CHECK_EQ(ns->served_as(pc.pid), partition_status::PS_SECONDARY); ns->remove_partition(pc.pid, false); break; default: - ASSERT_TRUE(false); + CHECK(false, ""); break; } } @@ -363,16 +362,17 @@ void migration_check_and_apply(app_mapper &apps, proposal->gpid.get_partition_index()); std::shared_ptr &the_app = apps.find(proposal->gpid.get_app_id())->second; - ASSERT_EQ(proposal->gpid.get_app_id(), the_app->app_id); - ASSERT_TRUE(proposal->gpid.get_partition_index() < the_app->partition_count); + CHECK_EQ(proposal->gpid.get_app_id(), the_app->app_id); + CHECK_LT(proposal->gpid.get_partition_index(), the_app->partition_count); dsn::partition_configuration &pc = the_app->partitions[proposal->gpid.get_partition_index()]; - ASSERT_FALSE(pc.primary.is_invalid()); - ASSERT_EQ(pc.secondaries.size(), 2); - for (auto &addr : pc.secondaries) - ASSERT_FALSE(addr.is_invalid()); - ASSERT_FALSE(is_secondary(pc, pc.primary)); + CHECK(!pc.primary.is_invalid(), ""); + CHECK_EQ(pc.secondaries.size(), 2); + for (auto &addr : pc.secondaries) { + CHECK(!addr.is_invalid(), ""); + } + CHECK(!is_secondary(pc, pc.primary), ""); for (unsigned int j = 0; j < proposal->action_list.size(); ++j) { configuration_proposal_action &act = proposal->action_list[j]; @@ -388,22 +388,23 @@ void migration_check_and_apply(app_mapper &apps, void app_mapper_compare(const app_mapper &mapper1, const app_mapper &mapper2) { - ASSERT_EQ(mapper1.size(), mapper2.size()); + CHECK_EQ(mapper1.size(), mapper2.size()); for (auto &kv : mapper1) { const std::shared_ptr &app1 = kv.second; - ASSERT_TRUE(mapper2.find(app1->app_id) != mapper2.end()); + CHECK(mapper2.find(app1->app_id) != mapper2.end(), ""); const std::shared_ptr app2 = mapper2.find(app1->app_id)->second; - ASSERT_EQ(app1->app_id, app2->app_id); - ASSERT_EQ(app1->app_name, app2->app_name); - ASSERT_EQ(app1->app_type, app2->app_type); - ASSERT_EQ(app1->status, app2->status); - ASSERT_TRUE(app1->status == dsn::app_status::AS_AVAILABLE || - app1->status == dsn::app_status::AS_DROPPED); + CHECK_EQ(app1->app_id, app2->app_id); + CHECK_EQ(app1->app_name, app2->app_name); + CHECK_EQ(app1->app_type, app2->app_type); + CHECK_EQ(app1->status, app2->status); + CHECK(app1->status == dsn::app_status::AS_AVAILABLE || + app1->status == dsn::app_status::AS_DROPPED, + ""); if (app1->status == dsn::app_status::AS_AVAILABLE) { - ASSERT_EQ(app1->partition_count, app2->partition_count); + CHECK_EQ(app1->partition_count, app2->partition_count); for (unsigned int i = 0; i < app1->partition_count; ++i) { - ASSERT_TRUE(is_partition_config_equal(app1->partitions[i], app2->partitions[i])); + CHECK(is_partition_config_equal(app1->partitions[i], app2->partitions[i]), ""); } } } diff --git a/src/runtime/rpc/network.cpp b/src/runtime/rpc/network.cpp index 6ba20fed25..a1fd672a12 100644 --- a/src/runtime/rpc/network.cpp +++ b/src/runtime/rpc/network.cpp @@ -171,12 +171,8 @@ inline bool rpc_session::unlink_message_for_send() auto n = _messages.next(); int bcount = 0; - dbg_dassert(0 == _sending_buffers.size(), - "sending_buffers should be empty, but size = %d", - (int)_sending_buffers.size()); - dbg_dassert(0 == _sending_msgs.size(), - "sending_msgs should be empty, but size = %d", - (int)_sending_msgs.size()); + DCHECK_EQ(0, _sending_buffers.size()); + DCHECK_EQ(0, _sending_msgs.size()); while (n != &_messages) { auto lmsg = CONTAINING_RECORD(n, message_ex, dl); @@ -429,7 +425,7 @@ bool rpc_session::on_recv_message(message_ex *msg, int delay_ms) return false; } - dbg_dassert(!is_client(), "only rpc server session can recv rpc requests"); + DCHECK(!is_client(), "only rpc server session can recv rpc requests"); _net.on_recv_request(msg, delay_ms); } diff --git a/src/runtime/rpc/rpc_engine.cpp b/src/runtime/rpc/rpc_engine.cpp index 1855efc342..1fe71a5779 100644 --- a/src/runtime/rpc/rpc_engine.cpp +++ b/src/runtime/rpc/rpc_engine.cpp @@ -95,8 +95,8 @@ bool rpc_client_matcher::on_recv_reply(network *net, uint64_t key, message_ex *r } } - dbg_dassert(call != nullptr, "rpc response task cannot be empty"); - dbg_dassert(timeout_task != nullptr, "rpc timeout task cannot be empty"); + DCHECK_NOTNULL(call, "rpc response task cannot be nullptr"); + DCHECK_NOTNULL(timeout_task, "rpc timeout task cannot be nullptr"); if (timeout_task != task::get_current_task()) { timeout_task->cancel(false); // no need to wait @@ -127,10 +127,12 @@ bool rpc_client_matcher::on_recv_reply(network *net, uint64_t key, message_ex *r ::dsn::unmarshall((dsn::message_ex *)reply, addr); // handle the case of forwarding to itself where addr == req->to_address. - dbg_dassert(addr != req->to_address, - "impossible forwarding to myself as this only happens when i'm pure client so " - "i don't get a named to_address %s", - addr.to_string()); + DCHECK_NE_MSG( + addr, + req->to_address, + "impossible forwarding to myself as this only happens when i'm pure client so " + "i don't get a named to_address {}", + addr); // server address side effect switch (req->server_address.type()) { @@ -223,7 +225,7 @@ void rpc_client_matcher::on_rpc_timeout(uint64_t key) } } - dbg_dassert(call != nullptr, "rpc response task is missing for rpc request %" PRIu64, key); + DCHECK_NOTNULL(call, "rpc response task is missing for rpc request {}", key); // if timeout if (!resend) { @@ -296,7 +298,7 @@ void rpc_client_matcher::on_call(message_ex *request, const rpc_response_task_pt timeout_ms = sp->rpc_request_resend_timeout_milliseconds; } - dbg_dassert(call != nullptr, "rpc response task cannot be empty"); + DCHECK_NOTNULL(call, "rpc response task cannot be nullptr"); task *timeout_task(new rpc_timeout_task(this, hdr.id, call->node())); { @@ -619,7 +621,7 @@ void rpc_engine::call_group(rpc_address addr, message_ex *request, const rpc_response_task_ptr &call) { - dbg_dassert(addr.type() == HOST_TYPE_GROUP, "only group is now supported"); + DCHECK_EQ_MSG(addr.type(), HOST_TYPE_GROUP, "only group is now supported"); auto sp = task_spec::get(request->local_rpc_code); switch (sp->grpc_mode) { @@ -644,8 +646,8 @@ void rpc_engine::call_ip(rpc_address addr, bool reset_request_id, bool set_forwarded) { - dbg_dassert(addr.type() == HOST_TYPE_IPV4, "only IPV4 is now supported"); - dbg_dassert(addr.port() > MAX_CLIENT_PORT, "only server address can be called"); + DCHECK_EQ_MSG(addr.type(), HOST_TYPE_IPV4, "only IPV4 is now supported"); + DCHECK_GT_MSG(addr.port(), MAX_CLIENT_PORT, "only server address can be called"); CHECK(!request->header->from_address.is_invalid(), "from address must be set before call call_ip"); @@ -766,8 +768,9 @@ void rpc_engine::reply(message_ex *response, error_code err) // request is forwarded, we cannot use the original rpc session, // so use client session to send response. else { - dbg_dassert(response->to_address.port() > MAX_CLIENT_PORT, - "target address must have named port in this case"); + DCHECK_GT_MSG(response->to_address.port(), + MAX_CLIENT_PORT, + "target address must have named port in this case"); // use the header format recorded in the message auto rpc_channel = sp ? sp->rpc_call_channel : RPC_CHANNEL_TCP; @@ -789,8 +792,9 @@ void rpc_engine::reply(message_ex *response, error_code err) // not connection oriented network, we always use the named network to send msgs else { - dbg_dassert(response->to_address.port() > MAX_CLIENT_PORT, - "target address must have named port in this case"); + DCHECK_GT_MSG(response->to_address.port(), + MAX_CLIENT_PORT, + "target address must have named port in this case"); auto rpc_channel = sp ? sp->rpc_call_channel : RPC_CHANNEL_TCP; network *net = _server_nets[response->header->from_address.port()][rpc_channel].get(); diff --git a/src/runtime/rpc/rpc_message.cpp b/src/runtime/rpc/rpc_message.cpp index 7ef3999164..6b06621e99 100644 --- a/src/runtime/rpc/rpc_message.cpp +++ b/src/runtime/rpc/rpc_message.cpp @@ -119,7 +119,6 @@ message_ex *message_ex::create_receive_message(const blob &data) auto data2 = data.range((int)sizeof(message_header)); msg->buffers.push_back(data2); - // dbg_dassert(msg->header->body_length > 0, "message %s is empty!", msg->header->rpc_name); return msg; } diff --git a/src/runtime/rpc/rpc_stream.h b/src/runtime/rpc/rpc_stream.h index 45f3d9f7d7..6cfd789124 100644 --- a/src/runtime/rpc/rpc_stream.h +++ b/src/runtime/rpc/rpc_stream.h @@ -26,19 +26,20 @@ #pragma once -#include "utils/utils.h" -#include "utils/binary_reader.h" -#include "utils/binary_writer.h" -#include "runtime/rpc/rpc_message.h" -#include "utils/error_code.h" -#include "utils/threadpool_code.h" -#include "runtime/task/task_code.h" -#include "common/gpid.h" #include "common/api_common.h" -#include "runtime/api_task.h" +#include "common/gpid.h" #include "runtime/api_layer1.h" +#include "runtime/api_task.h" #include "runtime/app_model.h" +#include "runtime/rpc/rpc_message.h" +#include "runtime/task/task_code.h" #include "utils/api_utilities.h" +#include "utils/binary_reader.h" +#include "utils/binary_writer.h" +#include "utils/error_code.h" +#include "utils/fmt_logging.h" +#include "utils/threadpool_code.h" +#include "utils/utils.h" namespace dsn { @@ -120,7 +121,7 @@ class rpc_write_stream : public binary_writer void *ptr; size_t sz; _msg->write_next(&ptr, &sz, size); - dbg_dassert(sz >= size, "allocated buffer size must be not less than the required size"); + DCHECK_GE_MSG(sz, size, "allocated buffer size must be not less than the required size"); bb.assign((const char *)ptr, 0, (int)sz); _last_write_next_total_size = total_size(); diff --git a/src/runtime/rpc/rpc_task.cpp b/src/runtime/rpc/rpc_task.cpp index 58085e6d85..b15be93e59 100644 --- a/src/runtime/rpc/rpc_task.cpp +++ b/src/runtime/rpc/rpc_task.cpp @@ -35,10 +35,11 @@ rpc_request_task::rpc_request_task(message_ex *request, rpc_request_handler &&h, _handler(std::move(h)), _enqueue_ts_ns(0) { - dbg_dassert( - TASK_TYPE_RPC_REQUEST == spec().type, - "%s is not a RPC_REQUEST task, please use DEFINE_TASK_CODE_RPC to define the task code", - spec().name.c_str()); + DCHECK_EQ_MSG( + TASK_TYPE_RPC_REQUEST, + spec().type, + "{} is not a RPC_REQUEST task, please use DEFINE_TASK_CODE_RPC to define the task code", + spec().name); _request->add_ref(); // released in dctor } @@ -76,10 +77,11 @@ rpc_response_task::rpc_response_task(message_ex *request, set_error_code(ERR_IO_PENDING); - dbg_dassert(TASK_TYPE_RPC_RESPONSE == spec().type, - "%s is not of RPC_RESPONSE type, please use DEFINE_TASK_CODE_RPC to define the " - "request task code", - spec().name.c_str()); + DCHECK_EQ_MSG(TASK_TYPE_RPC_RESPONSE, + spec().type, + "{} is not of RPC_RESPONSE type, please use DEFINE_TASK_CODE_RPC to define the " + "request task code", + spec().name); _request = request; _response = nullptr; diff --git a/src/runtime/task/task_queue.cpp b/src/runtime/task/task_queue.cpp index a992efe9fe..6bb5fa629c 100644 --- a/src/runtime/task/task_queue.cpp +++ b/src/runtime/task/task_queue.cpp @@ -88,7 +88,7 @@ void task_queue::enqueue_internal(task *task) } } } else { - dbg_dassert(TM_REJECT == throttle_mode, "unknow mode %d", (int)throttle_mode); + DCHECK_EQ_MSG(TM_REJECT, throttle_mode, "unknow mode {}", throttle_mode); if (ac_value > _spec->queue_length_throttling_threshold) { auto rtask = static_cast(task); diff --git a/src/utils/fmt_logging.h b/src/utils/fmt_logging.h index 09d531090b..973fa8a8e5 100644 --- a/src/utils/fmt_logging.h +++ b/src/utils/fmt_logging.h @@ -47,13 +47,6 @@ } \ } while (false) -// TODO(yingchun): use DCHECK instead of dbg_dassert -#ifndef NDEBUG -#define dbg_dassert CHECK -#else -#define dbg_dassert(x, ...) -#endif - #define CHECK_NOTNULL(p, ...) CHECK(p != nullptr, __VA_ARGS__) // Macros for writing log message prefixed by log_prefix(). @@ -116,6 +109,13 @@ #define CHECK_LT(var1, var2) CHECK_LT_MSG(var1, var2, "") // TODO(yingchun): add CHECK_NULL(ptr), CHECK_OK(err), CHECK(cond) +#define CHECK_NE_PREFIX(var1, var2) \ + do { \ + const auto &_v1 = (var1); \ + const auto &_v2 = (var2); \ + dassert_replica(_v1 != _v2, "{} vs {}", _v1, _v2); \ + } while (false) + #define CHECK_EQ_PREFIX(var1, var2) \ do { \ const auto &_v1 = (var1); \ @@ -166,3 +166,53 @@ error_code _err = (s); \ ERR_LOG_AND_RETURN_NOT_TRUE(_err == ERR_OK, _err, __VA_ARGS__); \ } while (0) + +#ifndef NDEBUG +#define DCHECK CHECK +#define DCHECK_NOTNULL CHECK_NOTNULL + +#define DCHECK_NE_MSG CHECK_NE_MSG +#define DCHECK_EQ_MSG CHECK_EQ_MSG +#define DCHECK_GE_MSG CHECK_GE_MSG +#define DCHECK_LE_MSG CHECK_LE_MSG +#define DCHECK_GT_MSG CHECK_GT_MSG +#define DCHECK_LT_MSG CHECK_LT_MSG + +#define DCHECK_NE CHECK_NE +#define DCHECK_EQ CHECK_EQ +#define DCHECK_GE CHECK_GE +#define DCHECK_LE CHECK_LE +#define DCHECK_GT CHECK_GT +#define DCHECK_LT CHECK_LT + +#define DCHECK_NE_PREFIX CHECK_NE_PREFIX +#define DCHECK_EQ_PREFIX CHECK_EQ_PREFIX +#define DCHECK_GE_PREFIX CHECK_GE_PREFIX +#define DCHECK_LE_PREFIX CHECK_LE_PREFIX +#define DCHECK_GT_PREFIX CHECK_GT_PREFIX +#define DCHECK_LT_PREFIX CHECK_LT_PREFIX +#else +#define DCHECK(x, ...) +#define DCHECK_NOTNULL(p, ...) + +#define DCHECK_NE_MSG(var1, var2, ...) +#define DCHECK_EQ_MSG(var1, var2, ...) +#define DCHECK_GE_MSG(var1, var2, ...) +#define DCHECK_LE_MSG(var1, var2, ...) +#define DCHECK_GT_MSG(var1, var2, ...) +#define DCHECK_LT_MSG(var1, var2, ...) + +#define DCHECK_NE(var1, var2) +#define DCHECK_EQ(var1, var2) +#define DCHECK_GE(var1, var2) +#define DCHECK_LE(var1, var2) +#define DCHECK_GT(var1, var2) +#define DCHECK_LT(var1, var2) + +#define DCHECK_NE_PREFIX(var1, var2) +#define DCHECK_EQ_PREFIX(var1, var2) +#define DCHECK_GE_PREFIX(var1, var2) +#define DCHECK_LE_PREFIX(var1, var2) +#define DCHECK_GT_PREFIX(var1, var2) +#define DCHECK_LT_PREFIX(var1, var2) +#endif