mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 29918: Introduced checkpoint function for Resources.
Date Thu, 15 Jan 2015 19:07:14 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message