----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39276/#review102607 -----------------------------------------------------------
Going to look at the test now, but wanted to provide this feedback first. 3rdparty/libprocess/include/process/event.hpp (line 128) <https://reviews.apache.org/r/39276/#comment160334> CHECK_NOTNULL(promise); And you might as well do this for the request too! CHECK_NOTNULL(request); CHECK_NOTNULL(promise); 3rdparty/libprocess/src/process.cpp (line 2345) <https://reviews.apache.org/r/39276/#comment160335> Promise<Response>* promise = new Promise<Response>(); Or: auto promise = new Promise<Response>(); 3rdparty/libprocess/src/process.cpp (line 2354) <https://reviews.apache.org/r/39276/#comment160336> Let's clean this up a little bit. IIUC this is the only place in the code where we dispatch to HttpProxy::handle, and it's not clear that we need a pointer to the future (it's a shared_ptr under the covers anyway). So how about we do something like: HttpProxy::handle(Future<Response>&& future, Request&& request) { items.push(new Item(move(future), move(request))); ...; } Then at the call site: // Dispatch to the proxy needs to be done at this point ... // ... // Also note that we need to pass a copy of 'request' because HttpProxy::handle takes ownership. dispatch(proxy, &HttpProxy::handle, promise->future(), Request(*request)); Sound good? Or am I missing something? - Benjamin Hindman On Oct. 14, 2015, 12:04 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39276/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2015, 12:04 p.m.) > > > Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff. > > > Bugs: MESOS-3705 > https://issues.apache.org/jira/browse/MESOS-3705 > > > Repository: mesos > > > Description > ------- > > When using the same socket to send multiple HTTP requests to different > actors. If the actor responsible for handling the first request is stuck > handling another event while a subsequent request can reply immediatly, the > order of the responses is altered violating HTTP Pipelining. > > This patch fixes that problem enforcing that every response is sent in the > order the corresponding request arrived. It also adds a test to reproduce the > issue and verify the fix works. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/event.hpp > 16ddbd77afa6efdf6bad201aa497ee102aa863ae > 3rdparty/libprocess/src/process.cpp > 0454554e7b6a39f94cfea02f08ca51ef6b35859a > 3rdparty/libprocess/src/tests/http_tests.cpp > d13d3888abbf3db552df4a9f83e54667e598ded9 > > Diff: https://reviews.apache.org/r/39276/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
