> On Dec. 13, 2012, 7:04 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 766 > > <https://reviews.apache.org/r/7655/diff/7/?file=237631#file237631line766> > > > > Is this Future<bool> necessary?
probably not. i just copied it from code elsewhere (cgroups?) > On Dec. 13, 2012, 7:04 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 790 > > <https://reviews.apache.org/r/7655/diff/7/?file=237631#file237631line790> > > > > Can you add the future failure reason to this log message? > > > > Do you want to also check for discarded? !ready catches discarded no ? > On Dec. 13, 2012, 7:04 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 978 > > <https://reviews.apache.org/r/7655/diff/7/?file=237631#file237631line978> > > > > Is logging an error ok here? > > Is it ok to proceed, or do you want to bail out and take some action > > instead of calling update? good catch. we should bail. to be consistent with my part 4 review, the guy doing checkpointing should ensure the directory exists. so moved the directory creation to status update manager. > On Dec. 13, 2012, 7:04 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1268 > > <https://reviews.apache.org/r/7655/diff/7/?file=237631#file237631line1268> > > > > What cleanup? The gc? The destroyExecutor? Both? both > On Dec. 13, 2012, 7:04 p.m., Ben Mahler wrote: > > src/tests/status_update_manager_tests.cpp, line 160 > > <https://reviews.apache.org/r/7655/diff/7/?file=237634#file237634line160> > > > > Can you add a comment on what each test is exercising? It's a bit hard > > for me to follow this and our other tests, since they are quite verbose. added a comment inside the first test. the second one is obvious? - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7655/#review14450 ----------------------------------------------------------- On Dec. 12, 2012, 11:13 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7655/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2012, 11:13 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > Integrated SUM into slave. > > > Diffs > ----- > > include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 > src/Makefile.am c94736df660a25b58dc47c07d9c56c3c26152a66 > src/slave/gc.cpp fe22f9b557215514e3d432d36af9dc0c377c437b > src/slave/paths.hpp fbf3fd84fb8f2590311b18d2afec2d2e0d30ef0a > src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 > src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 > src/slave/status_update_manager.hpp PRE-CREATION > src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed > src/tests/status_update_manager_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/7655/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
