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 2017/02/18 18:40:36 UTC
Review Request 56814: Wrapped IOSwitchboard.connect() in a dispatch.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56814/
-----------------------------------------------------------
Review request for mesos and Jie Yu.
Bugs: MESOS-7144
https://issues.apache.org/jira/browse/MESOS-7144
Repository: mesos
Description
-------
Since the IOSwitchboard is implemented as a MesosIsolatorProcess, most
of its API calls are automatically dispatched onto its underlying
process by an Isolator wrapper. However, the IOSwitchboard also
includes an additional connect() call which is not accessed through
the Isolator wrapper. As such, we need to wrap it in a dispatch call
manually.
Diffs
-----
src/slave/containerizer/mesos/io/switchboard.hpp 7a7ad2da047ef316f4382e45526d503c87a903a0
src/slave/containerizer/mesos/io/switchboard.cpp b948f3c44dd2e1af2228ca1579f24ae3a94160b0
Diff: https://reviews.apache.org/r/56814/diff/
Testing
-------
GTEST_FILTER="" make -j check
sudo src/mesos-tests
Thanks,
Kevin Klues
Re: Review Request 56814: Wrapped IOSwitchboard.connect() in a
dispatch.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56814/#review166124
-----------------------------------------------------------
Ship it!
Good catch!
- Jie Yu
On Feb. 18, 2017, 6:40 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56814/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2017, 6:40 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-7144
> https://issues.apache.org/jira/browse/MESOS-7144
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the IOSwitchboard is implemented as a MesosIsolatorProcess, most
> of its API calls are automatically dispatched onto its underlying
> process by an Isolator wrapper. However, the IOSwitchboard also
> includes an additional connect() call which is not accessed through
> the Isolator wrapper. As such, we need to wrap it in a dispatch call
> manually.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.hpp 7a7ad2da047ef316f4382e45526d503c87a903a0
> src/slave/containerizer/mesos/io/switchboard.cpp b948f3c44dd2e1af2228ca1579f24ae3a94160b0
>
> Diff: https://reviews.apache.org/r/56814/diff/
>
>
> Testing
> -------
>
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 56814: Wrapped IOSwitchboard.connect() in a
dispatch.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56814/
-----------------------------------------------------------
(Updated Feb. 22, 2017, 7:49 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Addressed Benjamin Bannier's comments.
Bugs: MESOS-7144
https://issues.apache.org/jira/browse/MESOS-7144
Repository: mesos
Description
-------
Since the IOSwitchboard is implemented as a MesosIsolatorProcess, most
of its API calls are automatically dispatched onto its underlying
process by an Isolator wrapper. However, the IOSwitchboard also
includes an additional connect() call which is not accessed through
the Isolator wrapper. As such, we need to wrap it in a dispatch call
manually.
Diffs (updated)
-----
src/slave/containerizer/mesos/io/switchboard.hpp 7a7ad2da047ef316f4382e45526d503c87a903a0
src/slave/containerizer/mesos/io/switchboard.cpp b948f3c44dd2e1af2228ca1579f24ae3a94160b0
Diff: https://reviews.apache.org/r/56814/diff/
Testing
-------
GTEST_FILTER="" make -j check
sudo src/mesos-tests
Thanks,
Kevin Klues
Re: Review Request 56814: Wrapped IOSwitchboard.connect() in a
dispatch.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56814/#review166244
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/io/switchboard.hpp (lines 110 - 111)
<https://reviews.apache.org/r/56814/#comment238153>
Was the fact that neither `_connect` nor `connect` a deliberate decision?
Currently, already from a semantical point of view they just perform a lookups without modifing internal state; there also is nothing in their impl requiring mutating access.
src/slave/containerizer/mesos/io/switchboard.cpp (line 664)
<https://reviews.apache.org/r/56814/#comment238162>
Stray space after `containerId]`.
- Benjamin Bannier
On Feb. 18, 2017, 7:40 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56814/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2017, 7:40 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-7144
> https://issues.apache.org/jira/browse/MESOS-7144
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the IOSwitchboard is implemented as a MesosIsolatorProcess, most
> of its API calls are automatically dispatched onto its underlying
> process by an Isolator wrapper. However, the IOSwitchboard also
> includes an additional connect() call which is not accessed through
> the Isolator wrapper. As such, we need to wrap it in a dispatch call
> manually.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.hpp 7a7ad2da047ef316f4382e45526d503c87a903a0
> src/slave/containerizer/mesos/io/switchboard.cpp b948f3c44dd2e1af2228ca1579f24ae3a94160b0
>
> Diff: https://reviews.apache.org/r/56814/diff/
>
>
> Testing
> -------
>
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
>
>
> Thanks,
>
> Kevin Klues
>
>