> 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). > > Alexander Rukletsov wrote: > 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()`.
> by setting the graceful shutdown timeout to the lower bound we test the force > shutdwon path I didn't quite grasp this, and it sounds a bit racy to me. Can you explain a bit more or point me to the patch where you need to know the minimum? I would grab you on IRC but I don't see you on there. :) - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/#review55718 ----------------------------------------------------------- On Oct. 8, 2014, 9:30 a.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26229/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2014, 9:30 a.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 > >
