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 48527: Add NetworkInfo.labels to CNI Network before passing to CNI plugin.
Date Fri, 17 Jun 2016 00:27:02 GMT


> On June 15, 2016, 6:08 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 987
> > <https://reviews.apache.org/r/48527/diff/2/?file=1417721#file1417721line987>
> >
> >     Is there a guarantee that the plugin would be waiting for the isolator to `write`
into the PIPE ?
> >     Also, this can potentially block the `network/cni` isolator if the plugin is
not fast enough in reading the pipe.
> >     
> >     Can we follow the existing idiom of writing mutated JSON into a temp file? Probably
checkpoint the JSON into the containers checkpoint DIR and set the stdinof the plugin to the
checkpointed JSON config. This would also serve the dual purpose of checkpointing the mutated
JSON to be used when deleteing the container.

See my comments below. I won't be too worried about the the pipe full issue as network config
is typically pretty small and the kernel buffer for a pipe is usually 64K. Also, the spec
says that the plugin will read the stdin for config.

If you're really worried about the blocking issue, use async process::io::write should solve
the issue.


- Jie


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


On June 15, 2016, 10:23 p.m., Dan Osborne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48527/
> -----------------------------------------------------------
> 
> (Updated June 15, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/48527
> 
> Fixes.
> 
> Review: https://reviews.apache.org/r/48754
> 
> Code review fixes.
> 
> 
> Checkpointed mutated networkinfo.
> 
> 
> Added error message if args is specified.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 09890cedf2e7a1846bd1cb250e117be1680a1b80

>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 106ff35320f37b2e75ed60381de1406459e6d515

>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 8af50c11b00240ac1d24605b119acbd96aaa50be

>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp b3fb0a32e3e74ca859ca58085efed5a87e4e626b

> 
> Diff: https://reviews.apache.org/r/48527/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>


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