----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29918/#review68286 -----------------------------------------------------------
src/slave/state.hpp <https://reviews.apache.org/r/29918/#comment112462> What's the return value here? I don't think it's necessary given that we usually FATAL on checkpoint failures. Also, can you move the comments to the implementation. This looks like a limitation on the implementation (not on the interface). src/slave/state.cpp <https://reviews.apache.org/r/29918/#comment112461> Add a period at the end of a comment. src/slave/state.cpp <https://reviews.apache.org/r/29918/#comment112463> s/file// src/slave/state.cpp <https://reviews.apache.org/r/29918/#comment112464> Kill this. src/slave/state.cpp <https://reviews.apache.org/r/29918/#comment112467> You probably wanna move the TODO here. src/slave/state.cpp <https://reviews.apache.org/r/29918/#comment112466> You want to close the file before returning. src/tests/slave_recovery_tests.cpp <https://reviews.apache.org/r/29918/#comment112470> s/R/role/ src/tests/slave_recovery_tests.cpp <https://reviews.apache.org/r/29918/#comment112468> Try<int> fd = os::open( file, O_RDONLY | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IRGRP | S_IRWXO); src/tests/slave_recovery_tests.cpp <https://reviews.apache.org/r/29918/#comment112469> That makes me feel that we should support repeated field for protobuf::read. In that way, we could simplify the checkpoint function above as well (instead of doing multiple writes, we can make it a single write). - Jie Yu On Jan. 15, 2015, 6:30 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29918/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2015, 6:30 p.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and > Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Introduced checkpoint function for Resources. > > > Diffs > ----- > > src/slave/state.hpp 70777cf6ab681c29ca4df601fe47903e1dbdf41f > src/slave/state.cpp a36fa53099300ee03f051b0f5eaaafe9f1da68d1 > src/tests/slave_recovery_tests.cpp 809822e63b05a21418cd9297c927d656d6fd871d > > Diff: https://reviews.apache.org/r/29918/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Michael Park > >
