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

util: add `affinity-cpus` starting argument #10773

Merged
merged 8 commits into from Jul 1, 2019

Conversation

Projects
None yet
4 participants
@lysu
Copy link
Member

commented Jun 11, 2019

What problem does this PR solve?

when TiDB deployed in NUMA machine, we often need startup multiple processes and self-binding to numa node's cpu, so we should do this.

  1. run lscpu to get info like this:
NUMA node0 CPU(s):     0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38
NUMA node1 CPU(s):     1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31,33,35,37,39
  1. run taskset to bind core and start tidb-server
nohup taskset -c 0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38 bin/tidb-server
nohup taskset -c 1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31,33,35,37,39 bin/tidb-server
  1. modify max-procs to meet bind count

the question's (2)(3) is not easy to use and often forgot to do (3)

What is changed and how it works?

make (2)(3) easier, add an optional argument when TiDB-server start.

just run this:

./bin/tidb-server -store tikv -path 127.0.0.1:2379 -affinity-cpus=2,3

to bind proc to cpu 2 and 3, it will also be base step to make tidb-ansible to make deploy easier after ansible detect (1)

Notice: this argument only works on linux.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

run

./bin/tidb-server -store tikv -path 127.0.0.1:2379 -affinity-cpus=2,3

take a look taskset to check bind works.

taskset -pc $(pgrep tidb-server)
pid 14881's current affinity list: 2,3

run sysbench point-get and see htop only those cores be 100%

Code changes

  • N/A

Side effects

  • N/A

Related changes

  • Need to cherry-pick to 3.0

This change is Reviewable

@codecov

This comment has been minimized.

Copy link

commented Jun 11, 2019

Codecov Report

Merging #10773 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #10773   +/-   ##
===========================================
  Coverage   81.0223%   81.0223%           
===========================================
  Files           418        418           
  Lines         89268      89268           
===========================================
  Hits          72327      72327           
  Misses        11717      11717           
  Partials       5224       5224

lysu added some commits Jun 11, 2019

fmt.Fprintf(os.Stderr, "set cpu affinity failure: %v", err)
exit()
}
runtime.GOMAXPROCS(len(cpu))

This comment has been minimized.

Copy link
@jackysp

jackysp Jun 12, 2019

Member

Will it conflict with max-procs?

This comment has been minimized.

Copy link
@lysu

lysu Jun 12, 2019

Author Member

good catch.. I have move this after setGlobalVars, so it will override max-procs..also remove reload max-procs function

Show resolved Hide resolved tidb-server/main.go Outdated
@jackysp
Copy link
Member

left a comment

LGTM

@lysu lysu requested a review from tiancaiamao Jun 13, 2019

@lysu

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

@tiancaiamao PTAL if free thx

@lysu

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

/run-all-tests tidb-test=pr/765

@XuHuaiyu

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

run sysbench point-get and see htop only one core be 100%

Since we've bind tidb-server to 2 cores, why there is only one core be 100%?

@@ -331,9 +360,6 @@ func reloadConfig(nc, c *config.Config) {
// like config.GetGlobalConfig().OOMAction.
// These config items will become available naturally after the global config pointer
// is updated in function ReloadGlobalConfig.
if nc.Performance.MaxProcs != c.Performance.MaxProcs {

This comment has been minimized.

Copy link
@XuHuaiyu

XuHuaiyu Jun 19, 2019

Contributor

Can we check whether we've set CPUAffinity, if so, we do not reset max-procs?

This comment has been minimized.

Copy link
@tiancaiamao

tiancaiamao Jun 24, 2019

Contributor

After this change, config MaxProcs in the config has no effect? @lysu

This comment has been minimized.

Copy link
@lysu

lysu Jun 26, 2019

Author Member

@XuHuaiyu you mean using taskset + -affinity-cpus at the same time? taskset is too hard to use, normally only manually set in some test env, after this pr will advise people using -affinity-cpus by issue a follow up tidb-ansible PR, and reset max-procs during starting in light.

This comment has been minimized.

Copy link
@lysu

lysu Jun 26, 2019

Author Member

@tiancaiamao this PR just want to reduce working to "set MaxProcs" when binding CPU cores, if a user doesn't bind cores can keep using MaxProcs as its like, but when a user bind cores, in 99% they want to use binded core.

@lysu

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2019

run sysbench point-get and see htop only one core be 100%

Since we've bind tidb-server to 2 cores, why there is only one core be 100%?

description fixed

@tiancaiamao

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2019

LGTM @XuHuaiyu

@lysu lysu added the status/LGT1 label Jun 26, 2019

@lysu

This comment has been minimized.

Copy link
Member Author

commented Jun 28, 2019

ping @XuHuaiyu PTAL if free thx~

@XuHuaiyu
Copy link
Contributor

left a comment

LGTM

@XuHuaiyu XuHuaiyu added status/LGT2 and removed status/LGT1 labels Jul 1, 2019

@XuHuaiyu

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

/run-all-tests

@tiancaiamao tiancaiamao merged commit 5409406 into pingcap:master Jul 1, 2019

14 checks passed

ci/circleci Your tests passed on CircleCI!
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/build_check_race Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev_2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/common-test job succeeded
Details
idc-jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/mybatis-test job succeeded
Details
idc-jenkins-ci-tidb/sqllogic-test-1 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/sqllogic-test-2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.