You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2018/06/06 00:31:15 UTC

Re: Review Request 67389: Windows: Implemented Windows IOCP async backend.

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




3rdparty/libprocess/src/libwinio_impl.cpp
Lines 62-74 (patched)
<https://reviews.apache.org/r/67389/#comment286816>

    You can probably move this function's body into the `KEY_TIMER` switch case, since we wouldn't want to call this function anywhere else.



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 118 (patched)
<https://reviews.apache.org/r/67389/#comment286821>

    Shouldn't this buffer be twice as large to account for both local and remote addresses (+16 bytes for each, as required by `::AcceptEx`)?
    
    `unsigned char buf[2 * sizeof(SOCKADDR_STORAGE) + 32];`



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 237-240 (patched)
<https://reviews.apache.org/r/67389/#comment286810>

    Let's make this into a CHECK or fatal error.  We would definitely want to notice if this branch is possible.



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 295-305 (patched)
<https://reviews.apache.org/r/67389/#comment286789>

    Isn't this quick poll almost the same as letting the outer `while` loop repeat?
    
    Or is it possible that every GQCS call can be interrupted by APCs, if there are enough incoming APCs?



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 312-315 (patched)
<https://reviews.apache.org/r/67389/#comment286774>

    It would be ok to `LOG(FATAL)` in these cases and exit immediately.


- Joseph Wu


On May 30, 2018, 11:54 a.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67389/
> -----------------------------------------------------------
> 
> (Updated May 30, 2018, 11:54 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668, MESOS-8671 and MESOS-8672
>     https://issues.apache.org/jira/browse/MESOS-8668
>     https://issues.apache.org/jira/browse/MESOS-8671
>     https://issues.apache.org/jira/browse/MESOS-8672
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Windows IOCP async backend, called libwinio, provides a single
> threaded event loop implementation that allows for async IO and timer
> events. libwinio wraps the native Win32 async functions using
> libprocess's primitives, which makes it easier to use and more type
> safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/CMakeLists.txt cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/libwinio_impl.hpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_impl.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67389/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>