Bug #19986
closed
PR #14886 creates a SafeTimer thread per PG
Added by Mark Nelson almost 7 years ago.
Updated almost 7 years ago.
Description
This appears to be causing huge numbers of idle safe_timer threads to be created on the OSD. While this is likely not ideal for a variety of reasons, the immediate fallout is that it's making wallclock profiling ridiculously slow and also making it really irritating to gather backtraces from gdb.
https://github.com/ceph/ceph/pull/14886
Hi Mark, Thanks for the report. The intention was that this should increase performance as it frees up the work queue to make progress by sleeping asynchronously, rather than synchronously as it was doing prior to 14886. I'll see what can be done about the issues you are seeing. Do you see the same sort of issue (to a lesser extent) when snap trimming is in progress as the sleep in that case is implemented in a very similar fashion?
We just don't need a Timer object/thread for every PG, Brad. The snap trim timer does the same thing with sleeps but uses a single Timer per OSD. :)
I guess in particular, the issue is that there is a cost per thread (not a huge one, but it exists). Mark is seeing it pretty aggressively because it dramatically increases the number of threads and he's doing hacks with libunwind on every thread and things, but it also manifests as higher VSS and RSS that we don't want. Should be pretty simple to have a shared Timer object instead of one per PG.
(I'm not sure if there's any particular reason we have more than one Timer to begin with; I guess just because they interact with different threads and locks and we don't want them to contend?)
Yep, seemed like the simplest implementation but I guess I should have seen this coming... I'll work out a better way to do this ASAP, as you say a shared object with some form of synchronisation.
This was a stupid error and I have a pretty simple fix. Testing it now and sorry for the mess.
[update]There's currently a problem with Shaman, etc. Will test this and post PR ASAP[/update]
- Status changed from New to In Progress
- Status changed from In Progress to Resolved
- Status changed from Resolved to Pending Backport
- Backport set to jewel,kraken
Must be backported together with #19497 - i.e. total of two master PRs
- Copied to Backport #20172: jewel: PR #14886 creates a SafeTimer thread per PG added
- Copied to Backport #20173: kraken: PR #14886 creates a SafeTimer thread per PG added
- Status changed from Pending Backport to Resolved
Also available in: Atom
PDF