You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2014/04/12 03:49:40 UTC
Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/
-----------------------------------------------------------
Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
3rdparty/libprocess/src/process.cpp 9654c0437edb43cff65dbefdf08dee9e18ef96ab
3rdparty/libprocess/src/tests/process_tests.cpp 9dae9310de89571f599333a8eb7ceeb47ccef54d
Diff: https://reviews.apache.org/r/20276/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On April 30, 2014, 5:45 a.m., Ben Mahler wrote:
> > Let me make sure I understand the backwards compatibility matrix here, assuming this lands in 0.19.0:
> >
> > (1) 0.18.0 libprocess -> 0.19.0 libprocess: OK (we don't send responses in presence of "libprocess" User-Agent)
> > (2) 0.19.0 libprocess -> 0.18.0 libprocess: OK (old libprocess never replies)
> >
> > (3) custom libprocess -> 0.18.0 libprocess: safe, custom libprocess gets no 202 replies
> > (4) 0.18.0 libprocess -> custom libprocess:
> > (a) SocketManager::link will use recv_data, which expects to decode requests but fails and closes socket upon receiving a response?
> > (b) SocketManager::send sockets will not be reading, TCP buffers will fill up and the custom libprocess send() calls will fail eventually?
Yes, that's correct. The idea is that everyone should adhere to the "if user agent is libprocess don't send a response" strategy then we should be okay.
> On April 30, 2014, 5:45 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2303-2309
> > <https://reviews.apache.org/r/20276/diff/2/?file=571359#file571359line2303>
> >
> > Maybe a comment that this ensures the libev watcher will consider the socket ready for reading? Is that what's required here?
Yup, added an extra sentence elaborating on that point.
> On April 30, 2014, 5:45 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, lines 1442-1449
> > <https://reviews.apache.org/r/20276/diff/2/?file=571360#file571360line1442>
> >
> > How does this test still pass? Seems like you're no longer sending a response when User-Agent contains "/libprocess", something I'm missing?
> >
> > Would it be better to split up this test into two cases:
> >
> > sending from libprocess -> no response
> > sending from fake libprocess -> 202
Ah, yes, that test does fail if just run by itself, but when run after applying https://reviews.apache.org/r/20277 (which is how I ran the tests) then everything passes. Fixed.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/#review41804
-----------------------------------------------------------
On April 30, 2014, 3:51 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20276/
> -----------------------------------------------------------
>
> (Updated April 30, 2014, 3:51 a.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
> 3rdparty/libprocess/src/process.cpp 26c16cf58c31102dc61b5844b3e4d75e5bc2764e
> 3rdparty/libprocess/src/tests/process_tests.cpp 745c3ada5f55722aed4adb4d0b1fcb16e4cb8e9b
>
> Diff: https://reviews.apache.org/r/20276/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/#review41804
-----------------------------------------------------------
Ship it!
Let me make sure I understand the backwards compatibility matrix here, assuming this lands in 0.19.0:
(1) 0.18.0 libprocess -> 0.19.0 libprocess: OK (we don't send responses in presence of "libprocess" User-Agent)
(2) 0.19.0 libprocess -> 0.18.0 libprocess: OK (old libprocess never replies)
(3) custom libprocess -> 0.18.0 libprocess: safe, custom libprocess gets no 202 replies
(4) 0.18.0 libprocess -> custom libprocess:
(a) SocketManager::link will use recv_data, which expects to decode requests but fails and closes socket upon receiving a response?
(b) SocketManager::send sockets will not be reading, TCP buffers will fill up and the custom libprocess send() calls will fail eventually?
3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/20276/#comment75429>
Maybe a comment that this ensures the libev watcher will consider the socket ready for reading? Is that what's required here?
3rdparty/libprocess/src/tests/process_tests.cpp
<https://reviews.apache.org/r/20276/#comment75430>
How does this test still pass? Seems like you're no longer sending a response when User-Agent contains "/libprocess", something I'm missing?
Would it be better to split up this test into two cases:
sending from libprocess -> no response
sending from fake libprocess -> 202
- Ben Mahler
On April 30, 2014, 3:51 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20276/
> -----------------------------------------------------------
>
> (Updated April 30, 2014, 3:51 a.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
> 3rdparty/libprocess/src/process.cpp 26c16cf58c31102dc61b5844b3e4d75e5bc2764e
> 3rdparty/libprocess/src/tests/process_tests.cpp 745c3ada5f55722aed4adb4d0b1fcb16e4cb8e9b
>
> Diff: https://reviews.apache.org/r/20276/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/
-----------------------------------------------------------
(Updated April 30, 2014, 3:51 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
Changes
-------
Only send HTTP responses if not a libprocess sender.
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
3rdparty/libprocess/src/process.cpp 26c16cf58c31102dc61b5844b3e4d75e5bc2764e
3rdparty/libprocess/src/tests/process_tests.cpp 745c3ada5f55722aed4adb4d0b1fcb16e4cb8e9b
Diff: https://reviews.apache.org/r/20276/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/#review40424
-----------------------------------------------------------
Out of curiosity, have you tested libprocess202 processes against libprocess processes? Curious if it breaks request multiplexing with plain-old-http?
- Brian Wickman
On April 12, 2014, 11:42 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20276/
> -----------------------------------------------------------
>
> (Updated April 12, 2014, 11:42 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
> 3rdparty/libprocess/src/process.cpp 9654c0437edb43cff65dbefdf08dee9e18ef96ab
> 3rdparty/libprocess/src/tests/process_tests.cpp 9dae9310de89571f599333a8eb7ceeb47ccef54d
>
> Diff: https://reviews.apache.org/r/20276/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/
-----------------------------------------------------------
(Updated April 12, 2014, 11:42 p.m.)
Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
Changes
-------
Updated dependencies.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
3rdparty/libprocess/src/process.cpp 9654c0437edb43cff65dbefdf08dee9e18ef96ab
3rdparty/libprocess/src/tests/process_tests.cpp 9dae9310de89571f599333a8eb7ceeb47ccef54d
Diff: https://reviews.apache.org/r/20276/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20276: Introduced 'Accepted' responses for libprocess
messages.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20276/
-----------------------------------------------------------
(Updated April 12, 2014, 5:02 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, Kevin Sweeney, and Brian Wickman.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp f85c06596ad7d9de4c2264ba1fbe13e8f1115f2c
3rdparty/libprocess/src/process.cpp 9654c0437edb43cff65dbefdf08dee9e18ef96ab
3rdparty/libprocess/src/tests/process_tests.cpp 9dae9310de89571f599333a8eb7ceeb47ccef54d
Diff: https://reviews.apache.org/r/20276/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman