You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Budnik <ab...@mesosphere.com> on 2018/08/25 10:12:09 UTC

Review Request 68232: Fixed IOSwitchboard waiting on EOF from attach container input request.

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.


Summary (updated)
-----------------

Fixed IOSwitchboard waiting on EOF from attach container input request.


Bugs: MESOS-9131
    https://issues.apache.org/jira/browse/MESOS-9131


Repository: mesos


Description (updated)
-------

Previously, when a corresponding nested container terminates, while the
user is attached to the container's stdin via `ATTACH_CONTAINER_INPUT`,
IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
for EOF message from the input HTTP connection. Since the IOSwitchboard
is stuck, the corresponding nested container is also stuck in
`DESTROYING` state.

This patch fixes the aforementioned issue by sending 200 `OK` response
for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
finished while reading from the HTTP input connection is not.


Diffs (updated)
-----

  src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 


Diff: https://reviews.apache.org/r/68232/diff/1/


Testing (updated)
-------

1. internal CI
2. sudo make check (Fedora 25)

This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.


Thanks,

Andrei Budnik


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Aug. 30, 2018, 12:24 p.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Line 1704 (original), 1704 (patched)
> > <https://reviews.apache.org/r/68232/diff/2/?file=2077426#file2077426line1705>
> >
> >     Does it have to be `Owned`? If yes and you need to extend the lifetime of the promise (why?), don't you need to capture it explicitly somewhere then?

Yes, it does. Otherwise, `f.abandon()` will be called in `Promise` destructor.


> On Aug. 30, 2018, 12:24 p.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Lines 1706-1712 (patched)
> > <https://reviews.apache.org/r/68232/diff/2/?file=2077426#file2077426line1707>
> >
> >     I think you can simply say
> >     ```
> >     promise->associate(readLoop);
> >     ...
> >     ```

Unfortunately, no. Second attempt to call `promise->associate()` is a no-op and simply returns `false`.


- Andrei


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


On Aug. 25, 2018, 10:12 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/3/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68232/#review208127
-----------------------------------------------------------




src/slave/containerizer/mesos/io/switchboard.cpp
Line 1704 (original), 1704 (patched)
<https://reviews.apache.org/r/68232/#comment291903>

    Does it have to be `Owned`? If yes and you need to extend the lifetime of the promise (why?), don't you need to capture it explicitly somewhere then?



src/slave/containerizer/mesos/io/switchboard.cpp
Lines 1706-1712 (patched)
<https://reviews.apache.org/r/68232/#comment291902>

    I think you can simply say
    ```
    promise->associate(readLoop);
    ...
    ```



src/slave/containerizer/mesos/io/switchboard.cpp
Line 1712 (original), 1722 (patched)
<https://reviews.apache.org/r/68232/#comment291900>

    not is pending seems a safer choice


- Alexander Rukletsov


On Aug. 25, 2018, 10:12 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/2/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68232/#review208261
-----------------------------------------------------------


Ship it!




Ship It!

- Qian Zhang


On Aug. 31, 2018, 7:48 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2018, 7:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/5/
> 
> 
> Testing
> -------
> 
> 1. internal Mesosphere CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

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



Patch looks great!

Reviews applied: [68230, 68231, 68232]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 31, 2018, 4:48 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2018, 4:48 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/5/
> 
> 
> Testing
> -------
> 
> 1. internal Mesosphere CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68232/#review208558
-----------------------------------------------------------


Fix it, then Ship it!





src/slave/containerizer/mesos/io/switchboard.cpp
Line 1225 (original), 1225 (patched)
<https://reviews.apache.org/r/68232/#comment292594>

    ... set `redirectFinished` promise which triggers a callback...



src/slave/containerizer/mesos/io/switchboard.cpp
Line 1227 (original), 1227 (patched)
<https://reviews.apache.org/r/68232/#comment292595>

    .. the EOF message.



src/slave/containerizer/mesos/io/switchboard.cpp
Lines 1707-1708 (patched)
<https://reviews.apache.org/r/68232/#comment292598>

    ```
    //
    // TODO(abudnik): Ideally we would have used `process::select()` to capture a transition into a terminal state for any of `{readLoop, redirectFinished}`. However, `select()` select currently does not capture a future that has failed. Another alternative would be to allow `promise::associate()` to accept multiple source futures.
    ```



src/slave/containerizer/mesos/io/switchboard.cpp
Lines 1710 (patched)
<https://reviews.apache.org/r/68232/#comment292596>

    [promise]


- Alexander Rukletsov


