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 2020/09/18 03:44:30 UTC
[GitHub] [incubator-pegasus] Smityz opened a new pull request #604: update
Smityz opened a new pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604
### What problem does this PR solve? <!--add issue link with summary if exists-->
Add a policy for `hotspot_partition_calculator` to send RPC to start hotkey detect in the replica_server auto.
### What is changed and how it works?
we can set `enable_hotkey_auto_detect = true` to start auto detect hotkey, and set `hot_partition_threshold` and `occurrence_threshold` to adjust sensitivity.
## Config changes
```diff
[pegasus.collector]
+ enable_hotkey_auto_detect
+ hot_partition_threshold
+ occurrence_threshold
```
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
hycdong commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r492438778
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
`expect_result` is set on Line 142, why set the same value again?
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r491913903
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -128,15 +145,44 @@ void hotspot_partition_calculator::data_analyse()
stat_histories_analyse(data_type, hot_points);
update_hot_point(data_type, hot_points);
}
+ if (!FLAGS_enable_hotkey_detect) {
+ return;
+ }
+ for (int data_type = 0; data_type <= 1; data_type++) {
+ detect_hotkey_in_hotpartition(data_type);
+ }
+}
+
+void hotspot_partition_calculator::detect_hotkey_in_hotpartition(int data_type)
+{
+ for (int index = 0; index < _hot_points.size(); index++) {
+ if (_hot_points[index][data_type].get()->get_value() >= FLAGS_hot_partition_threshold) {
+ if (++_hotpartition_pool[index][data_type] >= FLAGS_occurrence_threshold) {
+ derror_f("Find a {} hot partition {}.{}",
+ (data_type == partition_qps_type::READ_HOTSPOT_DATA ? "read" : "write"),
+ _app_name,
+ index);
+ send_hotkey_detect_request(_app_name,
+ index,
+ (data_type == dsn::apps::hotkey_type::type::READ)
+ ? dsn::apps::hotkey_type::type::READ
+ : dsn::apps::hotkey_type::type::WRITE,
+ dsn::apps::hotkey_detect_action::type::START);
+ }
+ } else {
+ _hotpartition_pool[index][data_type] =
+ std::max(_hotpartition_pool[index][data_type] - 1, 0);
Review comment:
I add some comments in the codes
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r491913903
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -128,15 +145,44 @@ void hotspot_partition_calculator::data_analyse()
stat_histories_analyse(data_type, hot_points);
update_hot_point(data_type, hot_points);
}
+ if (!FLAGS_enable_hotkey_detect) {
+ return;
+ }
+ for (int data_type = 0; data_type <= 1; data_type++) {
+ detect_hotkey_in_hotpartition(data_type);
+ }
+}
+
+void hotspot_partition_calculator::detect_hotkey_in_hotpartition(int data_type)
+{
+ for (int index = 0; index < _hot_points.size(); index++) {
+ if (_hot_points[index][data_type].get()->get_value() >= FLAGS_hot_partition_threshold) {
+ if (++_hotpartition_pool[index][data_type] >= FLAGS_occurrence_threshold) {
+ derror_f("Find a {} hot partition {}.{}",
+ (data_type == partition_qps_type::READ_HOTSPOT_DATA ? "read" : "write"),
+ _app_name,
+ index);
+ send_hotkey_detect_request(_app_name,
+ index,
+ (data_type == dsn::apps::hotkey_type::type::READ)
+ ? dsn::apps::hotkey_type::type::READ
+ : dsn::apps::hotkey_type::type::WRITE,
+ dsn::apps::hotkey_detect_action::type::START);
+ }
+ } else {
+ _hotpartition_pool[index][data_type] =
+ std::max(_hotpartition_pool[index][data_type] - 1, 0);
Review comment:
I add some comments in the codes
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
It is a different test case, expext result changed
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
It is a different test case, expect result changed
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490689971
##########
File path: src/server/hotspot_partition_calculator.h
##########
@@ -18,12 +18,15 @@
#pragma once
#include "hotspot_partition_stat.h"
+#include <dsn/utility/flags.h>
Review comment:
move `#include <dsn/utility/flags.h>` to behind of `#include <gtest/gtest_prod.h>`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490690545
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -19,14 +19,19 @@
#include "pegasus_server_test_base.h"
#include <gtest/gtest.h>
+#include <dsn/utility/fail_point.h>
namespace pegasus {
namespace server {
class hotspot_partition_test : public pegasus_server_test_base
{
public:
- hotspot_partition_test() : calculator("TEST", 8){};
+ hotspot_partition_test() : calculator("TEST", 8)
+ {
+ dsn::fail::setup();
+ dsn::fail::cfg("send_hotkey_detect_request", "return()");
Review comment:
you should call `dsn::fail::teardown()` in the dstor of `hotspot_partition_test`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r492445351
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
It is a different test case, expext result changed
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490735012
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +114,30 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
+ aggregate_analyse_data(test_rows, calculator);
Review comment:
` aggregate_analyse_data(generate_row_data(), calculator);`
remove the `test_rows`, so you can reduce the copy count of `std::vector<row_data>` by 1.
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r492445351
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
It is a different test case, expect result changed
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
hycdong commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r492438778
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
`expect_result` is set on Line 142, why set the same value again?
----------------------------------------------------------------
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] acelyc111 merged pull request #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
acelyc111 merged pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490689464
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -128,6 +147,36 @@ void hotspot_partition_calculator::data_analyse()
stat_histories_analyse(data_type, hot_points);
update_hot_point(data_type, hot_points);
}
+ if (!FLAGS_enable_hotkey_auto_detect) {
+ return;
+ }
+ for (int data_type = 0; data_type <= 1; data_type++) {
+ detect_hotkey_in_hotpartition(data_type);
+ }
+}
+
+void hotspot_partition_calculator::detect_hotkey_in_hotpartition(int data_type)
+{
+ for (int index = 0; index < _hot_points.size(); index++) {
+ if (_hot_points[index][data_type].get()->get_value() >= FLAGS_hot_partition_threshold) {
+ if (++_hotpartition_pool[index][data_type] >= FLAGS_occurrence_threshold) {
+ derror_f("Find a {} hot partition {}.{}",
+ (data_type == partition_qps_type::READ_HOTSPOT_DATA ? "read" : "write"),
+ _app_name,
+ index);
+ FAIL_POINT_INJECT_F("send_hotkey_detect_request", [](dsn::string_view) {});
Review comment:
you can move this fail point into func `send_hotkey_detect_request`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
hycdong commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r491781213
##########
File path: src/server/hotspot_partition_calculator.h
##########
@@ -63,7 +66,10 @@ class hotspot_partition_calculator
// saving historical data can improve accuracy
stat_histories _partitions_stat_histories;
+ std::vector<std::array<int, 2>> _hotpartition_pool;
Review comment:
Add comments for this structure to explain the meaning of `array<int, 2>`
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -128,15 +145,44 @@ void hotspot_partition_calculator::data_analyse()
stat_histories_analyse(data_type, hot_points);
update_hot_point(data_type, hot_points);
}
+ if (!FLAGS_enable_hotkey_detect) {
+ return;
+ }
+ for (int data_type = 0; data_type <= 1; data_type++) {
+ detect_hotkey_in_hotpartition(data_type);
+ }
+}
+
+void hotspot_partition_calculator::detect_hotkey_in_hotpartition(int data_type)
+{
+ for (int index = 0; index < _hot_points.size(); index++) {
+ if (_hot_points[index][data_type].get()->get_value() >= FLAGS_hot_partition_threshold) {
+ if (++_hotpartition_pool[index][data_type] >= FLAGS_occurrence_threshold) {
+ derror_f("Find a {} hot partition {}.{}",
+ (data_type == partition_qps_type::READ_HOTSPOT_DATA ? "read" : "write"),
+ _app_name,
+ index);
+ send_hotkey_detect_request(_app_name,
+ index,
+ (data_type == dsn::apps::hotkey_type::type::READ)
+ ? dsn::apps::hotkey_type::type::READ
+ : dsn::apps::hotkey_type::type::WRITE,
+ dsn::apps::hotkey_detect_action::type::START);
+ }
+ } else {
+ _hotpartition_pool[index][data_type] =
+ std::max(_hotpartition_pool[index][data_type] - 1, 0);
Review comment:
Why try to decrease the value of `_hotpartition_pool[index][data_type]`? Actually, I don't know the usage of `_hotpartition_pool`.
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490691916
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -58,6 +63,16 @@ class hotspot_partition_test : public pegasus_server_test_base
std::vector<std::vector<double>> result = get_calculator_result(calculator._hot_points);
ASSERT_EQ(result, expect_result);
}
+ void aggregate_analyse_data(std::vector<row_data> scenario,
Review comment:
`std::vector<row_data> &scenario`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
hycdong commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r491781213
##########
File path: src/server/hotspot_partition_calculator.h
##########
@@ -63,7 +66,10 @@ class hotspot_partition_calculator
// saving historical data can improve accuracy
stat_histories _partitions_stat_histories;
+ std::vector<std::array<int, 2>> _hotpartition_pool;
Review comment:
Add comments for this structure to explain the meaning of `array<int, 2>`
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -128,15 +145,44 @@ void hotspot_partition_calculator::data_analyse()
stat_histories_analyse(data_type, hot_points);
update_hot_point(data_type, hot_points);
}
+ if (!FLAGS_enable_hotkey_detect) {
+ return;
+ }
+ for (int data_type = 0; data_type <= 1; data_type++) {
+ detect_hotkey_in_hotpartition(data_type);
+ }
+}
+
+void hotspot_partition_calculator::detect_hotkey_in_hotpartition(int data_type)
+{
+ for (int index = 0; index < _hot_points.size(); index++) {
+ if (_hot_points[index][data_type].get()->get_value() >= FLAGS_hot_partition_threshold) {
+ if (++_hotpartition_pool[index][data_type] >= FLAGS_occurrence_threshold) {
+ derror_f("Find a {} hot partition {}.{}",
+ (data_type == partition_qps_type::READ_HOTSPOT_DATA ? "read" : "write"),
+ _app_name,
+ index);
+ send_hotkey_detect_request(_app_name,
+ index,
+ (data_type == dsn::apps::hotkey_type::type::READ)
+ ? dsn::apps::hotkey_type::type::READ
+ : dsn::apps::hotkey_type::type::WRITE,
+ dsn::apps::hotkey_detect_action::type::START);
+ }
+ } else {
+ _hotpartition_pool[index][data_type] =
+ std::max(_hotpartition_pool[index][data_type] - 1, 0);
Review comment:
Why try to decrease the value of `_hotpartition_pool[index][data_type]`? Actually, I don't know the usage of `_hotpartition_pool`.
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
`expect_result` is set on Line 142, why set the same value again?
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490692037
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +107,29 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int HOT_SCENARIO_0_READ_HOT_PARTITION = 7;
+ const int HOT_SCENARIO_0_WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[HOT_SCENARIO_0_READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[HOT_SCENARIO_0_WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
+ aggregate_analyse_data(test_rows, calculator);
+ }
+ std::vector<std::array<int, 2>> expect_result = {
+ {0, 100}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {100, 0}};
+ ASSERT_EQ(calculator._hotpartition_pool, expect_result);
+
+ test_rows = generate_row_data();
+ for (int i = 0; i < 50; i++) {
Review comment:
what's the meaning of number `50`?
we should avoid immediate value in our program
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490692965
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -42,6 +43,24 @@ DSN_DEFINE_int64("pegasus.collector",
"eliminate outdated historical "
"data");
+DSN_DEFINE_bool("pegasus.collector",
+ enable_hotkey_auto_detect,
+ false,
+ "auto detect hot key in the hot paritition");
+
+// if hot_partition_counter >= FLAGS_hotpartition_threshold, This partition is a hot partition
+DSN_DEFINE_int32("pegasus.collector",
Review comment:
I think you can move these comments into the descripton param of `DSN_DEFINE_xxx`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490742047
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +114,30 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
+ aggregate_analyse_data(test_rows, calculator);
+ }
+ std::vector<std::array<int, 2>> expect_result = {
+ {0, 100}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {100, 0}};
+ ASSERT_EQ(calculator._hotpartition_pool, expect_result);
+
+ test_rows = generate_row_data();
+ const int back_to_normal = 50;
+ for (int i = 0; i < back_to_normal; i++) {
+ aggregate_analyse_data(test_rows, calculator);
+ }
+ expect_result = {{0, 50}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {50, 0}};
Review comment:
Does this `50` has relative with `back_to_normal`?
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490735012
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +114,30 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
+ aggregate_analyse_data(test_rows, calculator);
Review comment:
` aggregate_analyse_data(generate_row_data(), calculator);`
remove the `test_rows`, so you can reduce the copy count of `std::vector<row_data>`.
----------------------------------------------------------------
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] acelyc111 merged pull request #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
acelyc111 merged pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490689971
##########
File path: src/server/hotspot_partition_calculator.h
##########
@@ -18,12 +18,15 @@
#pragma once
#include "hotspot_partition_stat.h"
+#include <dsn/utility/flags.h>
Review comment:
move `#include <dsn/utility/flags.h>` behind of `#include <gtest/gtest_prod.h>`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490690712
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -58,6 +63,16 @@ class hotspot_partition_test : public pegasus_server_test_base
std::vector<std::vector<double>> result = get_calculator_result(calculator._hot_points);
ASSERT_EQ(result, expect_result);
}
+ void aggregate_analyse_data(std::vector<row_data> scenario,
+ hotspot_partition_calculator &calculator)
+ {
+ calculator.data_aggregate(std::move(scenario));
+ calculator.data_analyse();
+ }
Review comment:
add a blank line between two fuctions.
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490689971
##########
File path: src/server/hotspot_partition_calculator.h
##########
@@ -18,12 +18,15 @@
#pragma once
#include "hotspot_partition_stat.h"
+#include <dsn/utility/flags.h>
Review comment:
move `#include <dsn/utility/flags.h>` to behind of `#include <gtest/gtest_prod.h>`
Ref: https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490742656
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +114,30 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
Review comment:
There are too many same code between 127-132 and 135-140
----------------------------------------------------------------
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] acelyc111 commented on a change in pull request #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490767238
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -42,6 +43,24 @@ DSN_DEFINE_int64("pegasus.collector",
"eliminate outdated historical "
"data");
+DSN_DEFINE_bool("pegasus.collector",
+ enable_hotkey_auto_detect,
Review comment:
```suggestion
enable_hotkey_detect,
```
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490690290
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +107,29 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int HOT_SCENARIO_0_READ_HOT_PARTITION = 7;
Review comment:
what does `HOT_SCENARIO_0` mean? could you remove this prefix?
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490743405
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +114,30 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
+ aggregate_analyse_data(test_rows, calculator);
+ }
+ std::vector<std::array<int, 2>> expect_result = {
+ {0, 100}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {100, 0}};
Review comment:
Does the 100 has relative with the default value of ` FLAGS_occurrence_threshold`? If so, you should change it to ` FLAGS_occurrence_threshold`
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490716519
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -58,6 +63,16 @@ class hotspot_partition_test : public pegasus_server_test_base
std::vector<std::vector<double>> result = get_calculator_result(calculator._hot_points);
ASSERT_EQ(result, expect_result);
}
+ void aggregate_analyse_data(std::vector<row_data> scenario,
Review comment:
In line 69
```cpp
calculator.data_aggregate(std::move(scenario));
```
It can't be a ref
----------------------------------------------------------------
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] acelyc111 commented on a change in pull request #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490769141
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -42,6 +43,24 @@ DSN_DEFINE_int64("pegasus.collector",
"eliminate outdated historical "
"data");
+DSN_DEFINE_bool("pegasus.collector",
+ enable_hotkey_auto_detect,
+ false,
+ "auto detect hot key in the hot paritition");
+
+DSN_DEFINE_int32("pegasus.collector",
+ hot_partition_threshold,
+ 3,
+ "threshold of hotspot partition value,if hot_partition_counter >= "
+ "FLAGS_hotpartition_threshold, This partition is a hot partition");
+
+DSN_DEFINE_int32("pegasus.collector",
+ occurrence_threshold,
+ 100,
+ "hot paritiotion occurrence times'threshold to send rpc to detect hotkey,if one "
Review comment:
```suggestion
"hot paritiotion occurrence times' threshold to send rpc to detect hotkey, if one "
```
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -42,6 +43,24 @@ DSN_DEFINE_int64("pegasus.collector",
"eliminate outdated historical "
"data");
+DSN_DEFINE_bool("pegasus.collector",
+ enable_hotkey_auto_detect,
+ false,
+ "auto detect hot key in the hot paritition");
+
+DSN_DEFINE_int32("pegasus.collector",
+ hot_partition_threshold,
+ 3,
+ "threshold of hotspot partition value,if hot_partition_counter >= "
Review comment:
User don't know what `hot_partition_counter ` is.
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -42,6 +43,24 @@ DSN_DEFINE_int64("pegasus.collector",
"eliminate outdated historical "
"data");
+DSN_DEFINE_bool("pegasus.collector",
+ enable_hotkey_auto_detect,
+ false,
+ "auto detect hot key in the hot paritition");
+
+DSN_DEFINE_int32("pegasus.collector",
+ hot_partition_threshold,
+ 3,
+ "threshold of hotspot partition value,if hot_partition_counter >= "
+ "FLAGS_hotpartition_threshold, This partition is a hot partition");
+
+DSN_DEFINE_int32("pegasus.collector",
+ occurrence_threshold,
+ 100,
+ "hot paritiotion occurrence times'threshold to send rpc to detect hotkey,if one "
+ "partition's _over_threshold_times_read/write >= FLAGS_occurrence_threshold "
Review comment:
These comments will be exposed to common users, so these internal variables should not appear here.
##########
File path: src/server/hotspot_partition_calculator.cpp
##########
@@ -42,6 +43,24 @@ DSN_DEFINE_int64("pegasus.collector",
"eliminate outdated historical "
"data");
+DSN_DEFINE_bool("pegasus.collector",
+ enable_hotkey_auto_detect,
+ false,
+ "auto detect hot key in the hot paritition");
+
+DSN_DEFINE_int32("pegasus.collector",
+ hot_partition_threshold,
+ 3,
+ "threshold of hotspot partition value,if hot_partition_counter >= "
Review comment:
```suggestion
"threshold of hotspot partition value, if hot_partition_counter >= "
```
##########
File path: src/server/test/config.ini
##########
@@ -509,3 +509,6 @@ onebox2 = 2
[pegasus.clusters]
onebox = 0.0.0.0:34701
onebox2 = 0.0.0.0:35701
+
+[pegasus.collector]
+enable_hotkey_auto_detect = true
Review comment:
```suggestion
enable_hotkey_detect = true
```
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r492445351
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
It is a different test case, expext result changed
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +121,37 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int READ_HOT_PARTITION = 7;
+ const int WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[WRITE_HOT_PARTITION].put_qps = 5000.0;
+ int hotpartition_count = FLAGS_occurrence_threshold;
+ std::vector<std::array<int, 2>> expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
+ aggregate_analyse_data(calculator, test_rows, expect_result, FLAGS_occurrence_threshold);
+ const int back_to_normal = 30;
+ hotpartition_count = FLAGS_occurrence_threshold - back_to_normal;
+ expect_result = {{0, hotpartition_count},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {0, 0},
+ {hotpartition_count, 0}};
Review comment:
It is a different test case, expect result changed
----------------------------------------------------------------
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 #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490740209
##########
File path: src/server/hotspot_partition_calculator.h
##########
@@ -17,13 +17,17 @@
#pragma once
-#include "hotspot_partition_stat.h"
#include <gtest/gtest_prod.h>
+
#include <dsn/perf_counter/perf_counter.h>
+#include <dsn/utility/flags.h>
+#include "hotspot_partition_stat.h"
namespace pegasus {
namespace server {
+DSN_DECLARE_int32(occurrence_threshold);
Review comment:
why did you add `DSN_DECLARE_int32`? I think you can remove it. Add it in where you really 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] levy5307 commented on a change in pull request #604: feat(hotspot): calculator auto detect hotkey in the hot partition
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #604:
URL: https://github.com/apache/incubator-pegasus/pull/604#discussion_r490692037
##########
File path: src/server/test/hotspot_partition_test.cpp
##########
@@ -92,6 +107,29 @@ TEST_F(hotspot_partition_test, hotspot_partition_policy)
test_rows[HOT_SCENARIO_1_WRITE_HOT_PARTITION].put_qps = 5000.0;
expect_vector = {{0, 0, 0, 4, 0, 0, 0, 0}, {0, 0, 4, 0, 0, 0, 0, 0}};
test_policy_in_scenarios(test_rows, expect_vector, calculator);
+ clear_calculator_histories(calculator);
+}
+
+TEST_F(hotspot_partition_test, send_hotkey_detect_request)
+{
+ const int HOT_SCENARIO_0_READ_HOT_PARTITION = 7;
+ const int HOT_SCENARIO_0_WRITE_HOT_PARTITION = 0;
+ std::vector<row_data> test_rows = generate_row_data();
+ test_rows[HOT_SCENARIO_0_READ_HOT_PARTITION].get_qps = 5000.0;
+ test_rows[HOT_SCENARIO_0_WRITE_HOT_PARTITION].put_qps = 5000.0;
+ for (int i = 0; i < FLAGS_occurrence_threshold; i++) {
+ aggregate_analyse_data(test_rows, calculator);
+ }
+ std::vector<std::array<int, 2>> expect_result = {
+ {0, 100}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {100, 0}};
+ ASSERT_EQ(calculator._hotpartition_pool, expect_result);
+
+ test_rows = generate_row_data();
+ for (int i = 0; i < 50; i++) {
Review comment:
what's the meaning of number `50`?
----------------------------------------------------------------
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