----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10747/#review19808 -----------------------------------------------------------
src/slave/cgroups_isolator.cpp <https://reviews.apache.org/r/10747/#comment40850> one arg per line. src/slave/cgroups_isolator.cpp <https://reviews.apache.org/r/10747/#comment40851> How about if (!status.isReady()) { LOG(ERROR) << "Failed to get status for " << pid << ": " << (status.isFailed() ? status.failure() : "discarded") } src/slave/cgroups_isolator.cpp <https://reviews.apache.org/r/10747/#comment40865> This reads different here and in slave.cpp. The reason I wanted you to print status here is because for cgroupsisolator there is a delay between when reaped() is called and when executorTerminated() is sent to the slave. And I wondered if the exit status would be useful if we ever failed to send executorTerminated (e.g., FATAL error in cgroups cleanup). To sum, either we should make the log message consistent or just not print the status. Thoughts? src/slave/process_isolator.cpp <https://reviews.apache.org/r/10747/#comment40852> thank you src/slave/process_isolator.cpp <https://reviews.apache.org/r/10747/#comment40853> ditto src/slave/reaper.hpp <https://reviews.apache.org/r/10747/#comment40854> s/ a Future in a failed state/failed Future/ src/slave/reaper.hpp <https://reviews.apache.org/r/10747/#comment40855> s/, / and the process hasn't been reaped yet/ ? src/slave/reaper.hpp <https://reviews.apache.org/r/10747/#comment40857> s/of/in/ ? If you like, you can kill the second sentence altogether. - Vinod Kone On April 26, 2013, 7:57 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10747/ > ----------------------------------------------------------- > > (Updated April 26, 2013, 7:57 p.m.) > > > Review request for mesos, Benjamin Hindman, Vinod Kone, and Ben Mahler. > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/slave/cgroups_isolator.hpp f8fabc4e1c3c303b35a76db96b4b2479bd7c8ff8 > src/slave/cgroups_isolator.cpp 8b79da50d8fb0c2c8716dd7d2c734b65c32f60b4 > src/slave/process_isolator.hpp 9875f4a6e8e109e31ad390fbd7a84d03ad747190 > src/slave/process_isolator.cpp 6e2af87d291d7c3448393c1ffa816f7020e2dff6 > src/slave/reaper.hpp 09844d8d47b143ee369e0c82b19d65a774df4a90 > src/slave/reaper.cpp bd3dcef07c370ad338b478755bf8f7ce6408e4a3 > src/slave/slave.hpp f9d9c2b32983700fb09b266a0abf2a396174ae05 > src/slave/slave.cpp a0d3864d3e02c7889646f9ff004dad82f67d263b > src/tests/reaper_tests.cpp 0809c1ff17eb949beb1bdd922fdced022aa202f3 > > Diff: https://reviews.apache.org/r/10747/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jiang Yan Xu > >
