Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-13 Thread Alexander Rojas 


> On Dec. 13, 2015, 11:55 a.m., Alexander Rojas wrote:
> > Ship It!

to clarify, it is a fiix-it the. ship it.


- Alexander


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


On Dec. 13, 2015, 12:11 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 13, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-13 Thread Alexander Rojas 

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

Ship it!


Ship It!

- Alexander Rojas


On Dec. 13, 2015, 12:11 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 13, 2015, 12:11 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-13 Thread Ben Mahler 


> On Dec. 13, 2015, 12:34 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, lines 1778-1794
> > 
> >
> > This test also suffers from the same problem.
> > Mind fixing this one as well?

Thanks for noticing that!


> On Dec. 13, 2015, 12:34 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, line 1416
> > 
> >
> > We capture the response, but don't use it later.
> > Should we either:
> > (1) not capture it
> > (2) `ASSERT_AWAIT_READY(response)` before `AWAIT_READY(body)`
> > (3) `ASSERT_AWAIT_READY(connection.send(request))`
> > What are your thoughts?

Hm.. what I'll do to make this even more explicit is I'll add an 
`EXPECT_TRUE(response.isPending())`. We won't receive a response (that's the 
root of the issue here) because libprocess does not send one for this old-style 
pseudo-HTTP protocol.


- Ben


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


On Dec. 12, 2015, 11:11 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-12 Thread Joris Van Remoortere 

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

Ship it!


Thanks for fixing this Ben!


3rdparty/libprocess/src/tests/process_tests.cpp (line 1416)


We capture the response, but don't use it later.
Should we either:
(1) not capture it
(2) `ASSERT_AWAIT_READY(response)` before `AWAIT_READY(body)`
(3) `ASSERT_AWAIT_READY(connection.send(request))`
What are your thoughts?



3rdparty/libprocess/src/tests/process_tests.cpp (lines 1778 - 1794)


This test also suffers from the same problem.
Mind fixing this one as well?


- Joris Van Remoortere


On Dec. 12, 2015, 11:11 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-12 Thread Ben Mahler 

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

Review request for mesos and Alexander Rojas.


Repository: mesos


Description
---

ProcessTest.Http1 hangs after a number of iterations because it uses
http::post to do libprocess message passing but it uses the
"User-Agent" header which means libprocess does not reply with a 202.


Diffs
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
3970dd83aa4ddd2cbe3664c157fc15943ab1182d 

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


Testing
---

Ran with many iterations.


Thanks,

Ben Mahler