You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2015/09/25 22:57:08 UTC
Review Request 38774: state: fix file descriptor leak
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/
-----------------------------------------------------------
Review request for mesos.
Bugs: mesos-3519
https://issues.apache.org/jira/browse/mesos-3519
Repository: mesos
Description
-------
state: fix file descriptor leak
Diffs
-----
src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
Diff: https://reviews.apache.org/r/38774/diff/
Testing
-------
Thanks,
Chi Zhang
Re: Review Request 38774: state: fix file descriptor leak
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100884
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38774]
All tests passed.
- Mesos ReviewBot
On Sept. 28, 2015, 8:42 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 28, 2015, 8:42 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100907
-----------------------------------------------------------
Ship it!
I'll get this committed shortly, thanks!
src/slave/state.cpp (line 655)
<https://reviews.apache.org/r/38774/#comment158166>
Ditto here
src/slave/state.cpp (line 731)
<https://reviews.apache.org/r/38774/#comment158165>
newline after this :)
- Ben Mahler
On Sept. 28, 2015, 8:42 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 28, 2015, 8:42 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/
-----------------------------------------------------------
(Updated Sept. 28, 2015, 8:42 p.m.)
Review request for mesos and Ben Mahler.
Bugs: mesos-3519
https://issues.apache.org/jira/browse/mesos-3519
Repository: mesos
Description
-------
state: fix file descriptor leak
Diffs (updated)
-----
src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
Diff: https://reviews.apache.org/r/38774/diff/
Testing
-------
make check
Thanks,
Chi Zhang
Re: Review Request 38774: state: fix file descriptor leak
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100690
-----------------------------------------------------------
Bad review!
Reviews applied: []
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos ReviewBot
On Sept. 25, 2015, 9 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 9 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100713
-----------------------------------------------------------
src/slave/state.cpp (lines 666 - 680)
<https://reviews.apache.org/r/38774/#comment157973>
Would you mind also just flattening this down to an os::close? This code is not helpful and it will help you perform a single close after the truncate.
src/slave/state.cpp (lines 756 - 770)
<https://reviews.apache.org/r/38774/#comment157974>
Ditto here.
- Ben Mahler
On Sept. 25, 2015, 9 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 9 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Anand Mazumdar <ma...@gmail.com>.
> On Sept. 26, 2015, 10:57 p.m., Neil Conway wrote:
> > Is there already an RAII wrapper for os::close() we can use? If not, might be worth writing one.
Not yet AFAIK, but related JIRA to have one : https://issues.apache.org/jira/browse/MESOS-3520
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100765
-----------------------------------------------------------
On Sept. 25, 2015, 9 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 9 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100765
-----------------------------------------------------------
Is there already an RAII wrapper for os::close() we can use? If not, might be worth writing one.
- Neil Conway
On Sept. 25, 2015, 9 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 9 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100688
-----------------------------------------------------------
src/slave/state.cpp (line 656)
<https://reviews.apache.org/r/38774/#comment157949>
Can be moved upper, out of this if/else, save one line of code. ;)
src/slave/state.cpp (line 746)
<https://reviews.apache.org/r/38774/#comment157950>
Ditto
- Cong Wang
On Sept. 25, 2015, 9 p.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38774/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 9 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
>
>
> Repository: mesos
>
>
> Description
> -------
>
> state: fix file descriptor leak
>
>
> Diffs
> -----
>
> src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
>
> Diff: https://reviews.apache.org/r/38774/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38774: state: fix file descriptor leak
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 9 p.m.)
Review request for mesos.
Bugs: mesos-3519
https://issues.apache.org/jira/browse/mesos-3519
Repository: mesos
Description
-------
state: fix file descriptor leak
Diffs
-----
src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0
Diff: https://reviews.apache.org/r/38774/diff/
Testing (updated)
-------
make check
Thanks,
Chi Zhang