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 55683: Rationalize process wait error checking.
Date Thu, 19 Jan 2017 01:11:38 GMT


> On Jan. 19, 2017, 12:43 a.m., Jiang Yan Xu wrote:
> > src/launcher/fetcher.cpp, line 114
> > <https://reviews.apache.org/r/55683/diff/1/?file=1607831#file1607831line114>
> >
> >     It's worth noting that 
> >     
> >     ```
> >     !WSUCCESS()
> >     ```
> >     
> >     translates to 
> >     
> >     ```
> >     !WIFEXITED(status) || WEXITSTATUS(status) != 0
> >     ```
> >     
> >     so it's not exactly the same as 
> >     
> >     
> >     ```
> >     WIFEXITED(status) && WEXITSTATUS(status) != 0
> >     ```
> >     
> >     But the result seems to be more correct than the previous: previously if the
subprocess is signaled, the fetcher would return `true`.
> >     
> >     This is then a bug fix which we could note in the description.

Right :)


> On Jan. 19, 2017, 12:43 a.m., Jiang Yan Xu wrote:
> > src/common/status_utils.hpp, line 26
> > <https://reviews.apache.org/r/55683/diff/1/?file=1607826#file1607826line26>
> >
> >     Use an adjective? `WSUCCESSFUL` for a boolean concept?

Agreed to rename it to `WSUCCEEDED`.


> On Jan. 19, 2017, 12:43 a.m., Jiang Yan Xu wrote:
> > src/common/status_utils.hpp, line 28
> > <https://reviews.apache.org/r/55683/diff/1/?file=1607826#file1607826line28>
> >
> >     I guess the same `#ifdef __WINDOWS__` guards apply?

Windows folks think this is probably OK.


- James


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


On Jan. 18, 2017, 6:25 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55683/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2017, 6:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, Michael Park, and Jiang
Yan Xu.
> 
> 
> Bugs: MESOS-6946
>     https://issues.apache.org/jira/browse/MESOS-6946
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce the WSUCCESS() API to test that a forked process exited
> successfully. Use it in all the places that were performing this test
> bespokely, and update error logs to user WSTRINGIFY() if appropriate.
> 
> 
> Diffs
> -----
> 
>   src/common/status_utils.hpp 26b94d49973dd61a6138e6a64d5ec0bdec6dea7e 
>   src/docker/executor.cpp 9b5c469e2d0f33e228ec746711e6bc6ed352cbc7 
>   src/hdfs/hdfs.cpp bebba8bb3c16f40293d21b5e00c353135a0f8f26 
>   src/launcher/default_executor.cpp 57e4799e750f8f5352a9fec58af40efe432ea865 
>   src/launcher/executor.cpp e035a4ee4438a3342f68a5548a5fd8d57a315cfa 
>   src/launcher/fetcher.cpp 5f9a38d3ae8fb9b4b21203f96d53c5bef0909eba 
>   src/linux/ns.hpp da43266ba67f76c9ca856584a540794062097691 
>   src/slave/containerizer/docker.cpp 7a8a7271b54af0b4dcdae7a2aa8a90a8a7d05fd6 
>   src/slave/containerizer/mesos/io/switchboard.cpp 2c4eed8f2dc7f91b9fd08f91f1b08484fca9a9af

>   src/slave/containerizer/mesos/launch.cpp e482ab8bdfc358f695b87cda72ca59fb64cd8c4d 
>   src/tests/script.cpp ef6b22aaeb056ed489f2c519e1a02644f2653329 
> 
> Diff: https://reviews.apache.org/r/55683/diff/
> 
> 
> Testing
> -------
> 
> make check && sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


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