This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 5ab400dea refactor(log): make macros LOG_AND_RETURN_NOT_* more general
(#1457)
5ab400dea is described below
commit 5ab400deafdd8b4bdfe2f18b360bb829ed084709
Author: Yingchun Lai <[email protected]>
AuthorDate: Thu Apr 20 23:19:03 2023 +0800
refactor(log): make macros LOG_AND_RETURN_NOT_* more general (#1457)
https://github.com/apache/incubator-pegasus/issues/887
Update `ERR_LOG_AND_RETURN_NOT_*` to `LOG_AND_RETURN_NOT_*` with a user
specified level, they would be more general to use.
---
src/replica/replication_app_base.cpp | 133 +++++++++++----------
.../ranger/ranger_resource_policy_manager.cpp | 15 +--
src/utils/fmt_logging.h | 10 +-
3 files changed, 86 insertions(+), 72 deletions(-)
diff --git a/src/replica/replication_app_base.cpp
b/src/replica/replication_app_base.cpp
index 8b20d6df8..b1123efe1 100644
--- a/src/replica/replication_app_base.cpp
+++ b/src/replica/replication_app_base.cpp
@@ -80,7 +80,8 @@ error_code write_blob_to_file(const std::string &file, const
blob &data)
{
std::string tmp_file = file + ".tmp";
disk_file *hfile = file::open(tmp_file.c_str(), O_WRONLY | O_CREAT |
O_BINARY | O_TRUNC, 0666);
- ERR_LOG_AND_RETURN_NOT_TRUE(hfile, ERR_FILE_OPERATION_FAILED, "open file
{} failed", tmp_file);
+ LOG_AND_RETURN_NOT_TRUE(
+ ERROR, hfile, ERR_FILE_OPERATION_FAILED, "open file {} failed",
tmp_file);
auto cleanup = defer([tmp_file]() {
utils::filesystem::remove_path(tmp_file); });
error_code err;
@@ -101,14 +102,15 @@ error_code write_blob_to_file(const std::string &file,
const blob &data)
tracker.wait_outstanding_tasks();
file::flush(hfile);
file::close(hfile);
- ERR_LOG_AND_RETURN_NOT_OK(err, "write file {} failed", tmp_file);
+ LOG_AND_RETURN_NOT_OK(ERROR, err, "write file {} failed", tmp_file);
CHECK_EQ(data.length(), sz);
// TODO(yingchun): need fsync too？
- ERR_LOG_AND_RETURN_NOT_TRUE(utils::filesystem::rename_path(tmp_file, file),
- ERR_FILE_OPERATION_FAILED,
- "move file from {} to {} failed",
- tmp_file,
- file);
+ LOG_AND_RETURN_NOT_TRUE(ERROR,
+ utils::filesystem::rename_path(tmp_file, file),
+ ERR_FILE_OPERATION_FAILED,
+ "move file from {} to {} failed",
+ tmp_file,
+ file);
return ERR_OK;
}
@@ -117,12 +119,13 @@ error_code write_blob_to_file(const std::string &file,
const blob &data)
error_code replica_init_info::load(const std::string &dir)
{
std::string info_path = utils::filesystem::path_combine(dir, kInitInfo);
- ERR_LOG_AND_RETURN_NOT_TRUE(utils::filesystem::path_exists(info_path),
- ERR_PATH_NOT_FOUND,
- "file({}) not exist",
- info_path);
- ERR_LOG_AND_RETURN_NOT_OK(
- load_json(info_path), "load replica_init_info from {} failed",
info_path);
+ LOG_AND_RETURN_NOT_TRUE(ERROR,
+ utils::filesystem::path_exists(info_path),
+ ERR_PATH_NOT_FOUND,
+ "file({}) not exist",
+ info_path);
+ LOG_AND_RETURN_NOT_OK(
+ ERROR, load_json(info_path), "load replica_init_info from {} failed",
info_path);
LOG_INFO("load replica_init_info from {} succeed: {}", info_path,
to_string());
return ERR_OK;
}
@@ -131,10 +134,11 @@ error_code replica_init_info::store(const std::string
&dir)
{
uint64_t start = dsn_now_ns();
std::string info_path = utils::filesystem::path_combine(dir, kInitInfo);
- ERR_LOG_AND_RETURN_NOT_OK(store_json(info_path),
- "store replica_init_info to {} failed,
time_used_ns = {}",
- info_path,
- dsn_now_ns() - start);
+ LOG_AND_RETURN_NOT_OK(ERROR,
+ store_json(info_path),
+ "store replica_init_info to {} failed, time_used_ns
= {}",
+ info_path,
+ dsn_now_ns() - start);
LOG_INFO("store replica_init_info to {} succeed, time_used_ns = {}: {}",
info_path,
dsn_now_ns() - start,
@@ -145,21 +149,24 @@ error_code replica_init_info::store(const std::string
&dir)
error_code replica_init_info::load_json(const std::string &file)
{
std::ifstream is(file, std::ios::binary);
- ERR_LOG_AND_RETURN_NOT_TRUE(
- is.is_open(), ERR_FILE_OPERATION_FAILED, "open file {} failed", file);
+ LOG_AND_RETURN_NOT_TRUE(
+ ERROR, is.is_open(), ERR_FILE_OPERATION_FAILED, "open file {} failed",
file);
int64_t sz = 0;
-
ERR_LOG_AND_RETURN_NOT_TRUE(utils::filesystem::file_size(std::string(file), sz),
- ERR_FILE_OPERATION_FAILED,
- "get file size of {} failed",
- file);
+ LOG_AND_RETURN_NOT_TRUE(ERROR,
+ utils::filesystem::file_size(std::string(file),
sz),
+ ERR_FILE_OPERATION_FAILED,
+ "get file size of {} failed",
+ file);
std::shared_ptr<char> buffer(utils::make_shared_array<char>(sz));
is.read((char *)buffer.get(), sz);
- ERR_LOG_AND_RETURN_NOT_TRUE(!is.bad(), ERR_FILE_OPERATION_FAILED, "read
file {} failed", file);
+ LOG_AND_RETURN_NOT_TRUE(
+ ERROR, !is.bad(), ERR_FILE_OPERATION_FAILED, "read file {} failed",
file);
is.close();
- ERR_LOG_AND_RETURN_NOT_TRUE(
+ LOG_AND_RETURN_NOT_TRUE(
+ ERROR,
json::json_forwarder<replica_init_info>::decode(blob(buffer, sz),
*this),
ERR_FILE_OPERATION_FAILED,
"decode json from file {} failed",
@@ -186,14 +193,15 @@ std::string replica_init_info::to_string()
error_code replica_app_info::load(const std::string &file)
{
std::ifstream is(file, std::ios::binary);
- ERR_LOG_AND_RETURN_NOT_TRUE(
- is.is_open(), ERR_FILE_OPERATION_FAILED, "open file {} failed", file);
+ LOG_AND_RETURN_NOT_TRUE(
+ ERROR, is.is_open(), ERR_FILE_OPERATION_FAILED, "open file {} failed",
file);
int64_t sz = 0;
-
ERR_LOG_AND_RETURN_NOT_TRUE(utils::filesystem::file_size(std::string(file), sz),
- ERR_FILE_OPERATION_FAILED,
- "get file size of {} failed",
- file);
+ LOG_AND_RETURN_NOT_TRUE(ERROR,
+ utils::filesystem::file_size(std::string(file),
sz),
+ ERR_FILE_OPERATION_FAILED,
+ "get file size of {} failed",
+ file);
std::shared_ptr<char> buffer(utils::make_shared_array<char>(sz));
is.read((char *)buffer.get(), sz);
@@ -203,8 +211,8 @@ error_code replica_app_info::load(const std::string &file)
int magic;
unmarshall(reader, magic, DSF_THRIFT_BINARY);
- ERR_LOG_AND_RETURN_NOT_TRUE(
- magic == 0xdeadbeef, ERR_INVALID_DATA, "data in file {} is invalid
(magic)", file);
+ LOG_AND_RETURN_NOT_TRUE(
+ ERROR, magic == 0xdeadbeef, ERR_INVALID_DATA, "data in file {} is
invalid (magic)", file);
unmarshall(reader, *_app, DSF_THRIFT_JSON);
return ERR_OK;
@@ -278,26 +286,28 @@ const ballot &replication_app_base::get_ballot() const {
return _replica->get_ba
error_code replication_app_base::open_internal(replica *r)
{
- ERR_LOG_AND_RETURN_NOT_TRUE(utils::filesystem::directory_exists(_dir_data),
- ERR_FILE_OPERATION_FAILED,
- "[{}]: replica data dir {} does not exist",
- r->name(),
- _dir_data);
+ LOG_AND_RETURN_NOT_TRUE(ERROR_PREFIX,
+ utils::filesystem::directory_exists(_dir_data),
+ ERR_FILE_OPERATION_FAILED,
+ "[{}]: replica data dir {} does not exist",
+ r->name(),
+ _dir_data);
- ERR_LOG_AND_RETURN_NOT_OK(open(), "[{}]: open replica app failed",
r->name());
+ LOG_AND_RETURN_NOT_OK(ERROR_PREFIX, open(), "[{}]: open replica app
failed", r->name());
_last_committed_decree = last_durable_decree();
auto err = _info.load(r->dir());
- ERR_LOG_AND_RETURN_NOT_OK(err, "[{}]: load replica_init_info failed",
r->name());
+ LOG_AND_RETURN_NOT_OK(ERROR_PREFIX, err, "[{}]: load replica_init_info
failed", r->name());
- ERR_LOG_AND_RETURN_NOT_TRUE(err != ERR_OK || last_durable_decree() >=
_info.init_durable_decree,
- ERR_INCOMPLETE_DATA,
- "[{}]: replica data is not complete coz "
- "last_durable_decree({}) <
init_durable_decree({})",
- r->name(),
- last_durable_decree(),
- _info.init_durable_decree);
+ LOG_AND_RETURN_NOT_TRUE(ERROR_PREFIX,
+ err != ERR_OK || last_durable_decree() >=
_info.init_durable_decree,
+ ERR_INCOMPLETE_DATA,
+ "[{}]: replica data is not complete coz "
+ "last_durable_decree({}) <
init_durable_decree({})",
+ r->name(),
+ last_durable_decree(),
+ _info.init_durable_decree);
return ERR_OK;
}
@@ -308,17 +318,19 @@ error_code
replication_app_base::open_new_internal(replica *r,
{
CHECK(utils::filesystem::remove_path(_dir_data), "remove data dir {}
failed", _dir_data);
CHECK(utils::filesystem::create_directory(_dir_data), "create data dir {}
failed", _dir_data);
- ERR_LOG_AND_RETURN_NOT_TRUE(utils::filesystem::directory_exists(_dir_data),
- ERR_FILE_OPERATION_FAILED,
- "[{}]: create replica data dir {} failed",
- r->name(),
- _dir_data);
-
- ERR_LOG_AND_RETURN_NOT_OK(open(), "[{}]: open replica app failed",
r->name());
+ LOG_AND_RETURN_NOT_TRUE(ERROR_PREFIX,
+ utils::filesystem::directory_exists(_dir_data),
+ ERR_FILE_OPERATION_FAILED,
+ "[{}]: create replica data dir {} failed",
+ r->name(),
+ _dir_data);
+
+ LOG_AND_RETURN_NOT_OK(ERROR_PREFIX, open(), "[{}]: open replica app
failed", r->name());
_last_committed_decree = last_durable_decree();
- ERR_LOG_AND_RETURN_NOT_OK(update_init_info(_replica, shared_log_start,
private_log_start, 0),
- "[{}]: open replica app failed",
- r->name());
+ LOG_AND_RETURN_NOT_OK(ERROR_PREFIX,
+ update_init_info(_replica, shared_log_start,
private_log_start, 0),
+ "[{}]: open replica app failed",
+ r->name());
return ERR_OK;
}
@@ -355,7 +367,8 @@ error_code replication_app_base::open()
error_code replication_app_base::close(bool clear_state)
{
- ERR_LOG_AND_RETURN_NOT_OK(stop(clear_state), "[{}]: stop storage failed",
replica_name());
+ LOG_AND_RETURN_NOT_OK(
+ ERROR_PREFIX, stop(clear_state), "[{}]: stop storage failed",
replica_name());
_last_committed_decree.store(0);
@@ -502,8 +515,8 @@ error_code replication_app_base::update_init_info(replica
*r,
_info.init_offset_in_shared_log = shared_log_offset;
_info.init_offset_in_private_log = private_log_offset;
- ERR_LOG_AND_RETURN_NOT_OK(
- _info.store(r->dir()), "[{}]: store replica_init_info failed",
r->name());
+ LOG_AND_RETURN_NOT_OK(
+ ERROR_PREFIX, _info.store(r->dir()), "[{}]: store replica_init_info
failed", r->name());
return ERR_OK;
}
diff --git a/src/runtime/ranger/ranger_resource_policy_manager.cpp
b/src/runtime/ranger/ranger_resource_policy_manager.cpp
index 440bcee71..d08df9466 100644
--- a/src/runtime/ranger/ranger_resource_policy_manager.cpp
+++ b/src/runtime/ranger/ranger_resource_policy_manager.cpp
@@ -298,8 +298,8 @@ void
ranger_resource_policy_manager::parse_policies_from_json(const rapidjson::V
dsn::error_code
ranger_resource_policy_manager::update_policies_from_ranger_service()
{
std::string ranger_policies;
-
ERR_LOG_AND_RETURN_NOT_OK(pull_policies_from_ranger_service(&ranger_policies),
- "Pull Ranger policies failed.");
+ LOG_AND_RETURN_NOT_OK(
+ ERROR, pull_policies_from_ranger_service(&ranger_policies), "Pull
Ranger policies failed.");
LOG_DEBUG("Pull Ranger policies success.");
auto err_code = load_policies_from_json(ranger_policies);
@@ -308,10 +308,11 @@ dsn::error_code
ranger_resource_policy_manager::update_policies_from_ranger_serv
// For the newly created table, its app envs must be empty. This needs
to be executed
// periodically to update the table's app envs, regardless of whether
the Ranger policy is
// updated or not.
- ERR_LOG_AND_RETURN_NOT_OK(sync_policies_to_app_envs(), "Sync policies
to app envs failed.");
+ LOG_AND_RETURN_NOT_OK(
+ ERROR, sync_policies_to_app_envs(), "Sync policies to app envs
failed.");
return dsn::ERR_OK;
}
- ERR_LOG_AND_RETURN_NOT_OK(err_code, "Parse Ranger policies failed.");
+ LOG_AND_RETURN_NOT_OK(ERROR, err_code, "Parse Ranger policies failed.");
start_to_dump_and_sync_policies();
@@ -579,7 +580,7 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
dsn::replication::configuration_list_apps_request list_req;
list_req.status = dsn::app_status::AS_AVAILABLE;
_meta_svc->get_server_state()->list_apps(list_req, list_resp);
- ERR_LOG_AND_RETURN_NOT_OK(list_resp.err, "list_apps failed.");
+ LOG_AND_RETURN_NOT_OK(ERROR, list_resp.err, "list_apps failed.");
for (const auto &app : list_resp.infos) {
std::string database_name =
get_database_name_from_app_name(app.app_name);
// Use "*" for table name of invalid Ranger rules to match datdabase
resources.
@@ -609,7 +610,7 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
dsn::replication::update_app_env_rpc rpc(std::move(req),
LPC_USE_RANGER_ACCESS_CONTROL);
_meta_svc->get_server_state()->set_app_envs(rpc);
- ERR_LOG_AND_RETURN_NOT_OK(rpc.response().err, "set_app_envs
failed.");
+ LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "set_app_envs
failed.");
break;
}
}
@@ -620,7 +621,7 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
dsn::replication::update_app_env_rpc rpc(std::move(req),
LPC_USE_RANGER_ACCESS_CONTROL);
_meta_svc->get_server_state()->del_app_envs(rpc);
- ERR_LOG_AND_RETURN_NOT_OK(rpc.response().err, "del_app_envs
failed.");
+ LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "del_app_envs
failed.");
}
}
diff --git a/src/utils/fmt_logging.h b/src/utils/fmt_logging.h
index 4b6b927f6..06db36ca7 100644
--- a/src/utils/fmt_logging.h
+++ b/src/utils/fmt_logging.h
@@ -256,19 +256,19 @@ inline const char *null_str_printer(const char *s) {
return s == nullptr ? "(nul
#define CHECK_LT_PREFIX(var1, var2) CHECK_LT_PREFIX_MSG(var1, var2, "")
// Return the given status if condition is not true.
-#define ERR_LOG_AND_RETURN_NOT_TRUE(s, err, ...)
\
+#define LOG_AND_RETURN_NOT_TRUE(level, s, err, ...)
\
do {
\
if (dsn_unlikely(!(s))) {
\
- LOG_ERROR("{}: {}", err, fmt::format(__VA_ARGS__));
\
+ LOG_##level("{}: {}", err, fmt::format(__VA_ARGS__));
\
return err;
\
}
\
} while (0)
// Return the given status if it is not ERR_OK.
-#define ERR_LOG_AND_RETURN_NOT_OK(s, ...)
\
+#define LOG_AND_RETURN_NOT_OK(level, s, ...)
\
do {
\
- error_code _err = (s);
\
- ERR_LOG_AND_RETURN_NOT_TRUE(_err == ERR_OK, _err, __VA_ARGS__);
\
+ ::dsn::error_code _err = (s);
\
+ LOG_AND_RETURN_NOT_TRUE(level, _err == ::dsn::ERR_OK, _err,
__VA_ARGS__); \
} while (0)
#ifndef NDEBUG
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
