----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63977/#review194660 -----------------------------------------------------------
Fix it, then Ship it! src/slave/slave.cpp Lines 1294-1296 (original), 1293-1295 (patched) <https://reviews.apache.org/r/63977/#comment273594> Stale? References downgrade src/slave/slave.cpp Lines 1538-1543 (patched) <https://reviews.apache.org/r/63977/#comment273593> What's the consequence of sending a partially downgraded result? Maybe we should spell that out as it's not that obvious to me src/slave/slave.cpp Line 1678 (original), 1660 (patched) <https://reviews.apache.org/r/63977/#comment273592> nice! src/slave/state.hpp Lines 96 (patched) <https://reviews.apache.org/r/63977/#comment273590> newline between the paragraphs? ``` // We ignore the `Try` from `downgradeResources` here because // for now, we checkpoint the result either way. // // TODO(mpark): Do something smarter with the result once // something like an agent recovery capability is introduced. ``` It's a little non-obvious to me what the implications of this are.. maybe document it? Does it mean that we checkpoint only partially downgraded and that's fine so long as we don't downgrade beyond the 1.x version that introduced refinement..? src/tests/slave_recovery_tests.cpp Lines 152-155 (patched) <https://reviews.apache.org/r/63977/#comment273589> Do you want a TODO to use a reflection based upgrade here? - Benjamin Mahler On Nov. 21, 2017, 6:07 a.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63977/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2017, 6:07 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-8221 > https://issues.apache.org/jira/browse/MESOS-8221 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/master/master.cpp cf8a22b636210e9f8d326592d8b64eab2c97caa4 > src/slave/slave.cpp 00935616c2328e2ad9fc170199298a3630a701ab > src/slave/state.hpp aaf8e5c2895e1cd88172c5bf73f028b629dc12c7 > src/tests/slave_recovery_tests.cpp bf2c5fcabdd4c16bdf5de1b641060e38783b8ee8 > > > Diff: https://reviews.apache.org/r/63977/diff/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Michael Park > >
