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 2021/06/25 03:10:50 UTC
[GitHub] [incubator-pegasus] Shuo-Jia opened a new pull request #774: feat: add read/write amp perf-counter support
Shuo-Jia opened a new pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774
### What problem does this PR solve? <!--add issue link with summary if exists-->
[read/write amp](https://github.com/facebook/rocksdb/wiki/RocksDB-Tuning-Guide#amplification-factors) has influence with read/write latency, we need add perf-counter support
### Checklist <!--REMOVE the items that are not applicable-->
##### Tests <!-- At least one of them must be included. -->
- Unit test
- Manual test (add detailed scripts or steps below)
##### Side effects
read amp need external cache to obtain, detail see: https://github.com/XiaoMi/pegasus-rocksdb/blob/v6.6.4-compatible/include/rocksdb/table.h#L247
--
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.
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
[GitHub] [incubator-pegasus] Shuo-Jia commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Shuo-Jia commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658489890
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1532,13 +1535,20 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
[this]() { this->update_replica_rocksdb_statistics(); },
_update_rdb_stat_interval);
+ dinfo_replica("start the update rocksdb amp statistics timer task");
+ _update_replica_amp_stat =
+ ::dsn::tasking::enqueue_timer(LPC_REPLICATION_LONG_COMMON,
+ nullptr,
Review comment:
should use
--
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.
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
[GitHub] [incubator-pegasus] Shuo-Jia commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Shuo-Jia commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658510543
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -46,6 +46,7 @@ namespace pegasus {
namespace server {
DEFINE_TASK_CODE(LPC_PEGASUS_SERVER_DELAY, TASK_PRIORITY_COMMON, ::dsn::THREAD_POOL_DEFAULT)
+DSN_DECLARE_int32(read_amp_bytes_per_bit);
Review comment:
it actually pass to rocksdb, so won't be updated rocksdb though set mutable
--
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.
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
[GitHub] [incubator-pegasus] levy5307 commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658450443
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -2274,6 +2288,33 @@ void pegasus_server_impl::update_replica_rocksdb_statistics()
dinfo_replica("_pfc_rdb_bf_point_negatives: {}", bf_point_negatives);
}
+void pegasus_server_impl::update_replica_rocksdb_amp_statistics()
+{
+ // Update _pfc_rdb_write_amplification
+ std::map<std::string, std::string> props;
+ if (_db->GetMapProperty(_data_cf, "rocksdb.cfstats", &props)) {
+ auto write_amplification_iter = props.find("compaction.Sum.WriteAmp");
+ auto write_amplification = write_amplification_iter == props.end()
+ ? 1
+ : std::stod(write_amplification_iter->second);
+ _pfc_rdb_write_amplification->set(write_amplification);
+ dinfo_replica("_pfc_rdb_write_amplification: {}", write_amplification);
+ }
+
+ // Update _pfc_rdb_read_amplification
+ if (FLAGS_read_amp_bytes_per_bit > 0) {
+ int64_t estimate_useful_bytes =
Review comment:
uint64_t
--
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.
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
[GitHub] [incubator-pegasus] levy5307 commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658450253
##########
File path: src/server/pegasus_server_impl_init.cpp
##########
@@ -44,6 +44,20 @@ DSN_DEFINE_bool("pegasus.server",
false,
"whether to enable write rate auto tune when open rocksdb write limit");
+// detail see
+// https://github.com/XiaoMi/pegasus-rocksdb/blob/v6.6.4-compatible/include/rocksdb/table.h#L247
+DSN_DEFINE_int32("pegasus.server",
+ read_amp_bytes_per_bit,
+ 0,
+ "config for using to calculate the "
+ "read amplification, must be a power "
+ "of 2, zero means disable count read "
+ "amplification");
+
+DSN_DEFINE_validator(read_amp_bytes_per_bit, [](const int64_t read_amp_bytes_per_bit) -> bool {
+ return read_amp_bytes_per_bit >= 0;
Review comment:
``` return read_amp_bytes_per_bit >= 0 && read_amp_bytes_per_bit & (read_amp_bytes_per_bit - 1) == 0;```
--
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.
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
[GitHub] [incubator-pegasus] Smityz commented on pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Smityz commented on pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#issuecomment-868259566
LGTM
--
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.
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
[GitHub] [incubator-pegasus] levy5307 commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658501893
##########
File path: src/server/pegasus_server_impl.h
##########
@@ -26,6 +26,7 @@
#include <rocksdb/options.h>
#include <dsn/perf_counter/perf_counter_wrapper.h>
#include <dsn/dist/replication/replication.codes.h>
+#include <dsn/utility/flags.h>
Review comment:
remove this line
--
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.
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
[GitHub] [incubator-pegasus] Shuo-Jia commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Shuo-Jia commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r659456587
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -2274,6 +2289,33 @@ void pegasus_server_impl::update_replica_rocksdb_statistics()
dinfo_replica("_pfc_rdb_bf_point_negatives: {}", bf_point_negatives);
}
+void pegasus_server_impl::update_replica_rocksdb_amp_statistics()
+{
+ // Update _pfc_rdb_write_amplification
+ std::map<std::string, std::string> props;
+ if (_db->GetMapProperty(_data_cf, "rocksdb.cfstats", &props)) {
+ auto write_amplification_iter = props.find("compaction.Sum.WriteAmp");
+ auto write_amplification = write_amplification_iter == props.end()
+ ? 1
+ : std::stod(write_amplification_iter->second);
+ _pfc_rdb_write_amplification->set(write_amplification);
+ dinfo_replica("_pfc_rdb_write_amplification: {}", write_amplification);
+ }
+
+ // Update _pfc_rdb_read_amplification
+ if (FLAGS_read_amp_bytes_per_bit > 0) {
+ auto estimate_useful_bytes =
+ _statistics->getTickerCount(rocksdb::READ_AMP_ESTIMATE_USEFUL_BYTES);
Review comment:
https://github.com/facebook/rocksdb/blob/v6.6.4/include/rocksdb/statistics.h#L101-L106 it hit count, we need calc `hitRate` via them
--
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
[GitHub] [incubator-pegasus] Shuo-Jia commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Shuo-Jia commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658514375
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -2274,6 +2289,33 @@ void pegasus_server_impl::update_replica_rocksdb_statistics()
dinfo_replica("_pfc_rdb_bf_point_negatives: {}", bf_point_negatives);
}
+void pegasus_server_impl::update_replica_rocksdb_amp_statistics()
+{
+ // Update _pfc_rdb_write_amplification
+ std::map<std::string, std::string> props;
+ if (_db->GetMapProperty(_data_cf, "rocksdb.cfstats", &props)) {
+ auto write_amplification_iter = props.find("compaction.Sum.WriteAmp");
+ auto write_amplification = write_amplification_iter == props.end()
+ ? 1
+ : std::stod(write_amplification_iter->second);
+ _pfc_rdb_write_amplification->set(write_amplification);
+ dinfo_replica("_pfc_rdb_write_amplification: {}", write_amplification);
+ }
+
+ // Update _pfc_rdb_read_amplification
+ if (FLAGS_read_amp_bytes_per_bit > 0) {
+ auto estimate_useful_bytes =
+ _statistics->getTickerCount(rocksdb::READ_AMP_ESTIMATE_USEFUL_BYTES);
Review comment:
Oh, I haven't noticed, it seem hitRate under different level
--
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.
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
[GitHub] [incubator-pegasus] hycdong commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
hycdong commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658507052
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -46,6 +46,7 @@ namespace pegasus {
namespace server {
DEFINE_TASK_CODE(LPC_PEGASUS_SERVER_DELAY, TASK_PRIORITY_COMMON, ::dsn::THREAD_POOL_DEFAULT)
+DSN_DECLARE_int32(read_amp_bytes_per_bit);
Review comment:
I suggest this config mutable.
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -2274,6 +2289,33 @@ void pegasus_server_impl::update_replica_rocksdb_statistics()
dinfo_replica("_pfc_rdb_bf_point_negatives: {}", bf_point_negatives);
}
+void pegasus_server_impl::update_replica_rocksdb_amp_statistics()
+{
+ // Update _pfc_rdb_write_amplification
+ std::map<std::string, std::string> props;
+ if (_db->GetMapProperty(_data_cf, "rocksdb.cfstats", &props)) {
+ auto write_amplification_iter = props.find("compaction.Sum.WriteAmp");
+ auto write_amplification = write_amplification_iter == props.end()
+ ? 1
+ : std::stod(write_amplification_iter->second);
+ _pfc_rdb_write_amplification->set(write_amplification);
+ dinfo_replica("_pfc_rdb_write_amplification: {}", write_amplification);
+ }
+
+ // Update _pfc_rdb_read_amplification
+ if (FLAGS_read_amp_bytes_per_bit > 0) {
+ auto estimate_useful_bytes =
+ _statistics->getTickerCount(rocksdb::READ_AMP_ESTIMATE_USEFUL_BYTES);
Review comment:
https://github.com/facebook/rocksdb/blob/v6.6.4/monitoring/statistics.cc
I notice that there are other statistics similar to what we like, such as `GET_HIT_L0`, `GET_HIT_L1`, `GET_HIT_L2_AND_UP`, what it the difference between them?
--
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.
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
[GitHub] [incubator-pegasus] hycdong commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
hycdong commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658517142
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -2274,6 +2289,33 @@ void pegasus_server_impl::update_replica_rocksdb_statistics()
dinfo_replica("_pfc_rdb_bf_point_negatives: {}", bf_point_negatives);
}
+void pegasus_server_impl::update_replica_rocksdb_amp_statistics()
+{
+ // Update _pfc_rdb_write_amplification
+ std::map<std::string, std::string> props;
+ if (_db->GetMapProperty(_data_cf, "rocksdb.cfstats", &props)) {
+ auto write_amplification_iter = props.find("compaction.Sum.WriteAmp");
+ auto write_amplification = write_amplification_iter == props.end()
+ ? 1
+ : std::stod(write_amplification_iter->second);
+ _pfc_rdb_write_amplification->set(write_amplification);
+ dinfo_replica("_pfc_rdb_write_amplification: {}", write_amplification);
+ }
+
+ // Update _pfc_rdb_read_amplification
+ if (FLAGS_read_amp_bytes_per_bit > 0) {
+ auto estimate_useful_bytes =
+ _statistics->getTickerCount(rocksdb::READ_AMP_ESTIMATE_USEFUL_BYTES);
Review comment:
Yes~ I think they are actually what we need, more accurate than read amplification rate
--
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.
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
[GitHub] [incubator-pegasus] Shuo-Jia merged pull request #774: feat: add more rocksdb perf-counter support
Posted by GitBox <gi...@apache.org>.
Shuo-Jia merged pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774
--
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
[GitHub] [incubator-pegasus] Shuo-Jia commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Shuo-Jia commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658510543
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -46,6 +46,7 @@ namespace pegasus {
namespace server {
DEFINE_TASK_CODE(LPC_PEGASUS_SERVER_DELAY, TASK_PRIORITY_COMMON, ::dsn::THREAD_POOL_DEFAULT)
+DSN_DECLARE_int32(read_amp_bytes_per_bit);
Review comment:
it actually pass to rocksdb, so won't be updated fo rocksdb though set mutable
--
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.
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
[GitHub] [incubator-pegasus] levy5307 commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658448200
##########
File path: src/server/pegasus_server_impl.h
##########
@@ -39,7 +40,7 @@
namespace pegasus {
namespace server {
-
+DSN_DECLARE_int32(read_amp_bytes_per_bit);
Review comment:
move this to the corresponding cpp file
--
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.
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
[GitHub] [incubator-pegasus] Smityz commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658447292
##########
File path: src/server/pegasus_server_impl.cpp
##########
@@ -1532,13 +1535,20 @@ ::dsn::error_code pegasus_server_impl::start(int argc, char **argv)
[this]() { this->update_replica_rocksdb_statistics(); },
_update_rdb_stat_interval);
+ dinfo_replica("start the update rocksdb amp statistics timer task");
+ _update_replica_amp_stat =
+ ::dsn::tasking::enqueue_timer(LPC_REPLICATION_LONG_COMMON,
+ nullptr,
Review comment:
why not use tracker here
--
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.
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
[GitHub] [incubator-pegasus] Smityz commented on a change in pull request #774: feat: add read/write amp perf-counter support
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #774:
URL: https://github.com/apache/incubator-pegasus/pull/774#discussion_r658449025
##########
File path: src/server/pegasus_server_impl_init.cpp
##########
@@ -44,6 +44,20 @@ DSN_DEFINE_bool("pegasus.server",
false,
"whether to enable write rate auto tune when open rocksdb write limit");
+// detail see
+// https://github.com/XiaoMi/pegasus-rocksdb/blob/v6.6.4-compatible/include/rocksdb/table.h#L247
+DSN_DEFINE_int32("pegasus.server",
+ read_amp_bytes_per_bit,
+ 0,
+ "config for using to calculate the "
+ "read amplification, must be a power "
+ "of 2, zero means disable count read "
+ "amplification");
+
+DSN_DEFINE_validator(read_amp_bytes_per_bit, [](const int64_t read_amp_bytes_per_bit) -> bool {
Review comment:
you can validator here is `read_amp_bytes_per_bi`t a power of 2 or equal to 0
--
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.
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