Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-11-12 Thread Alexander Rojas 


> On Nov. 10, 2015, 1:52 p.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2342
> > 
> >
> > No pointer +  std::move.

The pointer is needed for a follow up patch, since it is passed to a lambda and 
C++11 doesn't allow for rvalue references in lambdas (C++14 do though).


> On Nov. 10, 2015, 1:52 p.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2339-2341
> > 
> >
> > if we do move, remove this comment.

See below.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review105821
---


On Nov. 10, 2015, 4:58 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Nov. 10, 2015, 4:58 a.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 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 7eb4ef187b2cb358c370d0381db65b8e18668bab 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-11-10 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review105821
---



3rdparty/libprocess/include/process/event.hpp (line 145)


rename to `response`



3rdparty/libprocess/src/process.cpp (line 189)


All code related to this go to another patch.



3rdparty/libprocess/src/process.cpp (lines 2334 - 2336)


if we do move, remove this comment.



3rdparty/libprocess/src/process.cpp (line 2337)


No pointer +  std::move.



3rdparty/libprocess/src/process.cpp (lines 2341 - 2343)


Reuse similar comments for this code there.



3rdparty/libprocess/src/tests/http_tests.cpp (line 1178)


to separate patch, change other test and verify this one is actually needed.

Also write down that we need to reorder the ProcessBase::visit



3rdparty/libprocess/src/tests/http_tests.cpp (line 1183)


Remove.



3rdparty/libprocess/src/tests/http_tests.cpp (line 1218)


Try to reuse requests.


- Alexander Rojas


On Nov. 10, 2015, 4:58 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Nov. 10, 2015, 4:58 a.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 
> a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 7eb4ef187b2cb358c370d0381db65b8e18668bab 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-11-09 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Nov. 10, 2015, 4:58 a.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Rebase


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
  3rdparty/libprocess/src/tests/http_tests.cpp 
7eb4ef187b2cb358c370d0381db65b8e18668bab 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-11-05 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Nov. 5, 2015, 6:05 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Rebase.


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
  3rdparty/libprocess/src/tests/http_tests.cpp 
7eb4ef187b2cb358c370d0381db65b8e18668bab 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-11-04 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Nov. 4, 2015, 12:22 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Rebasing.


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp a94712b9ac3b60fb047b3a5a4d84a56fa4d02313 
  3rdparty/libprocess/src/tests/http_tests.cpp 
7eb4ef187b2cb358c370d0381db65b8e18668bab 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-22 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Oct. 22, 2015, 2:07 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Addresses issues in bernd-mesos review.


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-21 Thread Bernd Mathiske 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review103410
---



3rdparty/libprocess/include/process/event.hpp (line 123)


Suggestion: name this parameter "responseFuture"?

(Below, "promise" and "future" is short, but it carries too little meaning 
to follow the code easily without looking up and memorizing declarations.)



3rdparty/libprocess/src/process.cpp (line 216)


Aren't we making a copy here, too, now? Let's add that comment here!



3rdparty/libprocess/src/process.cpp (line 1034)


Why wrap request? This is already of the right type. And the incoming 
parameter is of the exact same const ref type as the expected argument.

Copying happens all the way down in this call chain.


- Bernd Mathiske


On Oct. 21, 2015, 6:18 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 21, 2015, 6:18 a.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 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-21 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Oct. 21, 2015, 3:18 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Rebase.


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-21 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Oct. 21, 2015, 11:41 a.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Adresses issues in Till's review.


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-21 Thread Alexander Rojas 


> On Oct. 21, 2015, 3:47 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2351
> > 
> >
> > The existing is incorrect but not sure which option is correct:
> > 
> > A. s/event's/events'/
> > B. s/event's/event/

