> On Aug. 2, 2019, 10:17 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > > Line 570 (original), 566 (patched) > > <https://reviews.apache.org/r/71193/diff/3/?file=2158799#file2158799line570> > > > > You are not changing the logic here but could you remind me why this > > error doesn't fail the update?
I think that we are just trying to be robust against host problems. This would only fail if there was a serious disk error, so we just try to maintain our internal consistency. > On Aug. 2, 2019, 10:17 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > > Lines 776 (patched) > > <https://reviews.apache.org/r/71193/diff/3/?file=2158799#file2158799line796> > > > > For `hashmap` there's already a `contains` defined. I did this to avoid doing multiple hash lookups, but I agree that using `contains` improves the readability. > On Aug. 2, 2019, 10:17 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > > Lines 778-779 (patched) > > <https://reviews.apache.org/r/71193/diff/3/?file=2158799#file2158799line798> > > > > You could've used a `else if` followed by an `else` to avoid another > > level of nesting. Aftew switching to `contains`, I think that this makes the most sense as is. > On Aug. 2, 2019, 10:17 p.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > > Line 807 (original) > > <https://reviews.apache.org/r/71193/diff/3/?file=2158799#file2158799line841> > > > > We could move the logging about the `dir` up above `erase` if we still > > want to log it (maybe for `VLOG(1)`)? Yeh, I think adding a `VLOG` makes sense here. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71193/#review217059 ----------------------------------------------------------- On July 30, 2019, 7:52 a.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71193/ > ----------------------------------------------------------- > > (Updated July 30, 2019, 7:52 a.m.) > > > Review request for mesos, Xudong Ni, Gilbert Song, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-9900 > https://issues.apache.org/jira/browse/MESOS-9900 > > > Repository: mesos > > > Description > ------- > > The `disk/xfs` isolator assumed that there would only be a single > directory path for each project quota. When we apply project quotas > to the overlayfs upperdir, that won't be true any more, since the > upperdir will come under the same quota as the task sandbox. > > Update the quota reclamation tracking to keep a set of disk paths that > the quota has been applied to, and only reclaim the project ID once all > those paths have been removed. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/xfs/disk.hpp > 94d44e7f39dc01037461015b499a1fc3169b24e8 > src/slave/containerizer/mesos/isolators/xfs/disk.cpp > 646330c65b24aa28801ec99d7909db08a3e05c79 > > > Diff: https://reviews.apache.org/r/71193/diff/3/ > > > Testing > ------- > > sudo make check (Frdora 30) > > > Thanks, > > James Peach > >
