Skip to content

Conversation

@rosinL
Copy link
Member

@rosinL rosinL commented Jan 26, 2024

When running crimson unittest, the seastar framework always
use and only use cpu0, and with many parallel crimson unittest
jobs, all the jobs are running on cpu0, the other cpu cores
can't used, make the make check run very slow, even timeout
happens. Use set_property RESOURCE_GROUPS to specify cpu resources
to crimson unittest, and accelerate make check running.

Fixes: https://tracker.ceph.com/issues/64117

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@rosinL
Copy link
Member Author

rosinL commented Jan 26, 2024

jenkins test make check arm64

@rosinL
Copy link
Member Author

rosinL commented Jan 26, 2024

For x86 make check, the longest crimson unittest job unittest-transaction-manager, costtime reduced from 1940.81 sec to 983.14 sec. As long tail job run-rbd-unit-tests-61.sh, the total test time reduced not so much. As make check arm is still running ,there is no Arm test data for now, but it does same effect on Arm.
Before(x86)

287/290 Test #257: unittest-omap-manager .....................   Passed  1642.30 sec
288/290 Test  #32: run-rbd-unit-tests-61.sh ..................   Passed  1728.38 sec
289/290 Test #264: unittest-fltree-onode-manager .............   Passed  1688.28 sec
290/290 Test #251: unittest-transaction-manager ..............   Passed  1940.81 sec

100% tests passed, 0 tests failed out of 290

Total Test time (real) = 1980.95 sec

After(x86)

287/290 Test #251: unittest-transaction-manager ..............   Passed  983.14 sec
288/290 Test  #31: run-rbd-unit-tests-1.sh ...................   Passed  1529.75 sec
289/290 Test #147: check-generated.sh ........................   Passed  1686.12 sec
290/290 Test  #32: run-rbd-unit-tests-61.sh ..................   Passed  1689.13 sec

100% tests passed, 0 tests failed out of 290

Total Test time (real) = 1689.17 sec

@rosinL
Copy link
Member Author

rosinL commented Jan 26, 2024

@ljflores @athanatos @Matan-B @rzarzynski @cyx1231st Please help to review, thanks.

@athanatos
Copy link
Contributor

@rosinL So different crimson unittest processes are being bound to core 0? I'd have expected --smp 0 to limit any particular process to a single reactor, but not to limit it to any specific core.

@rosinL
Copy link
Member Author

rosinL commented Jan 27, 2024

jenkins test make check arm64

@rosinL
Copy link
Member Author

rosinL commented Jan 27, 2024

@athanatos As I test, with --smp 0 , the test will report runtime error: division by zero. Are there some mistakes, please let me know.

@rosinL
Copy link
Member Author

rosinL commented Jan 27, 2024

I run 2 crimson unittest jobs parallelly
Without --cpuset, all the jobs run on cpu0

root@ubuntu:/home# ps -ef |grep unittest
root     3373367 1082838 59 01:27 pts/1    00:00:02 ./build/bin/unittest-fltree-onode-manager --memory 256M --smp 1
root     3373370  251881 56 01:27 pts/2    00:00:02 ./bin/unittest-transaction-manager --memory 256M --smp 1
root     3373374  715758  0 01:27 pts/4    00:00:00 grep --color=auto unittest
root@ubuntu:/home# taskset -acp 3373367
pid 3373367's current affinity list: 0-47
pid 3373368's current affinity list: 0
pid 3373369's current affinity list: 0
root@ubuntu:/home# taskset -acp 3373370
pid 3373370's current affinity list: 0-47
pid 3373371's current affinity list: 0
pid 3373372's current affinity list: 0

With --cpuset to separate jobs to different cores

root@ubuntu:/home# ps -ef |grep unittest
root     3373379 1082838 93 01:28 pts/1    00:00:10 ./build/bin/unittest-fltree-onode-manager --memory 256M --smp 1 --cpuset 1
root     3373382  251881 96 01:28 pts/2    00:00:06 ./bin/unittest-transaction-manager --memory 256M --smp 1 --cpuset 0
root     3373386  715758  0 01:28 pts/4    00:00:00 grep --color=auto unittest
root@ubuntu:/home# taskset -acp 3373379
pid 3373379's current affinity list: 0-47
pid 3373380's current affinity list: 1
pid 3373381's current affinity list: 1
root@ubuntu:/home# taskset -acp 3373382
pid 3373382's current affinity list: 0-47
pid 3373383's current affinity list: 0
pid 3373384's current affinity list: 0

@rosinL
Copy link
Member Author

rosinL commented Jan 27, 2024

@athanatos If there are other better solutions, please let me know, thanks.

@rosinL
Copy link
Member Author

rosinL commented Jan 27, 2024

jenkins test make check arm64

@rosinL rosinL force-pushed the wip-fix-64117 branch 3 times, most recently from e6ef226 to f1fa30c Compare January 27, 2024 09:18
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

@rosinL Hi Rixin, thank you for your efforts to improve the efficiency of crimson tests. i'd suggest taking a look at https://cmake.org/cmake/help/latest/manual/ctest.1.html#ctest-resource-allocation, and consider CPU shards as a resource. actually, the same applies to the memory.

@rosinL
Copy link
Member Author

rosinL commented Jan 29, 2024

@tchaikov I will do some research and have a try

@rosinL
Copy link
Member Author

rosinL commented Jan 29, 2024

jenkins test make check arm64

Copy link
Member

@cyx1231st cyx1231st left a comment

Choose a reason for hiding this comment

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

The --cpuset configuration policy looks good to me to distribute the crimson tests to all the available cores in a round-robin way.

@rosinL
Copy link
Member Author

rosinL commented Jan 30, 2024