Check [this](http://www.grammarbook.com/punctuation/apostro.asp) and went with 
_events'_.


> On Oct. 21, 2015, 3:47 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2349
> > 
> >
> > There is something wrong with the commas and periods - please check 
> > with a native speaker.

You're right, I checked some grammar books. No comma here.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review103336
---


On Oct. 16, 2015, 9:28 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 16, 2015, 9:28 a.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 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-21 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review103377
---



3rdparty/libprocess/src/process.cpp (line 2344)


You're right, check some grammar books, not comma in this case.



3rdparty/libprocess/src/process.cpp (line 2346)


Check [this](http://www.grammarbook.com/punctuation/apostro.asp) and went 
with _events'_.


- Alexander Rojas


On Oct. 16, 2015, 9:28 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 16, 2015, 9:28 a.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 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-20 Thread Till Toenshoff 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review103336
---



3rdparty/libprocess/src/process.cpp (line 2339)


s/handle/handled/
s/si/is/



3rdparty/libprocess/src/process.cpp (line 2344)


There is something wrong with the commas and periods - please check with a 
native speaker.



3rdparty/libprocess/src/process.cpp (line 2346)


The existing is incorrect but not sure which option is correct:

A. s/event's/events'/
B. s/event's/event/



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1155 - 1161)


Great description!



3rdparty/libprocess/src/tests/http_tests.cpp (line 1156)


s/to/two/



3rdparty/libprocess/src/tests/http_tests.cpp (line 1261)


s/expecting/expected/
s/finished its/finished processing its/


- Till Toenshoff


On Oct. 16, 2015, 7:28 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 16, 2015, 7:28 a.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 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-16 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review102907
---


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


On Oct. 16, 2015, 7:28 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 16, 2015, 7:28 a.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 
> 954d31424bc8f8ecfa78b80513c480f2514ec271 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-16 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Oct. 16, 2015, 9:28 a.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Addresses comments by Benh.


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 (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp 954d31424bc8f8ecfa78b80513c480f2514ec271 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-15 Thread Alexander Rojas 


> On Oct. 14, 2015, 2:17 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2354
> > 
> >
> > 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&& 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?

So the deal here with moving is with the `dispatch` function which is created 
via some boost macros to simulate variadic templates. So if we made something 
like the requested code `dispatch` gets resolved to:

```cpp
template 
void dispatch(const Process* process,
  void (T::*method)(P0, P1),
  A0 a0,
  A1 a1)
{
  dispatch(process->self(), method, a0, a1);
}
```

Which in turn calls another template function

```cpp
template 
void dispatch(const Process& process,
  void (T::*method)(P0, P1),
  A0 a0,
  A1 a1)
{
  dispatch(process.self(), method, a0, a1);
}
```

we can make the first `dispatch` to receive an rvalue reference, but since it 
doesn't forward its parameters with an `std::move` to the second, they get 
copied, but for some reason as constants values. When the third dispatch gets 
called:

```cpp
template 
void dispatch(const PID& pid, void (T::*method)(P0, P1), A0 a0, A1 a1)
{
  std::shared_ptr> f(
new std::function([=](ProcessBase* process) {
  assert(process != NULL);
  T* t = dynamic_cast(process);
  assert(t != NULL);
  (t->*method)(a0, a1);
}));
  internal::dispatch(pid, f, &typeid(method));
}
```

Creates an error where we are trying to move a `const reference`. So even if it 
worked, we wouldn't be saving copies since `dispatch` will be doing them 
anyway. However the pointer type is really unnecessary given that copies of 
`Future` are not expensive, so instead of a pointer, we can have a copy and 
the code will look like the one you proposed (removing that `new 
Future`) adn the copy of request is already there.

If there is nothing against this idea, I will be proceeding with it.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review102607
---


On Oct. 14, 2015, 5:35 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, 5:35 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
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-14 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review102645
---


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 3:35 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, 3:35 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
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-14 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Oct. 14, 2015, 5:35 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Updates test.
Fixed some of ben's issues.


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 (updated)
-

  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

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-14 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review102619
---


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


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
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-14 Thread Benjamin Hindman 

---
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)


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)


Promise* promise = new Promise();

Or:

auto promise = new Promise();



3rdparty/libprocess/src/process.cpp (line 2354)


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&& 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
> 
>

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-14 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

(Updated Oct. 14, 2015, 2:04 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


Changes
---

Rewrites the test.


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 (updated)
-

  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

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-13 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/#review102462
---


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 3 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 13, 2015, 3 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 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

2015-10-13 Thread Alexander Rojas 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39276/
---

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 d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 

Diff: https://reviews.apache.org/r/39276/diff/


Testing
---

make check


Thanks,

Alexander Rojas