> On Oct. 29, 2012, 8:35 p.m., Jie Yu wrote: > > src/linux/cgroups.cpp, line 1331 > > <https://reviews.apache.org/r/7756/diff/1/?file=181105#file181105line1331> > > > > Can we use Options::None() as default here and set "RETRIES" in the > > client function?
So now, after discussing with benh, I've removed the option and stuck with just unsigned int. This is because we don't want to expose "blocking" behavior in which the Freezer / EmptyWatcher retries forever. > On Oct. 29, 2012, 8:35 p.m., Jie Yu wrote: > > src/linux/cgroups.cpp, line 1374 > > <https://reviews.apache.org/r/7756/diff/1/?file=181105#file181105line1374> > > > > Add a comment here. There are three outcomes: > > 1) return true -> success > > 2) return false -> retry limit reached > > 3) return error -> invalid arguments Added to the future() signature above. > On Oct. 29, 2012, 8:35 p.m., Jie Yu wrote: > > src/linux/cgroups.cpp, line 1469 > > <https://reviews.apache.org/r/7756/diff/1/?file=181105#file181105line1469> > > > > Here, we do need to check the result of the previous function in the > > function call chain. So, we should introduce a parameter in these functions > > and move the comments from freeze() to here. Good point, moved the comment to kill(). However, I didn't add the boolean result from freeze, since: 1. We still kill / thaw / empty regardless of whether the freeze worked. 2. If the freeze did not work, it gets logged within the Freezer. I could be convinced otherwise if you had a particular reason for passing in the freeze result? > On Oct. 29, 2012, 8:35 p.m., Jie Yu wrote: > > src/linux/cgroups.cpp, line 1517 > > <https://reviews.apache.org/r/7756/diff/1/?file=181105#file181105line1517> > > > > Instead of copying the code here, let's refactor it to have a common > > helper function. Done. Made a killTasks(). - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7756/#review12879 ----------------------------------------------------------- On Oct. 29, 2012, 9:28 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7756/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2012, 9:28 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > Parent Review: https://reviews.apache.org/r/7712/ > > We'd like to retry the freeze, kill, thaw, watchEmpty steps when the freezing > of the cgroups fails. > > This simply retries the above steps when the cgroup does not become empty in > the given time interval. > > > Diffs > ----- > > src/linux/cgroups.hpp 8147919bf7f17e50e047f813db58f99fb9dfffe5 > src/linux/cgroups.cpp a6056c31d504f5570872c0ed123b28400640d5cf > > Diff: https://reviews.apache.org/r/7756/diff/ > > > Testing > ------- > > make check on ubuntu > > This version punts on testing for now, but testing suggestions welcome! > > > Thanks, > > Ben Mahler > >
