Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On July 11, 2017, 5:48 p.m., Benjamin Mahler wrote:
> > src/linux/perf.cpp
> > Line 238 (original), 243-249 (patched)
> > 
> >
> > This is executing `perf --version` twice, and if someone were to call 
> > `perf::version()` (or other `perf::` functions) without calling 
> > `perf::supported()` then the core dump issue would surface again?
> > 
> > Could we instead just update `perf::version` and all other perf 
> > commands we run to disable core dumps? They all should be going through the 
> > `Perf` class in the implementation.
> > 
> > (I don't think we mandated that any `perf::` function must only be 
> > called after `perf::supported()` has been called and returned true)

`perf --check` is moved to `Perf::execute()`.


- Andrei


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


On July 11, 2017, 9:25 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 11, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Benjamin Bannier, James Peach, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/5/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Benjamin Mahler]

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




src/linux/perf.cpp
Line 238 (original), 243-249 (patched)


This is executing `perf --version` twice, and if someone were to call 
`perf::version()` (or other `perf::` functions) without calling 
`perf::supported()` then the core dump issue would surface again?

Could we instead just update `perf::version` and all other perf commands we 
run to disable core dumps? They all should be going through the `Perf` class in 
the implementation.

(I don't think we mandated that any `perf::` function must only be called 
after `perf::supported()` has been called and returned true)


- Benjamin Mahler


On July 11, 2017, 9:25 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 11, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Benjamin Bannier, James Peach, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/4/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On July 7, 2017, 1:15 p.m., Alexander Rojas wrote:
> > src/linux/perf.cpp
> > Lines 245 (patched)
> > 
> >
> > In this case, `LOG(ERROR)` is the wrong log level, since you may expect 
> > in certain cases for the error to occur.
> > 
> > `LOG(INFO)` seems more reasonable.
> 
> Andrei Budnik wrote:
> But I see `LOG(ERROR)` below in the code in `supported()`. In addition, 
> it looks like it's a real issue when perf is broken or isn't installed. If 
> perf isn't available, then it's the issue that should be fixed, right?

Ok. Fixed.


- Andrei


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


On July 3, 2017, 10:57 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 10:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/4/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On July 7, 2017, 1:15 p.m., Alexander Rojas wrote:
> > src/linux/perf.cpp
> > Lines 245 (patched)
> > 
> >
> > In this case, `LOG(ERROR)` is the wrong log level, since you may expect 
> > in certain cases for the error to occur.
> > 
> > `LOG(INFO)` seems more reasonable.

But I see `LOG(ERROR)` below in the code in `supported()`. In addition, it 
looks like it's a real issue when perf is broken or isn't installed. If perf 
isn't available, then it's the issue that should be fixed, right?


- Andrei


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


On July 3, 2017, 10:57 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 10:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On July 7, 2017, 1:15 p.m., Alexander Rojas wrote:
> > src/linux/perf.cpp
> > Lines 243 (patched)
> > 
> >
> > Why this command? two lines later `perf::version()` is called. Isn't 
> > there a way to merge this with the `perf::version()` call?

`perf::version()` also used in tests:
```cpp
// Returns the detected perf version. Exposed for testing.
process::Future version();
```
If I move this code to `perf::version()`, then its failure might be interpreted 
in multiple ways:
1. `perf --version` failed => perf isn’t installed or coredumped
2. `parseVersion()` failed
`PerfTest.Version` test checks whether we are able to parse perf version. If 
perf isn’t installed, the test shall pass. Quote from James:
“This drops the test that verifies we can actually parse perf versions that are 
found in the wild. That test is necessary to catch when new systems play games 
with the perf version; otherwise we would just detect that the perf version 
isn’t supported and it would be hard to diagnose why not.”


- Andrei


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


On July 3, 2017, 10:57 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 10:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Alexander Rojas]

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




src/linux/perf.cpp
Lines 243 (patched)


Why this command? two lines later `perf::version()` is called. Isn't there 
a way to merge this with the `perf::version()` call?



src/linux/perf.cpp
Lines 245 (patched)


In this case, `LOG(ERROR)` is the wrong log level, since you may expect in 
certain cases for the error to occur.

`LOG(INFO)` seems more reasonable.


- Alexander Rojas


On July 3, 2017, 12:57 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 12:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Benjamin Bannier]

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


Ship it!




I assume this makes the workaround from 
`fcf8618be59ada6d1607696b13bc362bf7e85b0b` unnecessary? In that case, could you 
submit a RR to revert it?

- Benjamin Bannier


