Revise default background threads size

[Lloyd-Pottiger]

Signed-off-by: Lloyd-Pottiger yan1579196623@gmail.com

What problem does this PR solve?

Issue Number: close #4702

Problem Summary:

What is changed and how it works?

1. set the size of background & blockable background thread pool and GRPCCompletionQueuePool to a quarter of server_info.cpu_info.logical_cores.
2. Add two functions to separate initialization and get of BackgroundProcessingPool.

BackgroundProcessingPool & Context::initializeBackgroundPool(UInt16 pool_size);
BackgroundProcessingPool & Context::initializeBlockableBackgroundPool(UInt16 pool_size);

3. add initialization of background & blockable background thread pool in TiFlashTestEnv.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• JaySon-Huang
• JinheLin

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[JaySon-Huang]

better to use the function getNumberOfPhysicalCPUCores

[Lloyd-Pottiger]

better to use the function getNumberOfPhysicalCPUCores

Maybe it is better to use logical cpu numbers rather than physical cpu number? Usually one physical cpu have more than one cpu core, and there are hyper-threading in most cpus.

[Lloyd-Pottiger]

/run-all-tests

[sre-bot]

Coverage for changed files

Filename                                                                               Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Files which contain no functions:
/home/jenkins/agent/workspace/tiflash-build-common/tiflash/dbms/src/Core/Defines.h           0                 0         -           0                 0         -           0                 0         -           0                 0         -
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                        0                 0         -           0                 0         -           0                 0         -           0                 0         -

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17132      9492             44.59%    192860  96270        50.08%

full coverage report (for internal network access only)

[Lloyd-Pottiger]

/cc @JinheLin @JaySon-Huang

[JinheLin]

better to use the function getNumberOfPhysicalCPUCores

Maybe it is better to use logical cpu numbers rather than physical cpu number? Usually one physical cpu have more than one cpu core, and there are hyper-threading in most cpus.

Also, I think physical cpu is not a good idea in cloud environment.

[JaySon-Huang]

Also, I think physical cpu is not a good idea in cloud environment.

We planned to make getNumberOfPhysicalCPUCores respect the k8s/docker CPU limit, which is more friendly for cloud env https://github.com/pingcap/tiflash/pull/3852/files

But that PR is blocked since now @jiaqizho has no spare time to get that addressed and merged. Maybe @Lloyd-Pottiger take over that PR and continue to get it merged?

[JinheLin]

Also, I think physical cpu is not a good idea in cloud environment.

We planned to make getNumberOfPhysicalCPUCores respect the k8s/docker CPU limit, which is more friendly for cloud env https://github.com/pingcap/tiflash/pull/3852/files

But that PR is blocked since now @jiaqizho has no spare time to get that addressed and merged. Maybe @Lloyd-Pottiger take over that PR and continue to get it merged?

The CPU limit of cgroup is CPU time and it is related to logical CPU. Physical CPU is not suitable too.

I think @Lloyd-Pottiger can look into this PR and try to get it done.

[Lloyd-Pottiger]

Also, I think physical cpu is not a good idea in cloud environment.

We planned to make getNumberOfPhysicalCPUCores respect the k8s/docker CPU limit, which is more friendly for cloud env https://github.com/pingcap/tiflash/pull/3852/files

But that PR is blocked since now @jiaqizho has no spare time to get that addressed and merged. Maybe @Lloyd-Pottiger take over that PR and continue to get it merged?

@JaySon-Huang @JinheLin OK, I will

[jiaqizho]

Also, I think physical cpu is not a good idea in cloud environment.

We planned to make getNumberOfPhysicalCPUCores respect the k8s/docker CPU limit, which is more friendly for cloud env https://github.com/pingcap/tiflash/pull/3852/files
But that PR is blocked since now @jiaqizho has no spare time to get that addressed and merged. Maybe @Lloyd-Pottiger take over that PR and continue to get it merged?

@JaySon-Huang @JinheLin OK, I will

