mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 56080: Fixed error handling when writing to a cgroup control file.
Date Mon, 30 Jan 2017 17:14:31 GMT

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



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);
}

```

- James Peach


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