You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2017/01/09 20:35:03 UTC
Review Request 55355: Fixed an FD leak in the IO switchboard.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55355/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin Mahler, Joseph Wu, and Kevin Klues.
Repository: mesos
Description
-------
Previously, the IO switchboard could leak file descriptors
because it held a reference to its server socket within the
socket's accept loop. This patch explicitly discards the
future containing this reference to eliminate the leak.
Diffs
-----
src/slave/containerizer/mesos/io/switchboard.cpp 67c51678e87389ab08a81fe4832cffd3d83bdae9
Diff: https://reviews.apache.org/r/55355/diff/
Testing
-------
Run:
`bin/mesos-tests.sh --gtest_repeat=-1 --gtest_filter="IOSwitchboardServerTest.AttachInput"`
and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` repeatedly to ensure that the number of open FDs is not increasing.
Thanks,
Greg Mann
Re: Review Request 55355: Fixed an FD leak in the IO switchboard.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55355/#review160992
-----------------------------------------------------------
I'd run this change my @jie.
src/slave/containerizer/mesos/io/switchboard.cpp (line 993)
<https://reviews.apache.org/r/55355/#comment232237>
Can you move this declaration just below the `inputConnected` declaration?
It seems to group better there for me.
src/slave/containerizer/mesos/io/switchboard.cpp (line 1265)
<https://reviews.apache.org/r/55355/#comment232239>
It's probably worth mentioning here why we grab a reference to the accept (i.e. so that we can discard it in `finalize()` so that they FD associated with the socket doesn't get leaked).
- Kevin Klues
On Jan. 9, 2017, 8:35 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55355/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 8:35 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin Mahler, Joseph Wu, and Kevin Klues.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, the IO switchboard could leak file descriptors
> because it held a reference to its server socket within the
> socket's accept loop. This patch explicitly discards the
> future containing this reference to eliminate the leak.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/io/switchboard.cpp 67c51678e87389ab08a81fe4832cffd3d83bdae9
>
> Diff: https://reviews.apache.org/r/55355/diff/
>
>
> Testing
> -------
>
> Run:
> `bin/mesos-tests.sh --gtest_repeat=-1 --gtest_filter="IOSwitchboardServerTest.AttachInput"`
>
> and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` repeatedly to ensure that the number of open FDs is not increasing.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 55355: Fixed an FD leak in the IO switchboard.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55355/
-----------------------------------------------------------
(Updated Jan. 9, 2017, 8:35 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin Mahler, Joseph Wu, and Kevin Klues.
Repository: mesos
Description
-------
Previously, the IO switchboard could leak file descriptors
because it held a reference to its server socket within the
socket's accept loop. This patch explicitly discards the
future containing this reference to eliminate the leak.
Diffs
-----
src/slave/containerizer/mesos/io/switchboard.cpp 67c51678e87389ab08a81fe4832cffd3d83bdae9
Diff: https://reviews.apache.org/r/55355/diff/
Testing
-------
Run:
`bin/mesos-tests.sh --gtest_repeat=-1 --gtest_filter="IOSwitchboardServerTest.AttachInput"`
and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` repeatedly to ensure that the number of open FDs is not increasing.
Thanks,
Greg Mann