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

Experimental change to solve performance overshooting issue #1286

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

Conversation

sunshine-Chun
Copy link
Contributor

Summary:

This is a experimental change to solve the performance overshooting issue. The full change need to change the number of function parameters for several functions. Will apply the changes after we reach an agreement to make this change.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@@ -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

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

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants