Skip to content

Experimental change to solve performance overshooting issue #1286

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: fb-mysql-8.0.28
Choose a base branch
from
Open

Experimental change to solve performance overshooting issue #1286

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions plugin/clone/src/clone_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ uint64_t Client_Stat::task_target(uint64_t target_speed, uint64_t current_speed,
not set yet, start by assuming all active thread. */
auto active_tasks =
(current_target == 0) ? num_tasks : (current_speed / current_target);

active_tasks = num_tasks;
Copy link
Contributor Author

@sunshine-Chun sunshine-Chun Mar 20, 2023

Choose a reason for hiding this comment

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

The issue here is the calculation of active tasks is not correct. Let's say, the current_target is 700MB/S, and the current_speed is 900MB/S. Then the active_tasks will be 1.

According to the calculation for task_target below, the task target will be 700MB/s.
auto task_target = target_speed / active_tasks;
The task_target will be used as the current_target of next call of this task_target() function. Hence, it will go over the above calculation again and the task target speed will not be updated at all.

Since the max speed capacity for each thread is around 450MB/s, it will never throttle the thread. (Because it's always smaller than the target task speed 700MB/s).

For the change I made, I assume all threads created are active.

Copy link
Contributor

@laurynas-biveinis laurynas-biveinis Mar 21, 2023

Choose a reason for hiding this comment

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

Why InnoDB clone does not exhibit the issue?


/* Keep the value within current boundary. */
if (active_tasks == 0) {
Expand Down