----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8535/#review15402 -----------------------------------------------------------
src/messages/messages.proto <https://reviews.apache.org/r/8535/#comment33166> I can't give a shipit if this is required, you'll have to either: 1. Convince me it's ok to leave this required. 2. Document an upgrade plan that allows for this to be required. You might do 1 by telling me that this message is only contained within the slave, and never hits the master? Will the slave and executors be rolled to have this new proto at the same time? src/slave/cgroups_isolation_module.cpp <https://reviews.apache.org/r/8535/#comment33168> What is path? I think this should be renamed.. to pidCheckpointPath? For that matter, isn't the bool redundant? checkpoint == pidCheckpointPath.isSome(), right? ditto on this comment in each isolation module src/slave/slave.cpp <https://reviews.apache.org/r/8535/#comment33169> "Could not" or "Unable to" or "Failed to" src/slave/slave.cpp <https://reviews.apache.org/r/8535/#comment33170> Kill this TODO? Seems outdated.. src/tests/slave_state_tests.cpp <https://reviews.apache.org/r/8535/#comment33171> You also want to check that this is the result is true, right? - Ben Mahler On Dec. 19, 2012, 1:32 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8535/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2012, 1:32 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > Checkpoints slave id, framework pid, executor pids (libprocess, execed, > forked) and task info. > > > Diffs > ----- > > src/exec/exec.cpp 821a94fab1f5969183ecf9e28d7b6bc10920db24 > src/messages/messages.proto 815fcbbcb4a8643f50950a294cedf7281b2a187f > src/slave/cgroups_isolation_module.hpp > 669efa14ba2603764aa68ae19a44e79dbfdec192 > src/slave/cgroups_isolation_module.cpp > 0f2975d1adf874dba4d0a539eb5c99233cef6e6b > src/slave/flags.hpp 39e57f4104ee7a1538436ebbb9493581e28c99dd > src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 > src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 > src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c > src/slave/process_based_isolation_module.hpp > f1817192582e3646f8dcf17934ba7998829e8fd6 > src/slave/process_based_isolation_module.cpp > 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 > src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 > src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 > src/slave/state.hpp c1e4c782380b0076313d2216c40e86774050d445 > src/slave/state.cpp 4d4c2470c44fe630ec0694ee937131b4f0aafc4e > src/tests/protobuf_io_tests.cpp 979efac0f28c3d361f5c347f15933d89d9356bc9 > src/tests/slave_state_tests.cpp 1136ea93cffb1483458edad2605b0b4f83b61c44 > src/tests/status_update_manager_tests.cpp PRE-CREATION > src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 > > Diff: https://reviews.apache.org/r/8535/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
