mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitry Zhuk <dz...@twopensource.com>
Subject Re: Review Request 56080: Fixed error handling when writing to a cgroup control file.
Date Mon, 30 Jan 2017 18:56:07 GMT


> On Jan. 30, 2017, 5:14 p.m., James Peach wrote:
> > Since you need to detect errors, why use `iostreams` at all? This code is just writing
a buffer to a file, so `os::write` would let you do the same thing with lower overhead and
improved error handling.
> > 
> > AFAICT this function could be written as:
> > ```C
> > static Try<Nothing> write(
> >     const string& hierarchy,
> >     const string& cgroup,
> >     const string& control,
> >     const string& value)
> > {
> >   string path = path::join(hierarchy, cgroup, control);
> >   return os::write(path, value);
> > }
> > 
> > ```

There's similar code in `read`, which can be replaced with `os::read` (issue in `TODO(benh)`
seems to be resolved already).
Shall this be fixed in this review request? Or I can sumbit a separate review for `os::read`
and `os::write`, though it probably worth merging this patch to update tests.


- Dmitry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56080/#review163532
-----------------------------------------------------------


On Jan. 30, 2017, 3:20 p.m., Dmitry Zhuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56080/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7020
>     https://issues.apache.org/jira/browse/MESOS-7020
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Due to internal buffering in ofstream, actual write to the underlying
> device may not be performed by the time of checking for failures.
> This patch flushes written data to ensure fail check uses actual value.
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
>   src/tests/containerizer/cgroups_tests.cpp 5c1ef70fb1feb76de602418286989b48563f26ff

> 
> Diff: https://reviews.apache.org/r/56080/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message