mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 35911: Moved StatusUpdate.uuid from required to optional.
Date Sat, 27 Jun 2015 00:36:54 GMT


> On June 26, 2015, 10:30 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, lines 2656-2663
> > <https://reviews.apache.org/r/35911/diff/1/?file=993611#file993611line2656>
> >
> >     s/pure clients/HTTP API/
> >     
> >     How is this even possible with the current driver and slave semsntics? Should
this be a CHECK?
> >     
> >     also, pull this up to #2633.

For the second comment, it's not possible with the driver, but some folks might be using some
pure language bindings (e.g. pesos, jesos, etc). More notably, want to set us up for validating
when the HTTP API comes, the CHECK seems unnecessarily brittle given we can just increment
the metric and stay online..?


> On June 26, 2015, 10:30 p.m., Vinod Kone wrote:
> > src/slave/status_update_manager.cpp, lines 319-321
> > <https://reviews.apache.org/r/35911/diff/1/?file=993612#file993612line319>
> >
> >     Why is this being done here? This should go inside stream::update()?

Sure, I've moved it there.


> On June 26, 2015, 10:30 p.m., Vinod Kone wrote:
> > src/sched/sched.cpp, line 702
> > <https://reviews.apache.org/r/35911/diff/1/?file=993609#file993609line702>
> >
> >     why add a new condition? is it possible for update to not have an uuid when
it's not from master/driver?

After chatting with vinod, decided to go with separate if conditions and update the comment
to guide the reader as to what happened here.


- Ben


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


On June 26, 2015, 9:11 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35911/
> -----------------------------------------------------------
> 
> (Updated June 26, 2015, 9:11 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-2940
>     https://issues.apache.org/jira/browse/MESOS-2940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `TaskStatus` contains an optional `UUID`, which should be unset for master-generated
updates.
> 
> This udpates the `StatusUpdate` 'uuid' to also be optional.
> 
> Since the master and slave assume the 'uuid' is set, this adds CHECKs for now. Later,
we'll want to properly validate this instead. I left a TODO for this.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19 
>   src/common/type_utils.cpp f88dff76a0dd57f5e16dc4d908655b4bb9d460a5 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/messages/messages.proto 1c8d79e3fca365520cdd67051f8730593955cab6 
>   src/sched/sched.cpp bc76c71ae9d44bdddd291049223366e38cb0fd0c 
>   src/scheduler/scheduler.cpp 1efc6fb351e49deaa8f626823592bc9155f5137b 
>   src/slave/slave.cpp b3e1ccc28d2698d8bc91829d70787dc2fc17b80d 
>   src/slave/status_update_manager.cpp 0ad24500c3007202263794fd094fa60535d8f58c 
> 
> Diff: https://reviews.apache.org/r/35911/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


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