You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jojy Varghese <jo...@mesosphere.io> on 2015/12/08 08:03:28 UTC

Review Request 41078: Fixed tests to call socket accept before sending response.

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
-------

By calling accept before sending a response, the server would be ready for
client's next request.


Diffs
-----

  src/tests/containerizer/provisioner_docker_tests.cpp c63bf53fee40ef12536a16e11f4d5224c4e4278e 

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


Testing
-------

make check.


Thanks,

Jojy Varghese


Re: Review Request 41078: Fixed tests to call socket accept before sending response.

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Dec. 8, 2015, 7:08 a.m., Timothy Chen wrote:
> > Can you elaborate more without this fix what happened and why?

This fix moves "Accept" call before sending a response back to the client so that when the client sends the next request, the server is ready to accept it. Before this fix, there was a race condition between server sending the response and client sending the next request and server missing that request because the "accept" was not called yet.


- Jojy


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


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


Re: Review Request 41078: Fixed tests to call socket accept before sending response.

Posted by Timothy Chen <tn...@apache.org>.

> On Dec. 8, 2015, 7:08 a.m., Timothy Chen wrote:
> > Can you elaborate more without this fix what happened and why?
> 
> Jojy Varghese wrote:
>     This fix moves "Accept" call before sending a response back to the client so that when the client sends the next request, the server is ready to accept it. Before this fix, there was a race condition between server sending the response and client sending the next request and server missing that request because the "accept" was not called yet.

Sounds very fishy to me, we don't do this in Mesos itself as we take multiple requests in flight. What's the difference here? And is there something we should do so others don't have to worry about this?


- Timothy


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


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


Re: Review Request 41078: Fixed tests to call socket accept before sending response.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41078/#review109288
-----------------------------------------------------------


Can you elaborate more without this fix what happened and why?

- Timothy Chen


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


Re: Review Request 41078: Fixed tests to call socket accept before sending response.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41078/#review109717
-----------------------------------------------------------

Ship it!


Ship It!

- Timothy Chen


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


Re: Review Request 41078: Fixed tests to call socket accept before sending response.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41078/#review109306
-----------------------------------------------------------


Patch looks great!

Reviews applied: [40872, 40873, 41078]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>