You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2018/06/08 17:38:06 UTC
Re: Review Request 67386: Added `io::prepare_async` and `io::is_async`
functions for libprocess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67386/#review204498
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/include/process/io.hpp
Lines 49 (patched)
<https://reviews.apache.org/r/67386/#comment286974>
s/UNIX/POSIX
3rdparty/libprocess/include/process/io.hpp
Lines 58-60 (patched)
<https://reviews.apache.org/r/67386/#comment286978>
Maybe expand that `asynchronous` on POSIX means non-blocking and on Windows means overlapped and associated with an IOCP.
3rdparty/libprocess/include/process/io.hpp
Lines 60-91 (original), 78-111 (patched)
<https://reviews.apache.org/r/67386/#comment286977>
Just to note: this has semantically been true all along, it's not a new requirement we're adding to libprocess.
3rdparty/libprocess/include/process/io.hpp
Line 88 (original), 107 (patched)
<https://reviews.apache.org/r/67386/#comment286975>
Missing a space before the first backtick
3rdparty/libprocess/src/io.cpp
Lines 136-147 (patched)
<https://reviews.apache.org/r/67386/#comment286976>
Just to clarify: the reason we're not using the `os::` functions directly is that for Windows IOCP this step requires access to information (the handle which is the IO Completion Port) only available at the level of libprocess, not stout, right? We should probably document this so this makes sense to non-Windows people.
- Andrew Schwartzmeyer
On May 30, 2018, 11:56 a.m., Akash Gupta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67386/
> -----------------------------------------------------------
>
> (Updated May 30, 2018, 11:56 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
>
>
> Bugs: MESOS-5371 and MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-5371
> https://issues.apache.org/jira/browse/MESOS-8668
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `os::nonblock` and `os::isNonBlock` are stubs that only do things for
> Windows sockets. For the IOCP implementation, we need to associate the
> handles with the IOCP hande, which is created by libprocess. Thus, we
> need these new functions to prepare the handle for the async IO
> functions by either setting the hande to non-blocking on UNIX systems
> or by associating it with an IOCP handle on Windows.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/io.hpp cc2caf44e065bed40263f3820e95a4f7c378bb98
> 3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba
> 3rdparty/libprocess/src/socket.cpp 504cb541785650d2d05aabd25f5258b9bad52baa
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 568d77b94b3afef8c1ebdde71e14aa0b498da5c8
>
>
> Diff: https://reviews.apache.org/r/67386/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Akash Gupta
>
>