mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 64299: Passed versions when launching tasks.
Date Thu, 07 Dec 2017 17:33:59 GMT

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




src/slave/slave.cpp
Lines 2267 (patched)
<https://reviews.apache.org/r/64299/#comment271643>

    Can you add a TODO about that this is a best effort check because we don't check executor's
resources.



src/slave/slave.cpp
Lines 2299-2322 (patched)
<https://reviews.apache.org/r/64299/#comment271642>

    I'd suggest you combine this with that right below (line 2363). For instance, we forgot
that we cannot send TASK_DROPPED if framework is not partition aware.
    
    We probably should move the LOG(INFO) about launching tasks below all those checks.
    
    Also, I feel that we can probably combine some other checks too. I saw this code being
duplicated three times in this function :(


- Jie Yu


On Dec. 4, 2017, 1:12 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64299/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2017, 1:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8222
>     https://issues.apache.org/jira/browse/MESOS-8222
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In this patch we inject resource versions into task launch messages
> and add verification in the agent. We require that resource versions
> of resource providers whose resources are used in a task have not
> changed. With that we can make sure to e.g., not use resources created
> in speculated operations.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 883793a226849434eb833c3027d121635a86fdc4 
>   src/slave/slave.hpp 643d8559244f0842e82b293c1ef99cb26e111059 
>   src/slave/slave.cpp c07e25f668bef6b6fa3cae4b69ba90de3eb3bdcc 
>   src/tests/mock_slave.hpp d986125b3ab67e0ca3d6ba264795850b3eadb3d0 
>   src/tests/mock_slave.cpp 6d050ca76acee737f1996fbbd8ecdefbf1a699ee 
>   src/tests/slave_tests.cpp a7f6658fa467914cd74691b5631d26d85ba7c1eb 
> 
> 
> Diff: https://reviews.apache.org/r/64299/diff/1/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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