> On Nov. 21, 2016, 8:36 p.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/process.cpp, lines 3657-3682 > > <https://reviews.apache.org/r/53487/diff/4/?file=1568173#file1568173line3657> > > > > Can you add a TODO (in a separate patch) that we should handle > > discarded responses by not processing the request? > > > > As it stands, if the event is in the queue and the caller disconnects, > > we will still process the request here and deliver it to the route.
Good catch! would add the TODO in a subsequent patch. Also, would file another patch to take care of using `set()` instead of `associate()` as per another review comment. > On Nov. 21, 2016, 8:36 p.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/process.cpp, line 3729 > > <https://reviews.apache.org/r/53487/diff/4/?file=1568173#file1568173line3729> > > > > Seems to indicate that the ownership resides in the caller, as if you > > did: > > > > ``` > > void function(const unique_ptr& p) > > ``` > > > > Whereas the following would seem to suggest passing ownership: > > > > ``` > > void function(unique_ptr&& p) > > void function(unique_ptr p) > > ``` > > > > Also, in these latter options, you can move the pointer into a capture > > (not yet in C++11), whereas you couldn't with the first option. Used `Owned<Request> &&` instead. - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53487/#review156500 ----------------------------------------------------------- On Nov. 21, 2016, 7:25 p.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53487/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2016, 7:25 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-6466 > https://issues.apache.org/jira/browse/MESOS-6466 > > > Repository: mesos > > > Description > ------- > > Old libprocess style messages and routes not supporting request > streaming read the body from the piped reader. Otherwise, the > request is forwarded to the handler when the route supports > streaming. > > Review: https://reviews.apache.org/r/53487/ > > > Diffs > ----- > > 3rdparty/libprocess/include/process/process.hpp > f389d3d3b671e301a7ac911ad87ab13289e8c82f > 3rdparty/libprocess/src/process.cpp > ab2b5a9d38a3001d6a5daa1807fecb630c4b154d > > Diff: https://reviews.apache.org/r/53487/diff/ > > > Testing > ------- > > make check (Tests are added in https://reviews.apache.org/r/53490 i.e., after > we add support to the `Connection` abstraction for request streaming) > > > Thanks, > > Anand Mazumdar > >
