mgr/vol: reuse code to spawn threads#59847
Conversation
|
jenkins test make check |
|
This PR is under test in https://tracker.ceph.com/issues/68395. |
|
This PR is under test in https://tracker.ceph.com/issues/68419. |
|
|
||
| def spawn_new_thread(self, suffix): | ||
| t_name = f'{self.name_pfx}.{time.time()}.{suffix}' | ||
| log.info(f'spawning new thread with name {t_name}') |
There was a problem hiding this comment.
why are these in info log level? They should be at debug level IMO.
There was a problem hiding this comment.
This one is pretty useful bit of information, it should be an info level IMO.
There was a problem hiding this comment.
It just tells it's spawning a thread. INFO should be used when you dump something like counters, etc. which helps in understanding how those values change over time.
There was a problem hiding this comment.
The thread name can be pretty useful when going through logs.
joscollin
left a comment
There was a problem hiding this comment.
@rishabh-d-dave
What's the motivation behind adding the helpers ? (just to understand). They are used only once.
Not exactly, both helper call the third helper. The idea is to have same code launch threads for the sake of uniformity. Earlier threads names were different. |
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
5786656 to
419d1f4
Compare
|
Rebased to resolve the conflict. |
rishabh-d-dave
left a comment
There was a problem hiding this comment.
QA run was successful - https://tracker.ceph.com/projects/cephfs/wiki/Main#wip-rishabh-testing-20241007102024
Signed-off-by: Rishabh Dave <ridave@redhat.com>
Also add log messages for in these helper methods to allow tracking when and why more threads were spawned. Signed-off-by: Rishabh Dave <ridave@redhat.com>
419d1f4 to
a982495
Compare
vshankar
left a comment
There was a problem hiding this comment.
If info->debug was the only change and QA run was done, please go ahead and merge.
Same block of code is written twice to spawn threads in async_job.py.
This has already created an inconsistency in naming of threads. Have
a single method for this and use it instead.
This allows not allows reusing code but also reduces such inconsistency.
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
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windowsjenkins test rook e2e