From reviews-return-78626-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Fri Jun 8 17:38:11 2018 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 8EA5E18218 for ; Fri, 8 Jun 2018 17:38:11 +0000 (UTC) Received: (qmail 7758 invoked by uid 500); 8 Jun 2018 17:38:11 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 7721 invoked by uid 500); 8 Jun 2018 17:38:11 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 7706 invoked by uid 99); 8 Jun 2018 17:38:10 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 08 Jun 2018 17:38:10 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 62786C0101; Fri, 8 Jun 2018 17:38:10 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.399 X-Spam-Level: * X-Spam-Status: No, score=1.399 tagged_above=-999 required=6.31 tests=[HEADER_FROM_DIFFERENT_DOMAINS=0.249, HTML_MESSAGE=2, KAM_LAZY_DOMAIN_SECURITY=1, KAM_LOTSOFHASH=0.25, KAM_MANYTO=0.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id AI5HeTCHJ_Pi; Fri, 8 Jun 2018 17:38:07 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id 77A4B5F3CE; Fri, 8 Jun 2018 17:38:07 +0000 (UTC) Received: from reviews.apache.org (unknown [10.41.0.12]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 149CDE012B; Fri, 8 Jun 2018 17:38:07 +0000 (UTC) Received: from reviews-vm2.apache.org (localhost [IPv6:::1]) by reviews.apache.org (ASF Mail Server at reviews-vm2.apache.org) with ESMTP id D05F9C40220; Fri, 8 Jun 2018 17:38:06 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============7832370958262754951==" MIME-Version: 1.0 Subject: Re: Review Request 67386: Added `io::prepare_async` and `io::is_async` functions for libprocess. From: Andrew Schwartzmeyer To: Benjamin Mahler , Eric Mumau , Radhika Jandhyala , Joseph Wu , Andrew Schwartzmeyer , John Kordich Cc: Akash Gupta , mesos Date: Fri, 08 Jun 2018 17:38:06 -0000 Message-ID: <20180608173806.52747.73692@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Andrew Schwartzmeyer X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/67386/ X-Sender: Andrew Schwartzmeyer X-ReviewBoard-ShipIt: 1 References: <20180530185654.54257.9959@reviews-vm2.apache.org> In-Reply-To: <20180530185654.54257.9959@reviews-vm2.apache.org> Reply-To: Andrew Schwartzmeyer X-ReviewRequest-Repository: mesos --===============7832370958262754951== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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) s/UNIX/POSIX 3rdparty/libprocess/include/process/io.hpp Lines 58-60 (patched) 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) 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) Missing a space before the first backtick 3rdparty/libprocess/src/io.cpp Lines 136-147 (patched) 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 > > --===============7832370958262754951==--