https://github.com/pingcap/tiflash/pull/3852/files a tip: This PR tested on cgroup. but have not been tested in k8s/docker.

[Lloyd-Pottiger]

Also, I think physical cpu is not a good idea in cloud environment.

We planned to make getNumberOfPhysicalCPUCores respect the k8s/docker CPU limit, which is more friendly for cloud env https://github.com/pingcap/tiflash/pull/3852/files
But that PR is blocked since now @jiaqizho has no spare time to get that addressed and merged. Maybe @Lloyd-Pottiger take over that PR and continue to get it merged?

@JaySon-Huang @JinheLin OK, I will

https://github.com/pingcap/tiflash/pull/3852/files a tip: This PR tested on cgroup. but have not been tested in k8s/docker.

thx, I will try

[Lloyd-Pottiger]

/run-all-tests

[sre-bot]

Coverage for changed files

Filename                                Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Interpreters/Context.cpp                    518               329    36.49%         168                90    46.43%        1138               705    38.05%         276               209    24.28%
Interpreters/Context.h                       13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Storages/BackgroundProcessingPool.h           2                 0   100.00%           2                 0   100.00%           2                 0   100.00%           0                 0         -

Files which contain no functions:
Core/Defines.h                                0                 0         -           0                 0         -           0                 0         -           0                 0         -
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       533               334    37.34%         183                95    48.09%        1153               710    38.42%         276               209    24.28%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18330      9659             47.30%    205690  96734        52.97%

full coverage report (for internal network access only)

[JaySon-Huang]

/run-all-tests

[Lloyd-Pottiger]

/run-all-tests

[Lloyd-Pottiger]

/run-all-tests

[Lloyd-Pottiger]

/run-all-tests

[sre-bot]

Coverage for changed files

Filename                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Interpreters/Context.cpp                      514               323    37.16%         168                88    47.62%        1134               693    38.89%         272               207    23.90%
Interpreters/Context.h                         13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Interpreters/Settings.h                         1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Storages/BackgroundProcessingPool.cpp          91                26    71.43%          10                 1    90.00%         182                27    85.16%          56                18    67.86%
Storages/BackgroundProcessingPool.h             2                 0   100.00%           2                 0   100.00%           2                 0   100.00%           0                 0         -
TestUtils/TiFlashTestEnv.cpp                   22                 3    86.36%           4                 0   100.00%          67                 9    86.57%          12                 5    58.33%

Files which contain no functions:
Core/Defines.h                                  0                 0         -           0                 0         -           0                 0         -           0                 0         -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                         643               357    44.48%         198                94    52.53%        1399               734    47.53%         340               230    32.35%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18330      9655             47.33%    205690  96694        52.99%

full coverage report (for internal network access only)

[JaySon-Huang]

LGTM

And please update the description of this PR

[Lloyd-Pottiger]

/merge

[ti-chi-bot]

@Lloyd-Pottiger: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[ti-chi-bot]

This pull request has been accepted and is ready to merge.

Commit hash: e227778

[ti-chi-bot]

@Lloyd-Pottiger: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[sre-bot]

Coverage for changed files

Filename                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Interpreters/Context.cpp                      533               330    38.09%         169                88    47.93%        1155               700    39.39%         290               216    25.52%
Interpreters/Context.h                         13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Interpreters/Settings.h                         1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Storages/BackgroundProcessingPool.cpp          91                25    72.53%          10                 1    90.00%         182                26    85.71%          56                17    69.64%
Storages/BackgroundProcessingPool.h             2                 0   100.00%           2                 0   100.00%           2                 0   100.00%           0                 0         -
TestUtils/TiFlashTestEnv.cpp                   22                 3    86.36%           4                 0   100.00%          67                 9    86.57%          12                 5    58.33%

Files which contain no functions:
Core/Defines.h                                  0                 0         -           0                 0         -           0                 0         -           0                 0         -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                         662               363    45.17%         199                94    52.76%        1420               740    47.89%         358               238    33.52%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18327      9653             47.33%    205795  96644        53.04%

full coverage report (for internal network access only)