Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat: add perf-counter for backup request #419

Merged
merged 7 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/dist/replication/lib/replica.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ replica::replica(
// init table level latency perf counters
init_table_level_latency_counters();

counter_str = fmt::format("table_level_backup_request_qps@{}", _app_info.app_name);
levy5307 marked this conversation as resolved.
Show resolved Hide resolved
_counter_table_level_backup_request_qps.init_app_counter(
"eon.replica", counter_str.c_str(), COUNTER_TYPE_RATE, counter_str.c_str());

if (need_restore) {
// add an extra env for restore
_extra_envs.insert(
Expand Down Expand Up @@ -166,6 +170,8 @@ void replica::on_client_read(dsn::message_ex *request)
response_client_read(request, ERR_INVALID_STATE);
return;
}
} else {
_counter_table_level_backup_request_qps->increment();
Copy link
Contributor

@foreverneverer foreverneverer Mar 18, 2020

Choose a reason for hiding this comment

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

don't (or can't) distinguish request types(get, multi_get..)?

Copy link
Contributor Author

@levy5307 levy5307 Mar 18, 2020

Choose a reason for hiding this comment

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

If we need to distinguish get/multi_get, we should add the perf-counter in pegasus, not rdsn. But in pegasus, we can't get whether the request is backup request or not.
There is still one way to implement it, just like the way table level latency use. I think it is too complicated.

Copy link
Contributor

@foreverneverer foreverneverer Mar 18, 2020

Choose a reason for hiding this comment

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

Well, if can implement, we should consider whether to distinguish request types, and I think it is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why it is necessary? I think what we need is the total qps of backup request(which means the added server load), but the request type is not very important.

}

uint64_t start_time_ns = dsn_now_ns();
Expand Down
2 changes: 2 additions & 0 deletions src/dist/replication/lib/replica.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ class replica : public serverlet<replica>, public ref_counter, public replica_ba
friend class replica_duplicator_manager;
friend class load_mutation;
friend class replica_split_test;
friend class replica_test;

// replica configuration, updated by update_local_configuration ONLY
replica_configuration _config;
Expand Down Expand Up @@ -511,6 +512,7 @@ class replica : public serverlet<replica>, public ref_counter, public replica_ba
perf_counter_wrapper _counter_recent_write_throttling_delay_count;
perf_counter_wrapper _counter_recent_write_throttling_reject_count;
std::vector<perf_counter *> _counters_table_level_latency;
perf_counter_wrapper _counter_table_level_backup_request_qps;

dsn::task_tracker _tracker;
// the thread access checker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,26 @@ class replica_test : public replica_test_base
public:
dsn::app_info _app_info;
dsn::gpid pid = gpid(2, 1);
mock_replica_ptr _mock_replica;

public:
void SetUp() override
{
stub->install_perf_counters();
mock_app_info();
stub->generate_replica(_app_info, pid, partition_status::PS_PRIMARY, 1);
_mock_replica = stub->generate_replica(_app_info, pid, partition_status::PS_PRIMARY, 1);
}

int get_write_size_exceed_threshold_count()
{
return stub->_counter_recent_write_size_exceed_threshold_count->get_value();
}

int get_table_level_backup_request_qps()
{
return _mock_replica->_counter_table_level_backup_request_qps->get_integer_value();
}

void mock_app_info()
{
_app_info.app_id = 2;
Expand Down Expand Up @@ -59,5 +65,22 @@ TEST_F(replica_test, write_size_limited)
ASSERT_EQ(get_write_size_exceed_threshold_count(), count);
}

TEST_F(replica_test, backup_request_qps)
{
// create backup request
auto backup_request = dsn::message_ex::create_request(task_code());
levy5307 marked this conversation as resolved.
Show resolved Hide resolved
auto cleanup = dsn::defer([=]() { delete backup_request; });
struct dsn::message_header header;
header.context.u.is_backup_request = true;
backup_request->header = &header;

_mock_replica->on_client_read(backup_request);

// We have to sleep >= 0.1s, or the value this perf-counter will be 0, according to the
// implementation of perf-counter which type is COUNTER_TYPE_RATE.
usleep(1e5);
ASSERT_GT(get_table_level_backup_request_qps(), 0);
}

} // namespace replication
} // namespace dsn