address RSS growth when there are excessive annotation events #6115
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6115 +/- ##
=======================================
Coverage 83.35% 83.35%
=======================================
Files 521 521
Lines 84458 84464 +6
=======================================
+ Hits 70401 70407 +6
Misses 14057 14057
|
|
FYI - I ran 1024 jobs on this branch with heap profiling enabled. This was without the repro for #6114, so just one annotation per job. I captured the diff between before running any jobs, and after all jobs were inactive. It seems like most of this makes sense, but posting this here for your information (according to heap profile, 1024 jobs results in ~47M of heap, mostly from storing the job information in memory it appears) Oh, and here's one after running |
Problem: schedutil_alloc_respond_pack() has a va_start() without a corresponding va_end(). Add the va_end().
Problem: events posted with the EVENT_NO_COMMIT flag are kept in job->eventlog, contributing to memory growth. These events are not important and need not be kept. This does mean that journal consumers like job-list will not see those events when they are reloaded and the journal is replayed, but since the idea is that these events are ephemeral, that is probably OK. Nothing is lost when the instance restarts since these events were never stored in the KVS. Fix up one test that was expecting job list to recreate annotations after a reload.
Problem: when the scheduler was malfunctioning and spamming the job manager with annotations, we had no tools for diagnosis. Count events that are posted to the journal and include this count in the 'flux module stats job-manager' output. A rapidly increasing event count would have been a helpful clue in the above case.
Problem: when handling a successful alloc callback from the scheduler, if the KVS commit of R fails (unlikely), the rank 0 broker may crash with a double free. In the KVS commit continuation, take the future off of the global cleanup list before checking for KVS error, not after, because the future is destroyed on all this function's return paths. Since the alloc context is in the future aux container with alloc_destroy() registered as a destructor, and the future is destroyed on the function's error path, drop the additonal call to alloc_destroy() on the error path.
garlick
changed the title
|
I tacked on a commit that fixes a bug on an unlikely error path in libschedutil, but haven't had any other insights into the current heap behavior. I'm dropping the WIP because I think this addresses the major problem and could go in without further ado. |
|
Thanks! Setting MWP. |
Progress towards making flux core more robust in situations like #6114. Marking WIP because there's still some unexplained RSS growth with "annotation spam" to run down.