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/18 09:14:19 UTC
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1318: refactor(log): use LOG_ERROR_F instead of LOG_ERROR (1/2)
empiredan commented on code in PR #1318:
URL: https://github.com/apache/incubator-pegasus/pull/1318#discussion_r1073233822
##########
src/replica/replica_stub.cpp:
##########
@@ -1633,15 +1631,15 @@ void replica_stub::response_client(gpid id,
_counter_recent_read_fail_count->increment();
else
_counter_recent_write_fail_count->increment();
- LOG_ERROR("%s@%s: %s fail: client = %s, code = %s, timeout = %d, status = %s, error = %s",
- id.to_string(),
- _primary_address_str,
- is_read ? "read" : "write",
- request == nullptr ? "null" : request->header->from_address.to_string(),
- request == nullptr ? "null" : request->header->rpc_name,
- request == nullptr ? 0 : request->header->client.timeout_ms,
- enum_to_string(status),
- error.to_string());
+ LOG_ERROR_F("{}@{}: {} fail: client = {}, code = {}, timeout = {}, status = {}, error = {}",
+ id,
+ _primary_address_str,
+ is_read ? "read" : "write",
+ request == nullptr ? "null" : request->header->from_address,
Review Comment:
Since `const char *` and `rpc_address` have different types, and cannot convert to each other:
```suggestion
request == nullptr ? "null" : request->header->from_address.to_string(),
```
##########
src/replica/replica_restore.cpp:
##########
@@ -44,7 +44,7 @@ bool replica::remove_useless_file_under_chkpt(const std::string &chkpt_dir,
// filename --> file_path such as: file --> ***/***/file
std::map<std::string, std::string> name_to_filepath;
if (!::dsn::utils::filesystem::get_subfiles(chkpt_dir, sub_files, false)) {
- LOG_ERROR("%s: get subfile of dir(%s) failed", name(), chkpt_dir.c_str());
+ LOG_ERROR_PREFIX("get subfile of dir(%s) failed", chkpt_dir);
Review Comment:
```suggestion
LOG_ERROR_PREFIX("get subfile of dir({}) failed", chkpt_dir);
```
##########
src/replica/log_file.cpp:
##########
@@ -203,27 +202,27 @@ error_code log_file::read_next_log_block(/*out*/ ::dsn::blob &bb)
// if read_count is 0, then we meet the end of file
err = (bb.length() == 0 ? ERR_HANDLE_EOF : ERR_INCOMPLETE_DATA);
} else {
- LOG_ERROR("read data block header failed, size = %d vs %d, err = %s",
- bb.length(),
- (int)sizeof(log_block_header),
- err.to_string());
+ LOG_ERROR_F("read data block header failed, size = {} vs {}, err = {}",
+ bb.length(),
+ (int)sizeof(log_block_header),
Review Comment:
```suggestion
sizeof(log_block_header),
```
##########
src/replica/replica_2pc.cpp:
##########
@@ -524,10 +518,8 @@ void replica::on_append_log_completed(mutation_ptr &mu, error_code err, size_t s
if (err == ERR_OK) {
mu->set_logged();
} else {
- LOG_ERROR("%s: append shared log failed for mutation %s, err = %s",
- name(),
- mu->name(),
- err.to_string());
+ LOG_ERROR_PREFIX(
+ "append shared log failed for mutation {}, err = {}", mu->name(), err.to_string());
Review Comment:
```suggestion
"append shared log failed for mutation {}, err = {}", mu->name(), err);
```
##########
src/replica/log_file.cpp:
##########
@@ -203,27 +202,27 @@ error_code log_file::read_next_log_block(/*out*/ ::dsn::blob &bb)
// if read_count is 0, then we meet the end of file
err = (bb.length() == 0 ? ERR_HANDLE_EOF : ERR_INCOMPLETE_DATA);
} else {
- LOG_ERROR("read data block header failed, size = %d vs %d, err = %s",
- bb.length(),
- (int)sizeof(log_block_header),
- err.to_string());
+ LOG_ERROR_F("read data block header failed, size = {} vs {}, err = {}",
+ bb.length(),
+ (int)sizeof(log_block_header),
+ err);
}
return err;
}
log_block_header hdr = *reinterpret_cast<const log_block_header *>(bb.data());
if (hdr.magic != 0xdeadbeef) {
- LOG_ERROR("invalid data header magic: 0x%x", hdr.magic);
+ LOG_ERROR_F("invalid data header magic: {:#x}", static_cast<uint32_t>(hdr.magic));
return ERR_INVALID_DATA;
}
err = _stream->read_next(hdr.length, bb);
if (err != ERR_OK || hdr.length != bb.length()) {
- LOG_ERROR("read data block body failed, size = %d vs %d, err = %s",
- bb.length(),
- (int)hdr.length,
- err.to_string());
+ LOG_ERROR_F("read data block body failed, size = {} vs {}, err = {}",
+ bb.length(),
+ (int)hdr.length,
Review Comment:
```suggestion
hdr.length,
```
--
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