----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/#review56179 -----------------------------------------------------------
src/slave/slave.cpp <https://reviews.apache.org/r/26525/#comment96501> I think it is weird to call a checkpoint*() function that is a no-op if it is not checkpointing. It is my bad, because I realize I did the same mistake with checkpointTask(). Can you fix both checkpointExecutor() and checkpointTask() to only be called when we are checkpointing? src/slave/slave.cpp <https://reviews.apache.org/r/26525/#comment96502> This should be a CHECK once you fix the callers. src/slave/slave.cpp <https://reviews.apache.org/r/26525/#comment96504> Change this to VLOG(1). src/slave/slave.cpp <https://reviews.apache.org/r/26525/#comment96503> ditto. this should be a CHECK. - Vinod Kone On Oct. 9, 2014, 9:39 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26525/ > ----------------------------------------------------------- > > (Updated Oct. 9, 2014, 9:39 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > There are two places where 'new Executor' is called: > 1) launchExecutor > 2) recoverExecutor > > For 2), we don't need checkpointing. Therefore, putting checkpointing code in > Executor constructor and use state != RECOVERING to disginguish is not > explicit and confusing. > > > Diffs > ----- > > src/slave/slave.hpp 28697102047b972ecb3b6b627ee089b430549fc0 > src/slave/slave.cpp e56dcbd80114730949a0d4b553470802a4d38281 > > Diff: https://reviews.apache.org/r/26525/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
