mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.
Date Fri, 13 Jan 2017 23:08:23 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161611
-----------------------------------------------------------


Fix it, then Ship it!




Looks like there are a few more cases of status update acknowledgements that have this UUID
issue.


src/master/master.cpp (line 5824)
<https://reviews.apache.org/r/55509/#comment232870>

    Actually preincrement is the more common/style-guide-conforming style in Mesos and in
the rest of the file, it's the other instances in this method that are inconsistent.
    
    https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement



src/master/master.cpp (lines 5829 - 5830)
<https://reviews.apache.org/r/55509/#comment232872>

    About
    
    > ... from agent pid  with id ...
    
    If we move this check down to be after 
    
    ```
    Slave* slave = slaves.registered.get(update.slave_id());
    
    if (slave == nullptr) {
    ...
    }
    ```
    
    Then you can just use the output overload to do
    
    ```
    "agent " << *slave << ...;
    ```
    
    which prints the agent info more uniformly.


- Jiang Yan Xu


On Jan. 13, 2017, 10:41 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6920
>     https://issues.apache.org/jira/browse/MESOS-6920
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040 
> 
> Diff: https://reviews.apache.org/r/55509/diff/
> 
> 
> Testing
> -------
> 
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
> 
> 
> Thanks,
> 
> James Peach
> 
>


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