Skip to content

crimson: errorator parallel_for_each#41848

Merged
tchaikov merged 2 commits intoceph:masterfrom
xxhdx1985126:wip-errorator-parallel_for_each
Jun 17, 2021
Merged

crimson: errorator parallel_for_each#41848
tchaikov merged 2 commits intoceph:masterfrom
xxhdx1985126:wip-errorator-parallel_for_each

Conversation

@xxhdx1985126
Copy link
Copy Markdown
Member

@xxhdx1985126 xxhdx1985126 commented Jun 15, 2021

Signed-off-by: Xuehan Xu xxhdx1985126@gmail.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
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 api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@xxhdx1985126 xxhdx1985126 requested a review from a team as a code owner June 15, 2021 04:28
@xxhdx1985126 xxhdx1985126 mentioned this pull request Jun 15, 2021
Merged
3 tasks
tchaikov
tchaikov reviewed Jun 15, 2021
View reviewed changes
Comment thread src/test/crimson/test_errorator.cc Outdated
tchaikov
tchaikov reviewed Jun 15, 2021
View reviewed changes
Comment thread src/test/crimson/test_errorator.cc Outdated
std::vector<int> loops = {1, 2, 3};
return ertr::parallel_for_each(loops, [](int i) {
using namespace std::chrono_literals;
return seastar::sleep(1s).then([] {
Copy link
Copy Markdown
Contributor

@tchaikov tchaikov Jun 15, 2021

Choose a reason for hiding this comment

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

i'd avoid sleep in test unless necessary. it just slows down the unit test.

Copy link
Copy Markdown
Contributor

@rzarzynski rzarzynski Jun 16, 2021

Choose a reason for hiding this comment

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

Yeah, 1s is probably too much. However, the time domain is pretty good, intuitive way of illustrating the parallel part of the naming.

@sebastian-philipp
Copy link
Copy Markdown
Contributor

sebastian-philipp commented Jun 15, 2021

jenkins test make check

rzarzynski
rzarzynski reviewed Jun 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

Overall looks good!

Comment thread src/crimson/common/errorator.h Outdated
Comment thread src/crimson/common/errorator.h
Comment thread src/test/crimson/test_errorator.cc Outdated
std::vector<int> loops = {1, 2, 3};
return ertr::parallel_for_each(loops, [](int i) {
using namespace std::chrono_literals;
return seastar::sleep(1s).then([] {
Copy link
Copy Markdown
Contributor

@rzarzynski rzarzynski Jun 16, 2021

Choose a reason for hiding this comment

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

Yeah, 1s is probably too much. However, the time domain is pretty good, intuitive way of illustrating the parallel part of the naming.

@xxhdx1985126 xxhdx1985126 force-pushed the wip-errorator-parallel_for_each branch from 5611696 to 1a80130 Compare June 16, 2021 02:09
@xxhdx1985126
Copy link
Copy Markdown
Member Author

xxhdx1985126 commented Jun 16, 2021

@tchaikov @rzarzynski done:-)

tchaikov
tchaikov reviewed Jun 16, 2021
View reviewed changes
Comment thread src/crimson/common/errorator.h Outdated
tchaikov
tchaikov reviewed Jun 16, 2021
View reviewed changes
Comment thread src/crimson/common/errorator.h Outdated
@xxhdx1985126 xxhdx1985126 force-pushed the wip-errorator-parallel_for_each branch 2 times, most recently from 432a677 to 9e14a09 Compare June 16, 2021 02:48
@xxhdx1985126
Copy link
Copy Markdown
Member Author

xxhdx1985126 commented Jun 16, 2021

done:-)

@xxhdx1985126
crimson: add parallel_for_each to errorator
0908b11 
Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
@xxhdx1985126
test/crimson: add test case for errorator parallel_for_each
b1acbb0 
Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
@xxhdx1985126 xxhdx1985126 force-pushed the wip-errorator-parallel_for_each branch from 9e14a09 to b1acbb0 Compare June 16, 2021 09:04
@xxhdx1985126
Copy link
Copy Markdown
Member Author

xxhdx1985126 commented Jun 16, 2021

@tchaikov STL for_each() style done:-)

@tchaikov
Copy link
Copy Markdown
Contributor

tchaikov commented Jun 16, 2021 

[ RUN      ] OSDMapTest.BUG_42485
pgid [26,12,54,46] up [26,12,54,46]
from 26 to 30
pgid 3.2 up [16,25,53,31]
from 16 to 40
from 31 to 0
pure virtual method called
terminate called without an active exception
*** Caught signal (Aborted) **
 in thread 7f57dea8e700 thread_name:clean_upmap_tp
 ceph version Development (no_version) quincy (dev)
 1: /home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_osdmap(+0x2c602b) [0x563af8f1b02b]
 2: /lib/x86_64-linux-gnu/libpthread.so.0(+0x153c0) [0x7f57e39583c0]
 3: gsignal()
 4: abort()
 5: /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0x9e951) [0x7f57e10d9951]
 6: /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xaa47c) [0x7f57e10e547c]
 7: /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xaa4e7) [0x7f57e10e54e7]
 8: /usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xab245) [0x7f57e10e6245]
 9: (ThreadPool::WorkQueue<ParallelPGMapper::Item>::_void_dequeue()+0x27) [0x563af8eb1255]
 10: (ThreadPool::worker(ThreadPool::WorkThread*)+0x50b) [0x7f57e289c29f]
 11: (ThreadPool::WorkThread::entry()+0x36) [0x7f57e28a0654]
 12: (Thread::entry_wrapper()+0x87) [0x7f57e2877d3d]
 13: (Thread::_entry_func(void*)+0x1c) [0x7f57e2877cac]
 14: /lib/x86_64-linux-gnu/libpthread.so.0(+0x9609) [0x7f57e394c609]
 15: clone()
2021-06-16T09:14:50.665+0000 7f57dea8e700 -1 *** Caught signal (Aborted) **
 in thread 7f57dea8e700 thread_name:clean_upmap_tp

@tchaikov
Copy link
Copy Markdown
Contributor

tchaikov commented Jun 16, 2021

jenkins test make check

@tchaikov tchaikov merged commit f4eba94 into ceph:master Jun 17, 2021
@xxhdx1985126 xxhdx1985126 deleted the wip-errorator-parallel_for_each branch April 23, 2024 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tchaikov tchaikov tchaikov approved these changes

@rzarzynski rzarzynski rzarzynski approved these changes

Assignees

No one assigned

Labels

crimson tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xxhdx1985126 @sebastian-philipp @tchaikov @rzarzynski