You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kevin Klues <kl...@gmail.com> on 2016/12/07 02:44:04 UTC
Review Request 54465: Added synchronization between agent and
IOSwitchboard server for listen.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54465/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos
Description
-------
We use a pipe to synchronize with the io switchboard process to
determine when it is ready to listen for incoming connections. We do
not block the launch path with this pipe, but rather poll on it and
block incoming connections until it is ready.
Diffs
-----
src/slave/containerizer/mesos/io/switchboard.hpp f691b182d4029a15bbb3b1c158b176d43f265cc1
src/slave/containerizer/mesos/io/switchboard.cpp c54d64efe7dc526473427bca89a1b65205e16018
src/slave/containerizer/mesos/io/switchboard_main.cpp 6ce4cdb08e3e15eda21dcb740c4390613a88f5c5
Diff: https://reviews.apache.org/r/54465/diff/
Testing
-------
GTEST_FILTER="" make -j check
GTEST_FILTER="*IOSwitchboard*" src/mesos-tests
Thanks,
Kevin Klues
Re: Review Request 54465: Added synchronization between agent and
IOSwitchboard server for listen.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54465/#review158303
-----------------------------------------------------------
I posted a simple work around in https://reviews.apache.org/r/54472/
I think we anyway need a time based loop.
src/slave/containerizer/mesos/io/switchboard.cpp (lines 185 - 190)
<https://reviews.apache.org/r/54465/#comment229085>
My main concern is that after agent restarted, we don't have a way to do synchronization.
We probably still need a time based loop here.
src/slave/containerizer/mesos/io/switchboard.cpp (line 685)
<https://reviews.apache.org/r/54465/#comment229086>
I would probably return a failure in .then if the container is being destroyed (i.e., not in 'infos')
- Jie Yu
On Dec. 7, 2016, 3:21 a.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54465/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2016, 3:21 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Bugs: MESOS-6737
> https://issues.apache.org/jira/browse/MESOS-6737
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use a pipe to synchronize with the io switchboard process to
> determine when it is ready to listen for incoming connections. We do
> not block the launch path with this pipe, but rather poll on it and
> block incoming connections until it is ready.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.hpp f691b182d4029a15bbb3b1c158b176d43f265cc1
> src/slave/containerizer/mesos/io/switchboard.cpp c54d64efe7dc526473427bca89a1b65205e16018
> src/slave/containerizer/mesos/io/switchboard_main.cpp 6ce4cdb08e3e15eda21dcb740c4390613a88f5c5
> src/tests/containerizer/io_switchboard_tests.cpp 851a1015294882ca5401cb8fd60ceff6f933249d
>
> Diff: https://reviews.apache.org/r/54465/diff/
>
>
> Testing
> -------
>
> GTEST_FILTER="" make -j check
> GTEST_FILTER="*IOSwitchboard*" src/mesos-tests
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 54465: Added synchronization between agent and
IOSwitchboard server for listen.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54465/#review158301
-----------------------------------------------------------
LGTM overall
src/slave/containerizer/mesos/io/switchboard.hpp (line 235)
<https://reviews.apache.org/r/54465/#comment229080>
why is this optional? shouldn't this be required?
src/slave/containerizer/mesos/io/switchboard.cpp (lines 631 - 645)
<https://reviews.apache.org/r/54465/#comment229083>
can this be?
```
infos[containerId]->listening = process::io::poll(..)
.then([serverListeningPipe]() {
os::close(serverListeningPipe[0]);
return Nothing();
})
```
src/slave/containerizer/mesos/io/switchboard.cpp
<https://reviews.apache.org/r/54465/#comment229084>
why is this note no longer valid?
src/slave/containerizer/mesos/io/switchboard_main.cpp (line 97)
<https://reviews.apache.org/r/54465/#comment229075>
looks like at this point the server has called `listen` but not `accept`. i guess that should be ok?
- Vinod Kone
On Dec. 7, 2016, 3:21 a.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54465/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2016, 3:21 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Bugs: MESOS-6737
> https://issues.apache.org/jira/browse/MESOS-6737
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use a pipe to synchronize with the io switchboard process to
> determine when it is ready to listen for incoming connections. We do
> not block the launch path with this pipe, but rather poll on it and
> block incoming connections until it is ready.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.hpp f691b182d4029a15bbb3b1c158b176d43f265cc1
> src/slave/containerizer/mesos/io/switchboard.cpp c54d64efe7dc526473427bca89a1b65205e16018
> src/slave/containerizer/mesos/io/switchboard_main.cpp 6ce4cdb08e3e15eda21dcb740c4390613a88f5c5
> src/tests/containerizer/io_switchboard_tests.cpp 851a1015294882ca5401cb8fd60ceff6f933249d
>
> Diff: https://reviews.apache.org/r/54465/diff/
>
>
> Testing
> -------
>
> GTEST_FILTER="" make -j check
> GTEST_FILTER="*IOSwitchboard*" src/mesos-tests
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 54465: Added synchronization between agent and
IOSwitchboard server for listen.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54465/
-----------------------------------------------------------
(Updated Dec. 7, 2016, 3:21 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Added a test and fixed a small bug in `main` (which was never writing anything to the pipe originally!).
Bugs: MESOS-6737
https://issues.apache.org/jira/browse/MESOS-6737
Repository: mesos
Description
-------
We use a pipe to synchronize with the io switchboard process to
determine when it is ready to listen for incoming connections. We do
not block the launch path with this pipe, but rather poll on it and
block incoming connections until it is ready.
Diffs (updated)
-----
src/slave/containerizer/mesos/io/switchboard.hpp f691b182d4029a15bbb3b1c158b176d43f265cc1
src/slave/containerizer/mesos/io/switchboard.cpp c54d64efe7dc526473427bca89a1b65205e16018
src/slave/containerizer/mesos/io/switchboard_main.cpp 6ce4cdb08e3e15eda21dcb740c4390613a88f5c5
src/tests/containerizer/io_switchboard_tests.cpp 851a1015294882ca5401cb8fd60ceff6f933249d
Diff: https://reviews.apache.org/r/54465/diff/
Testing
-------
GTEST_FILTER="" make -j check
GTEST_FILTER="*IOSwitchboard*" src/mesos-tests
Thanks,
Kevin Klues
Re: Review Request 54465: Added synchronization between agent and
IOSwitchboard server for listen.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54465/
-----------------------------------------------------------
(Updated Dec. 7, 2016, 2:47 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Bugs: MESOS-6737
https://issues.apache.org/jira/browse/MESOS-6737
Repository: mesos
Description
-------
We use a pipe to synchronize with the io switchboard process to
determine when it is ready to listen for incoming connections. We do
not block the launch path with this pipe, but rather poll on it and
block incoming connections until it is ready.
Diffs
-----
src/slave/containerizer/mesos/io/switchboard.hpp f691b182d4029a15bbb3b1c158b176d43f265cc1
src/slave/containerizer/mesos/io/switchboard.cpp c54d64efe7dc526473427bca89a1b65205e16018
src/slave/containerizer/mesos/io/switchboard_main.cpp 6ce4cdb08e3e15eda21dcb740c4390613a88f5c5
Diff: https://reviews.apache.org/r/54465/diff/
Testing
-------
GTEST_FILTER="" make -j check
GTEST_FILTER="*IOSwitchboard*" src/mesos-tests
Thanks,
Kevin Klues