On July 3, 2017, 12:57 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 12:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On June 27, 2017, 5:44 a.m., Benjamin Mahler wrote:
> > src/tests/environment.cpp
> > Line 542 (original), 542-548 (patched)
> > 
> >
> > Shouldn't this just be covered by `perf::suppported`?
> > 
> > Would also be great to include some additional context here, like a 
> > pointer to the ticket or mentioning that this is more likely if running 
> > within a docker image?
> 
> Andrei Budnik wrote:
> Ticket is [MESOS-7160](https://issues.apache.org/jira/browse/MESOS-7160).
> 
> What I've got from the description of the problem is that `perf 
> --version` segfaults on an incompatible kernel version.
> The solution for this problem is provided in this patch. Then, we have 
> found that it is more likely caused by incorrect `os::subprocess()` behavior. 
> So, I have suspicion that `perf --version` might never really cause 
> segfaults. I'm going to fix `os::subprocess()` and send a new patch. 
> Afterwards, if the problem reproduces after the fix, then I'll continue to 
> work on this patch.
> 
> `perf::suppported()` invokes `os::subprocess()` which calls `abort()` 
> when path to a binary is invalid. SIGABRT causes coredumps.
> 
> James Peach wrote:
> Both of these are true :) `perf` will crash with kernel version 
> mismatches, and I strongly suspect there is a race in the process supervisor 
> that can cause a similar symptom when `perf` is not installed.
> 
> Benjamin Mahler wrote:
> I think my comment was misunderstood, I'm asking why we wouldn't have 
> perf::supported handle this correctly, rather than every caller having to 
> write logic to defend against an implementation issue in perf::supported.

I agree with you. Fixed.


- Andrei


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


On July 3, 2017, 10:57 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 10:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

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

(Updated July 3, 2017, 10:57 a.m.)


Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.


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


Repository: mesos


Description
---

For autotools build, the docker-build script performs a 'distcheck'
build. This type of build warns if any unexpected files are left in
the build directory after an uninstall, mainly to detect broken
uninstall Makefile rules. The return status of the build container is
the result of the distcheck.
This fixes an issue where in some dockerized configurations
invocations of 'perf' segfaulted (producing a core file as a
side-effect), where the failure case was already anticipated and
handled by the caller.


Diffs (updated)
-

  src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
  src/tests/containerizer/perf_tests.cpp 
d8aab08eb131f974821fb85662cbc6cc685d2f3e 


Diff: https://reviews.apache.org/r/60397/diff/3/

Changes: https://reviews.apache.org/r/60397/diff/2-3/


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI

Needs to be confirmed by Apache CI, e.g., reviewbot.


Thanks,

Andrei Budnik

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Benjamin Mahler]

> On June 27, 2017, 5:44 a.m., Benjamin Mahler wrote:
> > src/tests/environment.cpp
> > Line 542 (original), 542-548 (patched)
> > 
> >
> > Shouldn't this just be covered by `perf::suppported`?
> > 
> > Would also be great to include some additional context here, like a 
> > pointer to the ticket or mentioning that this is more likely if running 
> > within a docker image?
> 
> Andrei Budnik wrote:
> Ticket is [MESOS-7160](https://issues.apache.org/jira/browse/MESOS-7160).
> 
> What I've got from the description of the problem is that `perf 
> --version` segfaults on an incompatible kernel version.
> The solution for this problem is provided in this patch. Then, we have 
> found that it is more likely caused by incorrect `os::subprocess()` behavior. 
> So, I have suspicion that `perf --version` might never really cause 
> segfaults. I'm going to fix `os::subprocess()` and send a new patch. 
> Afterwards, if the problem reproduces after the fix, then I'll continue to 
> work on this patch.
> 
> `perf::suppported()` invokes `os::subprocess()` which calls `abort()` 
> when path to a binary is invalid. SIGABRT causes coredumps.
> 
> James Peach wrote:
> Both of these are true :) `perf` will crash with kernel version 
> mismatches, and I strongly suspect there is a race in the process supervisor 
> that can cause a similar symptom when `perf` is not installed.

I think my comment was misunderstood, I'm asking why we wouldn't have 
perf::supported handle this correctly, rather than every caller having to write 
logic to defend against an implementation issue in perf::supported.


- Benjamin


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


On June 30, 2017, 5:41 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated June 30, 2017, 5:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
>   src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/2/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

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

(Updated June 30, 2017, 5:41 p.m.)


Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.


Summary (updated)
-

Check perf version compatibility in tests with disabled coredumps.


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


Repository: mesos


Description
---

For autotools build, the docker-build script performs a 'distcheck'
build. This type of build warns if any unexpected files are left in
the build directory after an uninstall, mainly to detect broken
uninstall Makefile rules. The return status of the build container is
the result of the distcheck.
This fixes an issue where in some dockerized configurations
invocations of 'perf' segfaulted (producing a core file as a
side-effect), where the failure case was already anticipated and
handled by the caller.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
d8aab08eb131f974821fb85662cbc6cc685d2f3e 
  src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 


Diff: https://reviews.apache.org/r/60397/diff/2/

Changes: https://reviews.apache.org/r/60397/diff/1-2/


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI

Needs to be confirmed by Apache CI, e.g., reviewbot.


Thanks,

Andrei Budnik

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[James Peach]

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


Ship it!




Great idea! What do you think about setting the core rlimit to 0 in the the 
`subprocess` clone lambda in `perf::execute()`?

- James Peach


On June 23, 2017, 8:16 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated June 23, 2017, 8:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/1/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>