> On June 3, 2013, 6:55 p.m., Vinod Kone wrote: > > src/slave/cgroups_isolator.cpp, line 320 > > <https://reviews.apache.org/r/11126/diff/4/?file=300088#file300088line320> > > > > we use camelcase for variables. > > > > also, why not just call it 'exists'?
I was trying to avoid ambiguity with the 'exists' just above. > On June 3, 2013, 6:55 p.m., Vinod Kone wrote: > > src/slave/cgroups_isolator.cpp, lines 342-348 > > <https://reviews.apache.org/r/11126/diff/4/?file=300088#file300088line342> > > > > why not kill the oddly sandwiched 'remove' inside the if loop and just > > always do remove here. IOW, > > > > if (exists) { > > create() > > } > > > > remove() I suppose that's okay if we don't care about providing some guarantee that the test cgroup is removed. - Brenden ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11126/#review21350 ----------------------------------------------------------- On June 3, 2013, 6:49 p.m., Brenden Matthews wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11126/ > ----------------------------------------------------------- > > (Updated June 3, 2013, 6:49 p.m.) > > > Review request for mesos. > > > Description > ------- > > Make sure you clean up the cgroups test. > > Sometimes the cgroup test fails because the directory is already there. > This should ensure that it gets cleanup up in that case before we exit. > > We'll also check that it exists before we try creating it. If it > already exists, we'll skip the creation. > > Review: https://reviews.apache.org/r/11126 > > > Diffs > ----- > > src/slave/cgroups_isolator.cpp 9b3a3a5dfec27a119fdd47a88f016e21470eb88d > > Diff: https://reviews.apache.org/r/11126/diff/ > > > Testing > ------- > > Used in production at airbnb. > > > Thanks, > > Brenden Matthews > >
