acelyc111 commented on code in PR #1374:
URL:
https://github.com/apache/incubator-pegasus/pull/1374#discussion_r1126634892
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -277,21 +334,20 @@ void pegasus_server_impl::on_get(get_rpc rpc)
return;
}
+ METRIC_VAR_AUTO_LATENCY(get_latency_ns);
+
+ const auto &key = rpc.request();
rocksdb::Slice skey(key.data(), key.length());
std::string value;
rocksdb::Status status = _db->Get(_data_cf_rd_opts, _data_cf, skey,
&value);
if (status.ok()) {
if (check_if_record_expired(utils::epoch_now(), value)) {
- _pfc_recent_expire_count->increment();
- if (_verbose_log) {
- LOG_ERROR_PREFIX("rocksdb data expired for get from {}",
rpc.remote_address());
- }
+ METRIC_VAR_INCREMENT(read_expired_values);
+ log_expired_data_if_verbose(__FUNCTION__, rpc.remote_address(),
key);
Review Comment:
If using macro, `__FUNCTION__` can be omitted?
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -259,13 +259,70 @@ int
pegasus_server_impl::on_batched_write_requests(int64_t decree,
return _server_write->on_batched_write_requests(requests, count, decree,
timestamp);
}
+// Since LOG_ERROR_PREFIX depends on log_prefix(), this method could not be
declared as static or
Review Comment:
Is there any reason of why not using macros?
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -259,13 +259,70 @@ int
pegasus_server_impl::on_batched_write_requests(int64_t decree,
return _server_write->on_batched_write_requests(requests, count, decree,
timestamp);
}
+// Since LOG_ERROR_PREFIX depends on log_prefix(), this method could not be
declared as static or
+// with anonymous namespace.
+void pegasus_server_impl::log_expired_data(const char *op,
+ const dsn::rpc_address &addr,
+ const dsn::blob &hash_key,
+ const dsn::blob &sort_key) const
+{
+ LOG_ERROR_PREFIX("rocksdb data expired for {} from {}: hash_key = \"{}\",
sort_key = \"{}\"",
+ op,
+ addr,
+ pegasus::utils::c_escape_string(hash_key),
+ pegasus::utils::c_escape_string(sort_key));
+}
+
+void pegasus_server_impl::log_expired_data(const char *op,
+ const dsn::rpc_address &addr,
+ const dsn::blob &key) const
+{
+ dsn::blob hash_key, sort_key;
+ pegasus_restore_key(key, hash_key, sort_key);
+ log_expired_data(op, addr, hash_key, sort_key);
+}
+
+void pegasus_server_impl::log_expired_data_if_verbose(const char *op,
+ const dsn::rpc_address
&addr,
+ const dsn::blob
&hash_key,
+ const dsn::blob
&sort_key) const
+{
+ if (!_verbose_log) {
+ return;
+ }
+
+ log_expired_data(op, addr, hash_key, sort_key);
+}
+
+void pegasus_server_impl::log_expired_data_if_verbose(const char *op,
+ const dsn::rpc_address
&addr,
+ const dsn::blob &key)
const
+{
+ if (!_verbose_log) {
+ return;
+ }
+
+ log_expired_data(op, addr, key);
+}
+
+void pegasus_server_impl::log_expired_data_if_verbose(const char *op,
+ const dsn::rpc_address
&addr,
+ const rocksdb::Slice
&key) const
+{
+ if (!_verbose_log) {
Review Comment:
```suggestion
if (dsn_likely(!_verbose_log)) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
