You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2017/01/08 06:28:43 UTC
Review Request 55311: Added `process::initialize` to default
executor's `main`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55311/
-----------------------------------------------------------
Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
Repository: mesos
Description
-------
The default executor's `main` currently depends on `UPID`, which
networking libraries to do things like retrieve the address of the
current host. On Windows, this means that it is required to initialize
the winsock stack to be initialized before `UPID` is used.
To resolve this issue, we add a call to `process::initialize` at the
beginning of the `main`, which will cause the stack to be initialized
correctly.
Diffs
-----
src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa
Diff: https://reviews.apache.org/r/55311/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 55311: Added `process::initialize` to default
executor's `main`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Jan. 18, 2017, 1:50 a.m., Joseph Wu wrote:
> > src/launcher/default_executor.cpp, line 1055
> > <https://reviews.apache.org/r/55311/diff/1/?file=1599651#file1599651line1055>
> >
> > It actually feels like `process::Winsock winsock;` is more appropriate here, as we want the socket stack to be cleaned up afterwards as well.
> > Either than, or you should add a `process::finalize(true)` befow.
Tearing down winsock before libprocess is terminated will cause ugly failures as the test framework disposes itself. Let's do a call to `process::finalize(true)`.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55311/#review162013
-----------------------------------------------------------
On Jan. 8, 2017, 6:28 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55311/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2017, 6:28 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The default executor's `main` currently depends on `UPID`, which
> networking libraries to do things like retrieve the address of the
> current host. On Windows, this means that it is required to initialize
> the winsock stack to be initialized before `UPID` is used.
>
> To resolve this issue, we add a call to `process::initialize` at the
> beginning of the `main`, which will cause the stack to be initialized
> correctly.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa
>
> Diff: https://reviews.apache.org/r/55311/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55311: Added `process::initialize` to default
executor's `main`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Jan. 18, 2017, 1:50 a.m., Joseph Wu wrote:
> > src/launcher/default_executor.cpp, line 1055
> > <https://reviews.apache.org/r/55311/diff/1/?file=1599651#file1599651line1055>
> >
> > It actually feels like `process::Winsock winsock;` is more appropriate here, as we want the socket stack to be cleaned up afterwards as well.
> > Either than, or you should add a `process::finalize(true)` befow.
>
> Alex Clemmer wrote:
> Tearing down winsock before libprocess is terminated will cause ugly failures as the test framework disposes itself. Let's do a call to `process::finalize(true)`.
This might not have been clear, btw, so let me be more specific: if we use the `Winsock` class, we will shut down the winsock stack when we exit `main`. But, at that point, libprocess will typically still be running, and it is therefore highly likely that it will start throwing errors as (_e.g._) its open sockets start to recieve errors. The desired dispose path is for libprocess to dispose of all of its assets, _and then_ dispose of the winsock stack last. This is the semantics of `process::finalize(true)`, which is why we should use it here and not the `Winsock` class.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55311/#review162013
-----------------------------------------------------------
On Jan. 18, 2017, 6:37 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55311/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 6:37 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The default executor's `main` currently depends on `UPID`, which
> networking libraries to do things like retrieve the address of the
> current host. On Windows, this means that it is required to initialize
> the winsock stack to be initialized before `UPID` is used.
>
> To resolve this issue, we add a call to `process::initialize` at the
> beginning of the `main`, which will cause the stack to be initialized
> correctly.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp 57e4799e750f8f5352a9fec58af40efe432ea865
>
> Diff: https://reviews.apache.org/r/55311/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55311: Added `process::initialize` to default
executor's `main`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55311/#review162013
-----------------------------------------------------------
src/launcher/default_executor.cpp (line 1055)
<https://reviews.apache.org/r/55311/#comment233277>
It actually feels like `process::Winsock winsock;` is more appropriate here, as we want the socket stack to be cleaned up afterwards as well.
Either than, or you should add a `process::finalize(true)` befow.
- Joseph Wu
On Jan. 7, 2017, 10:28 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55311/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2017, 10:28 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The default executor's `main` currently depends on `UPID`, which
> networking libraries to do things like retrieve the address of the
> current host. On Windows, this means that it is required to initialize
> the winsock stack to be initialized before `UPID` is used.
>
> To resolve this issue, we add a call to `process::initialize` at the
> beginning of the `main`, which will cause the stack to be initialized
> correctly.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa
>
> Diff: https://reviews.apache.org/r/55311/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55311: Added `process::initialize` to default
executor's `main`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55311/#review162187
-----------------------------------------------------------
Ship it!
Ship It!
- Joseph Wu
On Jan. 18, 2017, 10:37 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55311/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 10:37 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The default executor's `main` currently depends on `UPID`, which
> networking libraries to do things like retrieve the address of the
> current host. On Windows, this means that it is required to initialize
> the winsock stack to be initialized before `UPID` is used.
>
> To resolve this issue, we add a call to `process::initialize` at the
> beginning of the `main`, which will cause the stack to be initialized
> correctly.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp 57e4799e750f8f5352a9fec58af40efe432ea865
>
> Diff: https://reviews.apache.org/r/55311/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55311: Added `process::initialize` to default
executor's `main`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55311/
-----------------------------------------------------------
(Updated Jan. 18, 2017, 6:37 p.m.)
Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
Changes
-------
Address Joseph's comments.
Repository: mesos
Description
-------
The default executor's `main` currently depends on `UPID`, which
networking libraries to do things like retrieve the address of the
current host. On Windows, this means that it is required to initialize
the winsock stack to be initialized before `UPID` is used.
To resolve this issue, we add a call to `process::initialize` at the
beginning of the `main`, which will cause the stack to be initialized
correctly.
Diffs (updated)
-----
src/launcher/default_executor.cpp 57e4799e750f8f5352a9fec58af40efe432ea865
Diff: https://reviews.apache.org/r/55311/diff/
Testing
-------
Thanks,
Alex Clemmer