common/ceph_timer: set thread name #34936
Conversation
This appears to be a cut-n-paste error introduced with f06848c, with the intent being to link unittest_ceph_timer against GTest::GTest. GTest::GTest is already linked via the add_ceph_unittest() cmake rule used for both unittest_rabin_chunk and unittest_ceph_timer, so the explicit target_link_libraries() call can be dropped. Signed-off-by: David Disseldorp <ddiss@suse.de>
Attempt to set a ceph_timer thread name using the ceph_pthread_setname() compatibility macro. Having an explicit thread name set is useful for debugging purposes. Signed-off-by: David Disseldorp <ddiss@suse.de>
|
@adamemerson looks like you've done some changes in this area recently. Any chance you'd be able to take a look? |
| @@ -145,6 +146,7 @@ class timer { | |||
| public: | |||
| timer() : suspended(false) { | |||
| thread = std::thread(&timer::timer_thread, this); | |||
| ceph_pthread_setname(thread.native_handle(), "ceph_timer"); | |||
There was a problem hiding this comment.
You might want to use make_named_thread from common/Thread.h (but you don't have to, either way is fine.)
There was a problem hiding this comment.
Thanks a lot for the review :-)
I saw that there are a few other wrappers to handle naming, but figured this was less intrusive. Happy to change it if somebody feels strongly about it though.
| @@ -335,5 +335,4 @@ target_link_libraries(unittest_rabin_chunk global ceph-common) | |||
| add_ceph_unittest(unittest_rabin_chunk) | |||
|
|
|||
| add_executable(unittest_ceph_timer test_ceph_timer.cc) | |||
| target_link_libraries(unittest_rabin_chunk GTest::GTest) | |||
|
@ddiss i change the title of this PR to "common/ceph_timer: set thread name" as i think the change of thread name is far more important than the cmake cleanup. so when people searching for the relevant change, he/she is more likely to find the right change. |
|
the failures are either tracked or fixed by |
No description provided.