mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 70927: Fixed a memory "leak" of filters in the allocator.
Date Mon, 24 Jun 2019 19:54:16 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70927/
-----------------------------------------------------------

(Updated June 24, 2019, 7:54 p.m.)


Review request for mesos, Andrei Sekretenko and Meng Zhu.


Changes
-------

Moved the expiry future into the filter and exposed it.


Bugs: MESOS-9852
    https://issues.apache.org/jira/browse/MESOS-9852


Repository: mesos


Description
-------

Per MESOS-9852, the allocator does not keep a handle to the offer
filter timer, which means it cannot remove the timer if it's no
longer relevant (e.g. due to revive). This means that timers build
up in memory.

Also, the offer filter is allocated on the heap, and is not deleted
until the expiry of the timer (which may take forever!), even if the
offer filter is no longer relevant (e.g. due to revive). This means
that offer filters build up in memory.

The fix applied is to manage offer filters using a shared_ptr in the
maps, which means that they get deleted when erased. The expiration
functions need to now use a weak_ptr in case they run after the
offer filter has been erased (which is possible due to racing between
the expiry timer firing and the timer being discarded).

To discard the timers when no longer needed, the destructors of the
filters perform the discard.

This was tested against a test which spins in a revive + long decline
loop. Previously, the RSS continues to grow, but after this fix
it remains the same.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 25472eb7f5cb8342d2c88449c9c4145d816fb718 
  src/master/allocator/mesos/hierarchical.cpp 7076e968589c05c770cf50ad289bc7889625b54f 


Diff: https://reviews.apache.org/r/70927/diff/2/

Changes: https://reviews.apache.org/r/70927/diff/1-2/


Testing
-------

make check in addition to the leak test described in the description
and included in the subsequent patch (that's not planned to be committed)


Thanks,

Benjamin Mahler


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message