You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2016/12/02 06:45:44 UTC
Review Request 54295: Added a synchronous version of loop for
io::read/write/redirect.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54295/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Jie Yu.
Repository: mesos
Description
-------
Added a synchronous version of loop for io::read/write/redirect.
Diffs
-----
3rdparty/libprocess/include/process/loop.hpp a78ea7da8a5a778d816a64fb46d9479cc2a4ed70
3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5
Diff: https://reviews.apache.org/r/54295/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 54295: Added a synchronous version of loop for
io::read/write/redirect.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54295/#review157814
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/include/process/loop.hpp (lines 32 - 36)
<https://reviews.apache.org/r/54295/#comment228413>
Can you update this paragraph accordingly?
3rdparty/libprocess/include/process/loop.hpp (lines 91 - 94)
<https://reviews.apache.org/r/54295/#comment228414>
This needs an update?
3rdparty/libprocess/include/process/loop.hpp (lines 169 - 178)
<https://reviews.apache.org/r/54295/#comment228412>
Do you want to capture a lambda here like you did for `discard` and `continuation`?
- Benjamin Mahler
On Dec. 2, 2016, 6:45 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54295/
> -----------------------------------------------------------
>
> (Updated Dec. 2, 2016, 6:45 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a synchronous version of loop for io::read/write/redirect.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/loop.hpp a78ea7da8a5a778d816a64fb46d9479cc2a4ed70
> 3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5
>
> Diff: https://reviews.apache.org/r/54295/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 54295: Added a synchronous version of loop for
io::read/write/redirect.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54295/#review157754
-----------------------------------------------------------
Patch looks great!
Reviews applied: [54295]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 2, 2016, 6:45 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54295/
> -----------------------------------------------------------
>
> (Updated Dec. 2, 2016, 6:45 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a synchronous version of loop for io::read/write/redirect.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/loop.hpp a78ea7da8a5a778d816a64fb46d9479cc2a4ed70
> 3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5
>
> Diff: https://reviews.apache.org/r/54295/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 54295: Added a synchronous version of loop for
io::read/write/redirect.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Dec. 6, 2016, 2:12 a.m., Benjamin Mahler wrote:
> > Is it possible to split the optional pid change from the discard logic change? If so that would be great!
> >
> > Looks like a chunk from the next patch slipped into this one?
Also, would be great to update the description to say that this also updates the discard logic to resolve racing (and which race in particular).
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54295/#review158095
-----------------------------------------------------------
On Dec. 5, 2016, 4:35 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54295/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2016, 4:35 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a synchronous version of loop for io::read/write/redirect.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/loop.hpp a78ea7da8a5a778d816a64fb46d9479cc2a4ed70
> 3rdparty/libprocess/src/http.cpp fc55bda0fae8a43794fb938718b3a720dc342473
> 3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5
>
> Diff: https://reviews.apache.org/r/54295/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 54295: Added a synchronous version of loop for
io::read/write/redirect.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54295/#review158095
-----------------------------------------------------------
Fix it, then Ship it!
Is it possible to split the optional pid change from the discard logic change? If so that would be great!
Looks like a chunk from the next patch slipped into this one?
3rdparty/libprocess/include/process/loop.hpp (line 102)
<https://reviews.apache.org/r/54295/#comment228802>
Where is ValueType defined? It looks like it's in the next review? Did you mean to include this chunk in the next review instead of this one? These don't seem to be used in this patch?
3rdparty/libprocess/include/process/loop.hpp (line 291)
<https://reviews.apache.org/r/54295/#comment228805>
Maybe add a comment about what the mutex is for?
- Benjamin Mahler
On Dec. 5, 2016, 4:35 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54295/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2016, 4:35 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a synchronous version of loop for io::read/write/redirect.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/loop.hpp a78ea7da8a5a778d816a64fb46d9479cc2a4ed70
> 3rdparty/libprocess/src/http.cpp fc55bda0fae8a43794fb938718b3a720dc342473
> 3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5
>
> Diff: https://reviews.apache.org/r/54295/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 54295: Added a synchronous version of loop for
io::read/write/redirect.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54295/
-----------------------------------------------------------
(Updated Dec. 5, 2016, 4:35 a.m.)
Review request for mesos, Benjamin Mahler and Jie Yu.
Repository: mesos
Description
-------
Added a synchronous version of loop for io::read/write/redirect.
Diffs (updated)
-----
3rdparty/libprocess/include/process/loop.hpp a78ea7da8a5a778d816a64fb46d9479cc2a4ed70
3rdparty/libprocess/src/http.cpp fc55bda0fae8a43794fb938718b3a720dc342473
3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5
Diff: https://reviews.apache.org/r/54295/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman