----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60105/#review178830 -----------------------------------------------------------
Fix it, then Ship it! src/slave/slave.cpp Lines 6011-6020 (patched) <https://reviews.apache.org/r/60105/#comment253074> Could you use the code in my comment: ``` if (!state->rebooted) { return Failure(message); } LOG(WARNING) << "Falling back to recover as a new agent due to error: " << message; // Clean up the slave state to avoid any state recovery for the // old agent. slaveState = None(); ``` I'll comment on the specific lines for the reasoning. src/slave/slave.cpp Lines 6013 (patched) <https://reviews.apache.org/r/60105/#comment253075> `else`: I was suggesting pulling up the direct return case so we can avoid another `else` block. Normally one style is not necessary superior than the other but in this case it helps avoid three levels if `if ... else` branching. src/slave/slave.cpp Lines 6014 (patched) <https://reviews.apache.org/r/60105/#comment253076> s/INFO/WARNING/. src/slave/slave.cpp Lines 6015 (patched) <https://reviews.apache.org/r/60105/#comment253078> No `\n` necessary. src/slave/slave.cpp Lines 6016 (patched) <https://reviews.apache.org/r/60105/#comment253079> We normally in logs first state what action is taken and then attach the error after `: `. In this case since the error has multiple lines, let's state the action first so it's not buried by the error. - Jiang Yan Xu On June 23, 2017, 2:30 p.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60105/ > ----------------------------------------------------------- > > (Updated June 23, 2017, 2:30 p.m.) > > > Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu. > > > Bugs: MESOS-6223 > https://issues.apache.org/jira/browse/MESOS-6223 > > > Repository: mesos > > > Description > ------- > > Prior to Mesos 1.4 we bypass the state recovery and > start as a new agent upon reboot. Starting in Mesos 1.4 > we'll attempt to recover the slave state even after reboot > except for when there is a SlaveInfo mismatch. > Here, we cleanup the slave state for a rebooted agent if > there's been a SlaveInfo mismatch during recovery to > ensure that no other state is recovered and the > agent enventually registers as a new agent. > > > Diffs > ----- > > src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c > > > Diff: https://reviews.apache.org/r/60105/diff/8/ > > > Testing > ------- > > make check done together with 60104 and 56895 > > > Thanks, > > Megha Sharma > >