@tchaikov According my understand, we need taskset or numactl or seastar arg --cpuset to distribute crimson tests to different cores. ctest resource allocation can consider CPU shards as a resource, but after CPU shard allocated, ctest cann't bind crimson test to the CPU shard automatically, still need taskset or numactl or seastar arg --cpuset to bind cores manually, It seems more complicated, Am I right?

@rosinL
Copy link
Member Author

rosinL commented Jan 30, 2024

jenkins test api

@rosinL
Copy link
Member Author

rosinL commented Feb 1, 2024

As make check(arm64) running pretty slow, Can we merge this or move a step forward if needed? @ljflores

@tchaikov
Copy link
Contributor

tchaikov commented Feb 1, 2024

@tchaikov According my understand, we need taskset or numactl or seastar arg --cpuset to distribute crimson tests to different cores. ctest resource allocation can consider CPU shards as a resource, but after CPU shard allocated, ctest cann't bind crimson test to the CPU shard automatically, still need taskset or numactl or seastar arg --cpuset to bind cores manually, It seems more complicated, Am I right?

ctest populates the allocated resource using environment variables like CTEST_RESOURCE_GROUP_<num>_<resource-type>. i'd suggest rework the crimson tests to respect these variables, and set the seastar application config accordingly. it's not that difficult i guess. please let me know if this does not work out for you.

@rosinL
Copy link
Member Author

rosinL commented Feb 4, 2024

ctest populates the allocated resource using environment variables like CTEST_RESOURCE_GROUP__. i'd suggest rework the crimson tests to respect these variables, and set the seastar application config accordingly

@tchaikov Do you mean like this:

int main(int argc, char** argv) {
    seastar::app_template app;
    // get cpuset from CTEST_RESOURCE_GROUP_<num>_<resource-type>
    // get new_argv with argc + 2  elements
    // copy argv to new_argv
    // new_argv[argc]="--cpuset"
    // new_argv[argc+1]="startcpuid-endcpuid"
    app.run(argc+2, new_argv, [] {
        return crimson_test_func();
    });
}

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

left couple comments. also, please explain the motive in the related commit message. and add paste the test results before and after the change to show the improvement contributed by this change.

@rosinL rosinL force-pushed the wip-fix-64117 branch 2 times, most recently from 60331be to e786793 Compare February 19, 2024 07:50
@rosinL
Copy link
Member Author

rosinL commented Feb 19, 2024

jenkins test make check

@rosinL
Copy link
Member Author

rosinL commented Feb 19, 2024

jenkins retest this please

@rosinL
Copy link
Member Author

rosinL commented Feb 19, 2024

For x86 irvingi07 with 24 cores, the Total Test time keep the same as the long tail job check-generated.sh, the longest crimson unittest unittest-transaction-manager job's time decreased from 2522.75 sec to 1375.03 sec, nearly 2X performance improvement.
Before

285/290 Test #257: unittest-omap-manager .....................   Passed  2010.54 sec
286/290 Test  #32: run-rbd-unit-tests-61.sh ..................   Passed  2425.26 sec
287/290 Test #148: readable.sh ...............................   Passed  2458.99 sec
288/290 Test #264: unittest-fltree-onode-manager .............   Passed  2093.77 sec
289/290 Test #251: unittest-transaction-manager ..............   Passed  2522.75 sec
290/290 Test #147: check-generated.sh ........................   Passed  2846.91 sec

100% tests passed, 0 tests failed out of 290

Total Test time (real) = 2919.83 sec

After

285/290 Test  #52: unittest_bufferlist .......................   Passed  1238.60 sec
286/290 Test #251: unittest-transaction-manager ..............   Passed  1375.03 sec
287/290 Test  #31: run-rbd-unit-tests-1.sh ...................   Passed  2063.50 sec
288/290 Test  #32: run-rbd-unit-tests-61.sh ..................   Passed  2481.31 sec
289/290 Test #148: readable.sh ...............................   Passed  2446.87 sec
290/290 Test #147: check-generated.sh ........................   Passed  2838.97 sec

100% tests passed, 0 tests failed out of 290

Total Test time (real) = 2914.18 sec

There are some older Arm server running pretty slow, the make
check jobs like `check-generated.sh` are killed as the job timeout.
Make CEPH_TEST_TIMEOUT more longer.

Signed-off-by: luo rixin <luorixin@huawei.com>
@rosinL rosinL requested a review from tchaikov February 21, 2024 01:05
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

@rosinL Rixin, we are close. lgtm modulo couple nits.

rosinL and others added 3 commits February 21, 2024 10:36
When running crimson unittest, the seastar framework always
use and only use cpu0, and with many parallel crimson unittest
jobs, all the jobs are running on cpu0, the other cpu cores
can't used, make the make check run very slow, even timeout
happens. Use set_property RESOURCE_GROUPS to specify cpu resources
to crimson unittest, and accelerate make check running.

Fixes: https://tracker.ceph.com/issues/64117

Co-authored-by: Kefu Chai <tchaikov@gmail.com>
Signed-off-by: luo rixin <luorixin@huawei.com>
Co-authored-by: Kefu Chai <tchaikov@gmail.com>
Signed-off-by: luo rixin <luorixin@huawei.com>
Co-authored-by: Kefu Chai <tchaikov@gmail.com>
Signed-off-by: luo rixin <luorixin@huawei.com>
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

lgtm

@tchaikov
Copy link
Contributor

@athanatos hey Sam, do you want to take a final look?

@rosinL
Copy link
Member Author

rosinL commented Feb 22, 2024

Can we merge this directly? As this pr only affect unittest and we have no qa suite to test this.

@tchaikov tchaikov merged commit ac1d58a into ceph:main Feb 22, 2024
@tchaikov
Copy link
Contributor

yes, agreed. merged.

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.

7 participants