----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64095/#review192528 -----------------------------------------------------------
src/status_update_manager/status_update_manager_process.hpp Lines 588 (patched) <https://reviews.apache.org/r/64095/#comment270729> Could we switch to returning an `Owned` here instead of a raw pointer? src/status_update_manager/status_update_manager_process.hpp Lines 593-596 (patched) <https://reviews.apache.org/r/64095/#comment270730> What are the implications of this for the empty file case? It seems the caller must be aware that they should delete an empty file, but I'm not sure that they receive specific enough feedback for this. Perhaps `recover` should remove the file if it's empty? src/status_update_manager/status_update_manager_process.hpp Lines 609-610 (patched) <https://reviews.apache.org/r/64095/#comment270733> Is there a reason you're not using `O_APPEND` here? src/status_update_manager/status_update_manager_process.hpp Lines 650 (patched) <https://reviews.apache.org/r/64095/#comment270746> You can probably remove this comment. src/status_update_manager/status_update_manager_process.hpp Lines 651 (patched) <https://reviews.apache.org/r/64095/#comment270745> s/update stream/updates/ src/status_update_manager/status_update_manager_process.hpp Lines 654 (patched) <https://reviews.apache.org/r/64095/#comment270748> Given the log line above, I think this comment can be removed as well. - Greg Mann On Dec. 1, 2017, 1:42 a.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64095/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2017, 1:42 a.m.) > > > Review request for mesos and Greg Mann. > > > Bugs: MESOS-8197 > https://issues.apache.org/jira/browse/MESOS-8197 > > > Repository: mesos > > > Description > ------- > > This actor handles the checkpointing, recovery, and retry of status > updates. > > It will initially be used by the offer operations status update > manager, but it was designed and implemented so that it can replace > the current implementation of the task status update manager. > > > Diffs > ----- > > src/status_update_manager/status_update_manager_process.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/64095/diff/3/ > > > Testing > ------- > > Added new tests at the end of the chain. They passed 5000 times on GNU/Linux. > > > Thanks, > > Gaston Kleiman > >
