mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 51511: Added check in FileEncoder's destructor.
Date Mon, 12 Sep 2016 18:55:29 GMT


> On Sept. 12, 2016, 2:11 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/encoder.hpp, line 259
> > <https://reviews.apache.org/r/51511/diff/1/?file=1488353#file1488353line259>
> >
> >     Do we want to fast fail on all errors returned by `close` or just `EBADFD`?
> >     This can fail on `EINTR` or `EIO` right?

According to the Posix spec: http://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html
> If close() is interrupted by a signal that is to be caught, it shall return -1 with errno
set to [EINTR] and the state of fildes is unspecified. If an I/O error occurred while reading
from or writing to the file system during close(), it may return -1 with errno set to [EIO];
if this error is returned, the state of fildes is unspecified.

For EINTR, the master doesn't handle signals and the agent only handles the SIGUSR, which
terminates the agent anyway.

For EIO, the man page (http://linux.die.net/man/2/close ), this seems to be filesystem-dependent:
> This can especially be observed with NFS and with disk quota

At the same time, we aren't doing writes with the `FileEncoder`, so EIO is unexpected in most
cases.

---

Based on that, I'd argue it would be safer to fail on all `close` errors.


- Joseph


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


On Aug. 29, 2016, 6 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51511/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2016, 6 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-6104
>     https://issues.apache.org/jira/browse/MESOS-6104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This check will force libprocess to fail fast if a file descriptor
> is closed underneath it.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/encoder.hpp 9cd0d3f55f2e9c9dc9ebb97b34a23375f8d4e07f 
> 
> Diff: https://reviews.apache.org/r/51511/diff/
> 
> 
> Testing
> -------
> 
> Re-ran the steps in the previous review for several minutes.  Saw that the master did
not CHECK fail.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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