> On Oct. 7, 2014, 9:46 p.m., Ben Mahler wrote: > > I'm curious why you need to expose both sides of the bounds? Our tests > > currently hard-code 1 second as the reap interval, and since Ian did not > > change the maximum, the tests continue to function as expected. > > > > Are you planning to follow up and adjust the tests? E.g. > > subprocess_tests.cpp, slave_recovery_tests.cpp, etc. > > > > Naming wise, how about: > > > > ``` > > process::REAP_INTERVAL(); > > ``` > > > > The examples in the ticket you referenced fall into two cases: > > > > (1) Static member functions (e.g. Duration::max() and Time::epoch()), and > > (2) Global functions (e.g. MAX_PING_TIMEOUT()). > > > > Since for (2) we've kept ALL_CAPS, we should stick to ALL_CAPS here for > > consistency. The benefit of ALL_CAPS for globals is that it provides a > > signal to the reader of the code that this is a constant value, despite the > > fact that we had to make it a function (for non-POD global safety).
The lower bound may be needed for testing situation when the reaper should not kick in, e.g. by setting the graceful shutdown timeout to the lower bound we test the force shutdwon path. The idea for the patch came when I had to hard-code both 1s and 100ms in my tests. I would propose we create a separate JIRA for updating the tests. Names are updated to `LOW_REAP_INTERVAL()` and `HIGH_REAP_INTERVAL()`. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/#review55718 ----------------------------------------------------------- On Oct. 7, 2014, 4:39 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26229/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2014, 4:39 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, > and Till Toenshoff. > > > Repository: mesos-git > > > Description > ------- > > Lower and upper bounds for the poll interval are refactored as static > functions visible to outer world. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/reap.hpp 9de5336 > 3rdparty/libprocess/src/reap.cpp ac14a86 > > Diff: https://reviews.apache.org/r/26229/diff/ > > > Testing > ------- > > make check (Mac OS 10.9.4, Ubuntu 14.04) > > > Thanks, > > Alexander Rukletsov > >