On Aug. 31, 2018, 11:48 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2018, 11:48 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/8/
> 
> 
> Testing
> -------
> 
> 1. internal Mesosphere CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68232/
-----------------------------------------------------------

(Updated Aug. 31, 2018, 11:48 a.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.


Bugs: MESOS-9131
    https://issues.apache.org/jira/browse/MESOS-9131


Repository: mesos


Description
-------

Previously, when a corresponding nested container terminated, while the
user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
for EOF message from the input HTTP connection. Since the IOSwitchboard
was stuck, the corresponding nested container was also stuck in
`DESTROYING` state.

This patch fixes the aforementioned issue by sending 200 `OK` response
for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
finished while reading from the HTTP input connection is not.


Diffs
-----

  src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 


Diff: https://reviews.apache.org/r/68232/diff/5/


Testing (updated)
-------

1. internal Mesosphere CI
2. sudo make check (Fedora 25)

This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.


Thanks,

Andrei Budnik


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

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



PASS: Mesos patch 68232 was successfully built and tested.

Reviews applied: `['68230', '68231', '68232']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2265/mesos-review-68232

- Mesos Reviewbot Windows


On Aug. 30, 2018, 3:46 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2018, 3:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/5/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68232/
-----------------------------------------------------------

(Updated Aug. 30, 2018, 3:46 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.


Changes
-------

Updated comments.


Bugs: MESOS-9131
    https://issues.apache.org/jira/browse/MESOS-9131


Repository: mesos


Description
-------

Previously, when a corresponding nested container terminated, while the
user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
for EOF message from the input HTTP connection. Since the IOSwitchboard
was stuck, the corresponding nested container was also stuck in
`DESTROYING` state.

This patch fixes the aforementioned issue by sending 200 `OK` response
for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
finished while reading from the HTTP input connection is not.


Diffs (updated)
-----

  src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 


Diff: https://reviews.apache.org/r/68232/diff/4/

Changes: https://reviews.apache.org/r/68232/diff/3-4/


Testing
-------

1. internal CI
2. sudo make check (Fedora 25)

This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.


Thanks,

Andrei Budnik


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

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



PASS: Mesos patch 68232 was successfully built and tested.

Reviews applied: `['68230', '68231', '68232']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2239/mesos-review-68232

- Mesos Reviewbot Windows


On Aug. 25, 2018, 10:12 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/2/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

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



Patch looks great!

Reviews applied: [68230, 68231, 68232]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 25, 2018, 10:12 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/2/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Aug. 29, 2018, 10:24 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp
> > Lines 1710 (patched)
> > <https://reviews.apache.org/r/68232/diff/2/?file=2077426#file2077426line1711>
> >
> >     Here we set the `promise` in the `readLoop`'s `onAny` callback, but what if `readLoop` fails? Will the failure be hidden in this case?

`readLoop.onAny()` callback is called when `readLoop` future is failed. So, the failure is will not be hidden.


- Andrei


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


On Aug. 25, 2018, 10:12 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/3/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 68232: Fixed IOSwitchboard waiting EOF from attach container input request.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68232/#review208085
-----------------------------------------------------------




src/slave/containerizer/mesos/io/switchboard.cpp
Lines 1033 (patched)
<https://reviews.apache.org/r/68232/#comment291845>

    I'd suggest to move this to L1037 (righer after `startRedirect`) and still name it `redirectFinished`.



src/slave/containerizer/mesos/io/switchboard.cpp
Lines 1710 (patched)
<https://reviews.apache.org/r/68232/#comment291846>

    Here we set the `promise` in the `readLoop`'s `onAny` callback, but what if `readLoop` fails? Will the failure be hidden in this case?


- Qian Zhang


On Aug. 25, 2018, 6:12 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68232/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2018, 6:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
>     https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, when a corresponding nested container terminated, while the
> user was attached to the container's stdin via `ATTACH_CONTAINER_INPUT`
> IOSwitchboard didn't terminate immediately. IOSwitchboard was waiting
> for EOF message from the input HTTP connection. Since the IOSwitchboard
> was stuck, the corresponding nested container was also stuck in
> `DESTROYING` state.
> 
> This patch fixes the aforementioned issue by sending 200 `OK` response
> for `ATTACH_CONTAINER_INPUT` call in the case when io redirect is
> finished while reading from the HTTP input connection is not.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/68232/diff/2/
> 
> 
> Testing
> -------
> 
> 1. internal CI
> 2. sudo make check (Fedora 25)
> 
> This test fixes `LaunchNestedContainerSessionKillTask` test, which can be found in the first patch of this patch chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>