Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(collector): sort out the structure of partition hotspot detection #597

Merged
merged 40 commits into from
Sep 10, 2020

Conversation

Smityz
Copy link
Contributor

@Smityz Smityz commented Sep 7, 2020

What problem does this PR solve?

This submission is mainly to merge the previous code, as part of the overall refactoring.

What is changed and how it works?

In the previous design, partition hotspot detection is designed as a 'strategic model'.
in this PR, I will deprecate the formal design, and combine hotspot_policy hotspot_algo_qps_variance andhotspot_calculator into hotspot_partition_calculator, and change some variable names by the way.
The change of data structure and function will be refactored in #592

Rename file:
src/server/table_hotspot_policy.h -> src/server/hotspot_partition_calculator.h
src/server/table_hotspot_policy.cpp -> src/server/hotspot_partition_calculator.cpp
src/server/test/hotspot_partition_test.cpp -> src/server/test/pegasus_tablehotspot_test.cpp 

Config changes

[pegasus.collector]
- hotspot_detect_algorithm

src/server/info_collector.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_policy.h Outdated Show resolved Hide resolved
src/server/hotspot_partition_policy.cpp Outdated Show resolved Hide resolved
src/server/info_collector.h Outdated Show resolved Hide resolved
src/server/info_collector.cpp Outdated Show resolved Hide resolved
src/server/info_collector.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.h Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.h Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.h Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.h Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
int sample_count = 0;
while (!temp_data.empty()) {
for (const auto &partition_data : temp_data.front()) {
if (partition_data.total_qps - 1.00 > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why need such prerequiste? What if a partition qps is 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to prevent too many 0 elements in the queue from interfering with the accuracy of the historical data, we choose to ignore some small values

Smityz and others added 2 commits September 9, 2020 20:45
Co-authored-by: Wu Tao <wutao1@xiaomi.com>
enum partition_qps_type
{
READ_HOTSPOT_DATA = 0,
WRITE_HOTSPOT_DATA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why READ_HOTSPOT_DATA init explictly, but WRITE_HOTSPOT_DATA no?

Copy link
Contributor

@levy5307 levy5307 Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in cpp, the enum is increased by one orderly. so we only need to init the first one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first default 0, so also can not be inited?

typedef std::vector<std::vector<std::unique_ptr<dsn::perf_counter_wrapper>>> hot_partition_counters;

// hotspot_partition_calculator is used to find the hot partition in a table.
class hotspot_partition_calculator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class hotspot_partition_calculator
class hotspot_partition_calculator : public replica_base

replica_base has a app_name() method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hotspot_partition_calculator stores the statistical data of the correspond table, _app_name is for that table, not itself.

@neverchanje neverchanje merged commit b057848 into apache:master Sep 10, 2020
@Smityz Smityz mentioned this pull request Sep 17, 2020
@Smityz Smityz deleted the del_old_alg branch October 20, 2020 09:24
@neverchanje neverchanje mentioned this pull request Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/config-change Added or modified configuration that should be noted on release note of new version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants