mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 52135: Implemented the *_NESTED_CONTAINER calls in the agent API.
Date Thu, 22 Sep 2016 19:45:46 GMT


> On Sept. 22, 2016, 5:23 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, lines 3235-3280
> > <https://reviews.apache.org/r/52135/diff/2/?file=1507935#file1507935line3235>
> >
> >     Hmm. I would put them in their own separate tests since the setup for them is
easy enough.

Sounds good.


> On Sept. 22, 2016, 5:23 p.m., Vinod Kone wrote:
> > src/slave/http.cpp, lines 1977-1979
> > <https://reviews.apache.org/r/52135/diff/2/?file=1507934#file1507934line1977>
> >
> >     I'm wondering if we should include the whole ContainerTermination proto in the
response instead of just the status.  For example, the "message" and "reasons" might be useful
if the container OOMs.
> >     
> >     Thoughts?

Agreed, I'll punt on this for a separate follow-up.

My thinking for omitting them was that TaskState.Reason seemed unfortunately wide (many values
are not applicable here). So, I was hoping we could introduce a container specific termination
reason (OOM, OOD, etc). Note that right now the resources are not used, so it won't be useful
just yet.

For message, we could add it but I'd like to add it ideally after we have the Reason in place
(OOM, OOD, etc), to avoid users parsing / relying on the message format (that might change
underneath them).


- Benjamin


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


On Sept. 22, 2016, 6:06 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52135/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2016, 6:06 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2449
>     https://issues.apache.org/jira/browse/MESOS-2449
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the wiring for the *_NESTED_CONTAINER calls,
> including validation and calling into the containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 0aae14583faf95fee678b3a3424c96cdba531968 
>   src/tests/api_tests.cpp 26f99f7c337fbbc5278d1b30d3d5c8f659ddf5ca 
> 
> Diff: https://reviews.apache.org/r/52135/diff/
> 
> 
> Testing
> -------
> 
> Added a test that mocks out the containerizer.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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